work with the new BaseObject layout in snapshot callbacks

src/base_object-inl.h

@@ -44,7 +44,7 @@ BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
   CHECK_EQ(false, object.IsEmpty());
   CHECK_GE(object->InternalFieldCount(), BaseObject::kInternalFieldCount);
   object->SetAlignedPointerInInternalField(BaseObject::kEmbedderType,
-      &kNodeEmbedderId);
+                                           &kNodeEmbedderId);
   object->SetAlignedPointerInInternalField(
       BaseObject::kSlot,
       static_cast<void*>(this));
@@ -161,8 +161,8 @@ void BaseObject::LazilyInitializedJSTemplateConstructor(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   DCHECK(args.IsConstructCall());
   CHECK_GE(args.This()->InternalFieldCount(), BaseObject::kInternalFieldCount);
-  args.This()->SetAlignedPointerInInternalField(
-      BaseObject::kEmbedderType, &kNodeEmbedderId);
+  args.This()->SetAlignedPointerInInternalField(BaseObject::kEmbedderType,
+                                                &kNodeEmbedderId);
   args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
 }
 

src/env.cc

@@ -1444,6 +1444,7 @@ void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb,
                                             Local<Object> holder,
                                             int index,
                                             InternalFieldInfo* info) {
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   DeserializeRequest request{cb, {isolate(), holder}, index, info};
   deserialize_requests_.push_back(std::move(request));
 }

src/node_blob.cc

@@ -479,7 +479,7 @@ void BlobBindingData::PrepareForSerialization(
 }
 
 InternalFieldInfo* BlobBindingData::Serialize(int index) {
-  DCHECK_EQ(index, BaseObject::kSlot);
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   InternalFieldInfo* info = InternalFieldInfo::New(type());
   return info;
 }

src/node_file.cc

@@ -2433,7 +2433,7 @@ void BindingData::Deserialize(Local<Context> context,
                               Local<Object> holder,
                               int index,
                               InternalFieldInfo* info) {
-  DCHECK_EQ(index, BaseObject::kSlot);
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   HandleScope scope(context->GetIsolate());
   Environment* env = Environment::GetCurrent(context);
   BindingData* binding = env->AddBindingData<BindingData>(context, holder);
@@ -2450,7 +2450,7 @@ void BindingData::PrepareForSerialization(Local<Context> context,
 }
 
 InternalFieldInfo* BindingData::Serialize(int index) {
-  DCHECK_EQ(index, BaseObject::kSlot);
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   InternalFieldInfo* info = InternalFieldInfo::New(type());
   return info;
 }

src/node_process_methods.cc

@@ -531,7 +531,7 @@ void BindingData::PrepareForSerialization(Local<Context> context,
 }
 
 InternalFieldInfo* BindingData::Serialize(int index) {
-  DCHECK_EQ(index, BaseObject::kSlot);
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   InternalFieldInfo* info = InternalFieldInfo::New(type());
   return info;
 }
@@ -540,7 +540,7 @@ void BindingData::Deserialize(Local<Context> context,
                               Local<Object> holder,
                               int index,
                               InternalFieldInfo* info) {
-  DCHECK_EQ(index, BaseObject::kSlot);
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   v8::HandleScope scope(context->GetIsolate());
   Environment* env = Environment::GetCurrent(context);
   // Recreate the buffer in the constructor.

src/node_snapshotable.cc

@@ -380,10 +380,13 @@ void DeserializeNodeInternalFields(Local<Object> holder,
     return;
   }
 
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   Environment* env_ptr = static_cast<Environment*>(env);
   const InternalFieldInfo* info =
       reinterpret_cast<const InternalFieldInfo*>(payload.data);
-
+  // TODO(joyeecheung): we can add a constant kNodeEmbedderId to the
+  // beginning of every InternalFieldInfo to ensure that we don't
+  // step on payloads that were not serialized by Node.js.
   switch (info->type) {
 #define V(PropertyName, NativeTypeName)                                        \
   case EmbedderObjectType::k_##PropertyName: {                                 \
@@ -408,13 +411,32 @@ StartupData SerializeNodeContextInternalFields(Local<Object> holder,
                      "Serialize internal field, index=%d, holder=%p\n",
                      static_cast<int>(index),
                      *holder);
-  void* ptr = holder->GetAlignedPointerFromInternalField(BaseObject::kSlot);
-  if (ptr == nullptr) {
+  // We only do one serialization for the kEmbedderType slot, the result
+  // contains everything necessary for deserializing the entire object,
+  // including the fields whose index is bigger than kEmbedderType
+  // (most importantly, BaseObject::kSlot).
+  // For Node.js this design is enough for all the native binding that are
+  // serializable.
+  if (index != BaseObject::kEmbedderType) {
+    return StartupData{nullptr, 0};
+  }
+
+  void* type_ptr = holder->GetAlignedPointerFromInternalField(index);
+  if (type_ptr == nullptr) {
+    return StartupData{nullptr, 0};
+  }
+
+  uint16_t type = *(static_cast<uint16_t*>(type_ptr));
+  per_process::Debug(DebugCategory::MKSNAPSHOT, "type = 0x%x\n", type);
+  if (type != kNodeEmbedderId) {
     return StartupData{nullptr, 0};
   }
 
-  DCHECK(static_cast<BaseObject*>(ptr)->is_snapshotable());
-  SnapshotableObject* obj = static_cast<SnapshotableObject*>(ptr);
+  void* binding_ptr =
+      holder->GetAlignedPointerFromInternalField(BaseObject::kSlot);
+  per_process::Debug(DebugCategory::MKSNAPSHOT, "binding = %p\n", binding_ptr);
+  DCHECK(static_cast<BaseObject*>(binding_ptr)->is_snapshotable());
+  SnapshotableObject* obj = static_cast<SnapshotableObject*>(binding_ptr);
   per_process::Debug(DebugCategory::MKSNAPSHOT,
                      "Object %p is %s, ",
                      *holder,
@@ -434,8 +456,9 @@ void SerializeBindingData(Environment* env,
   env->ForEachBindingData([&](FastStringKey key,
                               BaseObjectPtr<BaseObject> binding) {
     per_process::Debug(DebugCategory::MKSNAPSHOT,
-                       "Serialize binding %i, %p, type=%s\n",
+                       "Serialize binding %i (%p), object=%p, type=%s\n",
                        static_cast<int>(i),
+                       binding.get(),
                        *(binding->object()),
                        key.c_str());
 

src/node_snapshotable.h

@@ -30,11 +30,6 @@ enum class EmbedderObjectType : uint8_t {
 // When serializing an embedder object, we'll serialize the native states
 // into a chunk that can be mapped into a subclass of InternalFieldInfo,
 // and pass it into the V8 callback as the payload of StartupData.
-// TODO(joyeecheung): the classification of types seem to be wrong.
-// We'd need a type for each field of each class of native object.
-// Maybe it's fine - we'll just use the type to invoke BaseObject constructors
-// and specify that the BaseObject has only one field for us to serialize.
-// And for non-BaseObject embedder objects, we'll use field-wise types.
 // The memory chunk looks like this:
 //
 // [   type   ] - EmbedderObjectType (a uint8_t)

src/node_v8.cc

@@ -124,15 +124,15 @@ void BindingData::Deserialize(Local<Context> context,
                               Local<Object> holder,
                               int index,
                               InternalFieldInfo* info) {
-  DCHECK_EQ(index, BaseObject::kSlot);
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   HandleScope scope(context->GetIsolate());
   Environment* env = Environment::GetCurrent(context);
   BindingData* binding = env->AddBindingData<BindingData>(context, holder);
   CHECK_NOT_NULL(binding);
 }
 
 InternalFieldInfo* BindingData::Serialize(int index) {
-  DCHECK_EQ(index, BaseObject::kSlot);
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
   InternalFieldInfo* info = InternalFieldInfo::New(type());
   return info;
 }