[Bindings] Create and use V8 context snapshots

third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp

Index: third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp b/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..da328445d97f02d0a7c5eede630abbb7aa95f7f4
--- /dev/null
+++ b/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp
@@ -0,0 +1,485 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "bindings/core/v8/V8SnapshotUtil.h"
+
+#include <array>
+#include <cstring>
+
+#include "bindings/core/v8/GeneratedCodeHelper.h"
+#include "bindings/core/v8/V8Document.h"
+#include "bindings/core/v8/V8EventTarget.h"
+#include "bindings/core/v8/V8HTMLDocument.h"
+#include "bindings/core/v8/V8Initializer.h"
+#include "bindings/core/v8/V8Node.h"
+#include "bindings/core/v8/V8Window.h"
+#include "platform/bindings/DOMWrapperWorld.h"
+#include "platform/bindings/V8ObjectConstructor.h"
+#include "platform/bindings/V8PerIsolateData.h"
+#include "platform/bindings/V8PrivateProperty.h"
+#include "platform/instrumentation/tracing/TraceEvent.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+namespace {
+
+intptr_t* g_snapshot_reference_table = nullptr;
+
+// TODO(peria): This method is almost a copy of
+// V8PerContext::ConstructorForTypeSlowCase(), so merge with it.
+v8::Local<v8::Function> ConstructPlainType(v8::Isolate* isolate,
+                                           const DOMWrapperWorld& world,
+                                           v8::Local<v8::Context> context,
+                                           const WrapperTypeInfo* type) {
+  v8::Context::Scope scope(context);

[haraken, 2017/07/06 13:15:52]

Add CHECK(V8HTMLDocument::wrapperTypeInfo.Equals(type));.

[peria, 2017/07/07 06:22:00]

if we need it, we need 
 V8HTMLDocument::... || V8Document::... || V8Node::... || V8EventElement::...
because it calls itself recursively. (#48)

+  // We shouldn't reach this point for the types that are implemented in v8 such
+  // as typed arrays and hence don't have domTemplateFunction.
+  DCHECK(type->dom_template_function);
+  v8::Local<v8::FunctionTemplate> interface_template =
+      type->domTemplate(isolate, world);
+  // Getting the function might fail if we're running out of stack or memory.
+  v8::Local<v8::Function> interface_object =
+      interface_template->GetFunction(context).ToLocalChecked();
+
+  if (type->parent_class) {
+    v8::Local<v8::Object> prototype_template =
+        ConstructPlainType(isolate, world, context, type->parent_class);
+    CHECK(interface_object->SetPrototype(context, prototype_template)
+              .ToChecked());
+  }
+
+  v8::Local<v8::Value> prototype_value =
+      interface_object->Get(context, V8AtomicString(isolate, "prototype"))
+          .ToLocalChecked();
+  CHECK(prototype_value->IsObject());
+  v8::Local<v8::Object> prototype_object = prototype_value.As<v8::Object>();
+  if (prototype_object->InternalFieldCount() ==
+          kV8PrototypeInternalFieldcount &&
+      type->wrapper_type_prototype ==
+          WrapperTypeInfo::kWrapperTypeObjectPrototype) {
+    prototype_object->SetAlignedPointerInInternalField(
+        kV8PrototypeTypeIndex, const_cast<WrapperTypeInfo*>(type));
+  }
+  type->PreparePrototypeAndInterfaceObject(
+      context, world, prototype_object, interface_object, interface_template);
+
+  return interface_object;
+}
+
+// TODO(peria): This method is almost a copy of
+// V8PerContext::CreateWrapperFromCacheSlowCase(), so merge with it.
+v8::Local<v8::Object> CreatePlainWrapper(v8::Isolate* isolate,
+                                         const DOMWrapperWorld& world,
+                                         v8::Local<v8::Context> context,
+                                         const WrapperTypeInfo* type) {
+  CHECK(V8HTMLDocument::wrapperTypeInfo.Equals(type));
+
+  v8::Context::Scope scope(context);
+  v8::Local<v8::Function> interface_object =
+      ConstructPlainType(isolate, world, context, type);
+  CHECK(!interface_object.IsEmpty());
+  v8::Local<v8::Object> instance_template =
+      V8ObjectConstructor::NewInstance(isolate, interface_object)
+          .ToLocalChecked();
+  v8::Local<v8::Object> wrapper = instance_template->Clone();
+  wrapper->SetAlignedPointerInInternalField(kV8DOMWrapperTypeIndex,
+                                            const_cast<WrapperTypeInfo*>(type));
+  return wrapper;
+}
+
+constexpr int kWorldIdForNonMainWorld =
+    DOMWrapperWorld::WorldId::kIsolatedWorldIdLimit - 1;
+
+int GetSnapshotIndexForWorld(const DOMWrapperWorld& world) {
+  return world.IsMainWorld() ? 0 : 1;
+}
+
+struct SnapshotInterface {
+  const WrapperTypeInfo* wrapper_type_info;
+  InstallRuntimeEnabledFeaturesOnTemplateFunction install_function;
+};
+
+SnapshotInterface g_snapshot_interfaces[] = {

[haraken, 2017/07/06 13:15:53]

Add a detailed comment about what are contained in the snapshot.

[peria, 2017/07/07 06:22:00]

Done.

+    {&V8Window::wrapperTypeInfo,
+     V8Window::InstallRuntimeEnabledFeaturesOnTemplate},
+    {&V8HTMLDocument::wrapperTypeInfo,
+     V8HTMLDocument::InstallRuntimeEnabledFeaturesOnTemplate},
+    {&V8EventTarget::wrapperTypeInfo,
+     V8EventTarget::InstallRuntimeEnabledFeaturesOnTemplate},
+    {&V8Node::wrapperTypeInfo, V8Node::InstallRuntimeEnabledFeaturesOnTemplate},
+    {&V8Document::wrapperTypeInfo,
+     V8Document::InstallRuntimeEnabledFeaturesOnTemplate},
+};
+constexpr size_t kSnapshotInterfaceSize =
+    WTF_ARRAY_LENGTH(g_snapshot_interfaces);
+
+enum class InternalFieldType : uint8_t {
+  kNone,
+  kNodeType,
+  kDocumentType,
+  kHTMLDocumentType,
+  kHTMLDocumentObject,
+};
+
+const WrapperTypeInfo* FieldTypeToWrapperTypeInfo(InternalFieldType type) {
+  switch (type) {
+    case InternalFieldType::kNone:
+      NOTREACHED();
+      break;
+    case InternalFieldType::kNodeType:
+      return &V8Node::wrapperTypeInfo;
+    case InternalFieldType::kDocumentType:
+      return &V8Document::wrapperTypeInfo;
+    case InternalFieldType::kHTMLDocumentType:
+      return &V8HTMLDocument::wrapperTypeInfo;
+    case InternalFieldType::kHTMLDocumentObject:
+      return &V8HTMLDocument::wrapperTypeInfo;
+  }
+  NOTREACHED();
+  return nullptr;
+}
+
+struct DataForDeserializer {

[haraken, 2017/07/06 13:15:53]

struct => class

[Yuki, 2017/07/06 14:12:28]

I'm okay with "struct => class", but in that case, we should rename "document =>
document_" as well.

[Yuki, 2017/07/06 14:15:25]

Oops, maybe not okay.  I'm not sure, but V8 would expect DataForDeserializer to
be a PoD.  Then, this should be a struct.

[peria, 2017/07/07 06:22:00]

Hmm, is there any background to do it?
I feel it not needed to make it a class; i.e. it needs to make the member
private and to create a getter.

+  STACK_ALLOCATED();
+  Member<Document> document;
+};
+
+int CountExternalReferenceEntries() {
+  if (!g_snapshot_reference_table)
+    return 0;
+
+  int count = 0;
+  for (intptr_t* p = g_snapshot_reference_table; *p; ++p)
+    ++count;
+  return count;
+}
+
+}  // namespace
+
+v8::Local<v8::Context> V8SnapshotUtil::CreateContext(

[haraken, 2017/07/06 13:15:53]

CreateContextFromSnapshot

[peria, 2017/07/07 06:21:58]

Done.

+    v8::Isolate* isolate,
+    const DOMWrapperWorld& world,
+    v8::ExtensionConfiguration* extension_configuration,
+    v8::Local<v8::Object> global_proxy,
+    Document* document) {
+  if (!CanCreateContextFromSnapshot(isolate, world, document)) {
+    return v8::Local<v8::Context>();
+  }
+
+  const int index = GetSnapshotIndexForWorld(world);
+  DataForDeserializer data{document};
+  v8::DeserializeInternalFieldsCallback callback =
+      v8::DeserializeInternalFieldsCallback(&DeserializeInternalField, &data);
+  v8::Local<v8::Context> context =
+      v8::Context::FromSnapshot(isolate, index, callback,
+                                extension_configuration, global_proxy)
+          .ToLocalChecked();
+  VLOG(1) << "A context is created from snapshot for "
+          << (world.IsMainWorld() ? "" : "non-") << "main world";
+
+  return context;
+}
+
+void V8SnapshotUtil::SetupContext(v8::Local<v8::Context> context,

[haraken, 2017/07/06 13:15:54]

SetupContext => InstallRuntimeEnabledFeatures

[peria, 2017/07/07 06:22:02]

Done.

+                                  Document* document) {
+  ScriptState* script_state = ScriptState::From(context);
+  v8::Isolate* isolate = script_state->GetIsolate();
+  const DOMWrapperWorld& world = script_state->World();
+  if (!CanCreateContextFromSnapshot(isolate, world, document)) {
+    return;
+  }
+
+  TRACE_EVENT1("v8", "InstallRuntimeEnabled", "IsMainFrame",

[haraken, 2017/07/06 13:15:53]

"V8SnapshotUtil::InstallRuntimeEnabledFeatures"

[peria, 2017/07/07 06:22:03]

Done.

+               world.IsMainWorld());
+
+  v8::Local<v8::String> prototype_str = V8AtomicString(isolate, "prototype");
+  V8PerContextData* data = script_state->PerContextData();
+
+  v8::Local<v8::Object> global_proxy = context->Global();
+  {
+    v8::Local<v8::Object> window_wrapper =
+        global_proxy->GetPrototype().As<v8::Object>();
+    const WrapperTypeInfo* type = &V8Window::wrapperTypeInfo;
+    v8::Local<v8::Function> interface = data->ConstructorForType(type);

[haraken, 2017/07/06 13:15:53]

interface => instance

[Yuki, 2017/07/06 14:12:28]

No, this is not a platform object (aka instance object), this is an interface
object.

Ditto for belows.

[peria, 2017/07/07 06:21:58]

+1 to Yuki.

+    v8::Local<v8::Object> prototype = interface->Get(context, prototype_str)
+                                          .ToLocalChecked()
+                                          .As<v8::Object>();
+    V8Window::install_runtime_enabled_features_function_(
+        isolate, world, window_wrapper, prototype, interface);
+  }
+  {
+    const WrapperTypeInfo* type = &V8EventTarget::wrapperTypeInfo;
+    v8::Local<v8::Function> interface = data->ConstructorForType(type);

[haraken, 2017/07/06 13:15:53]

interface => instance

[peria, 2017/07/07 06:21:58]

Acknowledged.

+    v8::Local<v8::Object> prototype = interface->Get(context, prototype_str)
+                                          .ToLocalChecked()
+                                          .As<v8::Object>();
+    V8EventTarget::InstallRuntimeEnabledFeatures(
+        isolate, world, v8::Local<v8::Object>(), prototype, interface);
+  }
+
+  if (!world.IsMainWorld()) {

[haraken, 2017/07/06 13:15:53]

Why is it okay to skip the rest of this method for non-main worlds?

[Yuki, 2017/07/06 14:12:28]

We lazily instantiate a V8 wrapper for document in isolated worlds.  We eagerly
instantiate the V8 wrapper for document only in the main world.

Peria is not going to change this situation.  The snapshot for isolated worlds
doesn't contain a document wrapper, and it gets lazily instantiated later.

[haraken, 2017/07/06 14:22:13]

Makes sense.

The logic to handle the eagerly instantiated window.document is making the code
a lot complex. Let's add a comment to the places that are handling
window.document.

+    return;
+  }
+
+  {
+    CHECK(document);
+    DCHECK(document->IsHTMLDocument());
+    CHECK(document->ContainsWrapper());
+    v8::Local<v8::Object> document_wrapper =
+        ToV8(document, global_proxy, isolate).As<v8::Object>();
+    const WrapperTypeInfo* type = &V8HTMLDocument::wrapperTypeInfo;
+    v8::Local<v8::Function> interface = data->ConstructorForType(type);

[haraken, 2017/07/06 13:15:53]

interface => instance

[peria, 2017/07/07 06:22:01]

Acknowledged.

+    v8::Local<v8::Object> prototype = interface->Get(context, prototype_str)
+                                          .ToLocalChecked()
+                                          .As<v8::Object>();
+    V8HTMLDocument::InstallRuntimeEnabledFeatures(
+        isolate, world, document_wrapper, prototype, interface);
+  }
+  {
+    const WrapperTypeInfo* type = &V8Document::wrapperTypeInfo;
+    v8::Local<v8::Function> interface = data->ConstructorForType(type);

[haraken, 2017/07/06 13:15:53]

interface => instance

[peria, 2017/07/10 03:39:12]

Acknowledged.

+    v8::Local<v8::Object> prototype = interface->Get(context, prototype_str)
+                                          .ToLocalChecked()
+                                          .As<v8::Object>();
+    V8Document::InstallRuntimeEnabledFeatures(
+        isolate, world, v8::Local<v8::Object>(), prototype, interface);
+  }
+  {
+    const WrapperTypeInfo* type = &V8Node::wrapperTypeInfo;
+    v8::Local<v8::Function> interface = data->ConstructorForType(type);

[haraken, 2017/07/06 13:15:54]

interface => instance

[peria, 2017/07/07 06:21:57]

Acknowledged.

+    v8::Local<v8::Object> prototype = interface->Get(context, prototype_str)
+                                          .ToLocalChecked()
+                                          .As<v8::Object>();
+    V8Node::InstallRuntimeEnabledFeatures(
+        isolate, world, v8::Local<v8::Object>(), prototype, interface);
+  }
+}
+
+void V8SnapshotUtil::EnsureInterfaceTemplates(v8::Isolate* isolate) {
+  if (V8PerIsolateData::From(isolate)->GetV8ContextMode() !=
+      V8PerIsolateData::V8ContextMode::kUseSnapshot) {
+    return;
+  }
+
+  SnapshotInterface& snapshot_window = g_snapshot_interfaces[0];
+  DCHECK(V8Window::wrapperTypeInfo.Equals(snapshot_window.wrapper_type_info));
+  // Update the install function for V8Window to work for partial interfaces.
+  snapshot_window.install_function =
+      V8Window::install_runtime_enabled_features_on_template_function_;
+
+  EnsureInterfaceTemplatesForWorld(isolate, DOMWrapperWorld::MainWorld());
+  EnsureInterfaceTemplatesForWorld(
+      isolate,
+      *DOMWrapperWorld::EnsureIsolatedWorld(isolate, kWorldIdForNonMainWorld));

[haraken, 2017/07/06 13:15:53]

This forcibly creates an isolated world. Not only does this increase memory
usage but also slows down a fast path of ToV8() (which assumes that there is
only one main world in common cases).

We should avoid creating an isolated world here.

[peria, 2017/07/10 03:39:12]

Done.

+}
+
+void V8SnapshotUtil::SetReferenceTable(intptr_t* table) {
+  DCHECK(!g_snapshot_reference_table);
+  g_snapshot_reference_table = table;
+}
+
+intptr_t* V8SnapshotUtil::GetReferenceTable() {
+  DCHECK(g_snapshot_reference_table);
+  return g_snapshot_reference_table;
+}
+
+v8::StartupData V8SnapshotUtil::TakeSnapshot() {
+  DCHECK_EQ(V8PerIsolateData::From(V8PerIsolateData::MainThreadIsolate())
+                ->GetV8ContextMode(),
+            V8PerIsolateData::V8ContextMode::kTakeSnapshot);
+
+  v8::SnapshotCreator* creator =
+      V8PerIsolateData::From(V8PerIsolateData::MainThreadIsolate())
+          ->GetSnapshotCreator();
+  v8::Isolate* isolate = creator->GetIsolate();
+  CHECK_EQ(isolate, v8::Isolate::GetCurrent());
+
+  VLOG(1) << "External reference table has " << CountExternalReferenceEntries()
+          << " entries.";
+
+  // Disable all runtime enabled featuers

[haraken, 2017/07/06 13:15:53]

features

[peria, 2017/07/07 06:21:58]

Done.

+  RuntimeEnabledFeatures::SetStableFeaturesEnabled(false);
+  RuntimeEnabledFeatures::SetExperimentalFeaturesEnabled(false);
+  RuntimeEnabledFeatures::SetTestFeaturesEnabled(false);
+
+  {
+    v8::HandleScope handleScope(isolate);
+    creator->SetDefaultContext(v8::Context::New(isolate));

[haraken, 2017/07/06 13:15:54]

Why do we need to create a context here? Won't it be enough to create a context
at line 458?

[Yuki, 2017/07/06 14:12:28]

This is the *default* context that is not associated with any structure such as
V8Window's global template.  I don't know details, but peria said
v8::SnapshotCreater requires the default context to be registered (as a
fallback?).

+
+    TakeSnapshotForWorld(creator, DOMWrapperWorld::MainWorld());
+    // For non main worlds, we can use any type to create a context.
+    TakeSnapshotForWorld(creator, *DOMWrapperWorld::EnsureIsolatedWorld(
+                                      isolate, kWorldIdForNonMainWorld));
+  }
+
+  // Snapshot is taken on the main thread, but it can be used on other threads.
+  // So we remove a message handler for the main thread.

[haraken, 2017/07/06 13:15:53]

Other threads don't use the snapshot, right?

[peria, 2017/07/07 06:21:56]

As for now, it is correct, but we don't have to make it a rule.

+  isolate->RemoveMessageListeners(V8Initializer::MessageHandlerInMainThread);
+
+  return creator->CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
+}
+
+v8::StartupData V8SnapshotUtil::SerializeInternalField(
+    v8::Local<v8::Object> object,
+    int index,
+    void*) {
+  InternalFieldType field_type = InternalFieldType::kNone;
+  const WrapperTypeInfo* wrapper_type = ToWrapperTypeInfo(object);
+  if (kV8DOMWrapperObjectIndex == index) {
+    if (blink::V8HTMLDocument::wrapperTypeInfo.Equals(wrapper_type)) {
+      field_type = InternalFieldType::kHTMLDocumentObject;
+    }
+  } else if (kV8DOMWrapperTypeIndex == index) {
+    if (blink::V8HTMLDocument::wrapperTypeInfo.Equals(wrapper_type)) {
+      field_type = InternalFieldType::kHTMLDocumentType;
+    } else if (blink::V8Document::wrapperTypeInfo.Equals(wrapper_type)) {
+      field_type = InternalFieldType::kDocumentType;
+    } else if (blink::V8Node::wrapperTypeInfo.Equals(wrapper_type)) {
+      field_type = InternalFieldType::kNodeType;
+    }
+  }
+  CHECK_NE(field_type, InternalFieldType::kNone);
+
+  int size = sizeof(InternalFieldType);
+  char* data = new char[size];

[haraken, 2017/07/06 13:15:52]

Who deallocates the memory?

[peria, 2017/07/07 06:22:00]

V8's snapshot serializer does.
https://cs.chromium.org/chromium/src/v8/src/snapshot/partial-serializer.cc?l=149

[haraken, 2017/07/07 06:59:11]

OK, let's add a comment.

[peria, 2017/07/10 03:39:13]

Done.

+  std::memcpy(data, &field_type, size);
+
+  return {data, size};
+}
+
+void V8SnapshotUtil::DeserializeInternalField(v8::Local<v8::Object> object,
+                                              int index,
+                                              v8::StartupData payload,
+                                              void* ptr) {
+  // DeserializeInternalField() expects to be called in the main world
+  // with |document| being HTMLDocument.
+  CHECK_EQ(payload.raw_size, static_cast<int>(sizeof(InternalFieldType)));
+  InternalFieldType type =
+      *reinterpret_cast<const InternalFieldType*>(payload.data);
+
+  const WrapperTypeInfo* wrapper_type_info = FieldTypeToWrapperTypeInfo(type);
+  switch (type) {
+    case InternalFieldType::kNodeType:
+    case InternalFieldType::kDocumentType:
+    case InternalFieldType::kHTMLDocumentType: {
+      CHECK_EQ(index, kV8DOMWrapperTypeIndex);
+      object->SetAlignedPointerInInternalField(
+          index, const_cast<WrapperTypeInfo*>(wrapper_type_info));
+      return;
+    }
+    case InternalFieldType::kHTMLDocumentObject: {
+      CHECK_EQ(index, kV8DOMWrapperObjectIndex);
+      v8::Isolate* isolate = v8::Isolate::GetCurrent();
+      DataForDeserializer* data = static_cast<DataForDeserializer*>(ptr);
+      ScriptWrappable* document = data->document;
+      DCHECK(document);
+
+      // Make reference from wrapper to document
+      object->SetAlignedPointerInInternalField(index, document);
+      // Make reference from document to wrapper
+      CHECK(document->SetWrapper(isolate, wrapper_type_info, object));
+      WrapperTypeInfo::WrapperCreated();
+      return;
+    }
+    case InternalFieldType::kNone:
+      NOTREACHED();
+      return;
+  }
+
+  NOTREACHED();
+}
+
+bool V8SnapshotUtil::CanCreateContextFromSnapshot(v8::Isolate* isolate,
+                                                  const DOMWrapperWorld& world,
+                                                  Document* document) {
+  DCHECK(document);
+  if (V8PerIsolateData::From(isolate)->GetV8ContextMode() !=
+      V8PerIsolateData::V8ContextMode::kUseSnapshot) {
+    return false;
+  }
+
+  // In case we create a context for the main world from snapshot, we also need
+  // a HTMLDocument.

[haraken, 2017/07/06 13:15:52]

Would you help me understand why we need this condition?

[Yuki, 2017/07/06 14:12:28]

We eagerly instantiates the V8 wrapper for window.document of type HTMLDocument,
however, it's possible that window.document is of type SVGDocument.  The
snapshot for the main world contains the wrapper of type HTMLDocument, and it's
NOT applicable to a SVGDocument, so we cannot use the snapshot in case of
SVGDocument.  However, the snapshot for isolated worlds does not contain any
wrapper for window.document, so we can use the snapshot in case of SVGDocument,
too.

So we need this condition: If main world => must be a HTMLDocument exactly, not
a SVGDocument or anything.

Peria may want to brush up the comment, explaining a non-applicable case with an
example (SVGDocument).

[peria, 2017/07/07 06:21:55]

Done.

[peria, 2017/07/07 06:22:00]

Acknowledged.

+  return !world.IsMainWorld() || document->IsHTMLDocument();
+}
+
+void V8SnapshotUtil::EnsureInterfaceTemplatesForWorld(
+    v8::Isolate* isolate,
+    const DOMWrapperWorld& world) {
+  V8PerIsolateData* data = V8PerIsolateData::From(isolate);
+  const int index_offset = world.IsMainWorld() ? 0 : kSnapshotInterfaceSize;
+
+  for (size_t i = 0; i < kSnapshotInterfaceSize; ++i) {
+    auto& snapshot_interface = g_snapshot_interfaces[i];
+    const WrapperTypeInfo* wrapper_type_info =
+        snapshot_interface.wrapper_type_info;
+    v8::Local<v8::FunctionTemplate> interface_template =
+        v8::FunctionTemplate::FromSnapshot(isolate, index_offset + i)

[haraken, 2017/07/06 13:15:54]

Add a comment about what index_offset+i is doing.

[peria, 2017/07/07 06:21:58]

Done.

+            .ToLocalChecked();
+    snapshot_interface.install_function(isolate, world, interface_template);
+    CHECK(!interface_template.IsEmpty());
+    data->SetInterfaceTemplate(world, wrapper_type_info, interface_template);
+  }
+}
+
+void V8SnapshotUtil::TakeSnapshotForWorld(v8::SnapshotCreator* creator,
+                                          const DOMWrapperWorld& world) {
+  v8::Isolate* isolate = creator->GetIsolate();
+  CHECK_EQ(isolate, v8::Isolate::GetCurrent());
+
+  // Function templates
+  v8::HandleScope handleScope(isolate);
+  std::array<v8::Local<v8::FunctionTemplate>, kSnapshotInterfaceSize>

[haraken, 2017/07/06 13:15:52]

Use WTF::Vector.

[Yuki, 2017/07/06 14:12:28]

IIUC, it's wrong to put a v8::Local<T> on heap.  v8::Local<T> must be put on
stack.  WTF::Vector<T> allocates content objects on heap, doesn't it?

+      interface_templates;
+  v8::Local<v8::FunctionTemplate> window_template;
+  for (size_t i = 0; i < kSnapshotInterfaceSize; ++i) {
+    const WrapperTypeInfo* wrapper_type_info =
+        g_snapshot_interfaces[i].wrapper_type_info;
+    v8::Local<v8::FunctionTemplate> interface_template =
+        wrapper_type_info->domTemplate(isolate, world);
+    CHECK(!interface_template.IsEmpty());
+    interface_templates[i] = interface_template;
+    if (V8Window::wrapperTypeInfo.Equals(wrapper_type_info)) {
+      window_template = interface_template;
+    }
+  }
+  CHECK(!window_template.IsEmpty());
+
+  v8::Local<v8::ObjectTemplate> window_instance_template =
+      window_template->InstanceTemplate();
+  CHECK(!window_instance_template.IsEmpty());
+
+  v8::Local<v8::Context> context;
+  {
+    V8PerIsolateData::UseCounterDisabledScope use_counter_disabled(

[haraken, 2017/07/06 13:15:54]

Add a comment about why we need this.

[Yuki, 2017/07/06 14:12:28]

UseCOunterDisabledScope has its own excuse on their side.  I think that we don't
want duplicates.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/bindi...

[peria, 2017/07/07 06:21:57]

+1

[peria, 2017/07/07 06:21:59]

Acknowledged.

+        V8PerIsolateData::From(isolate));
+    context = v8::Context::New(isolate, nullptr, window_instance_template);
+  }
+  CHECK(!context.IsEmpty());
+
+  if (world.IsMainWorld()) {

[haraken, 2017/07/06 13:15:53]

Add a comment about what this block is doing.

[peria, 2017/07/07 06:21:55]

Done.

+    v8::Context::Scope scope(context);
+    v8::Local<v8::Object> document_wrapper = CreatePlainWrapper(
+        isolate, world, context, &V8HTMLDocument::wrapperTypeInfo);
+    int indices[] = {kV8DOMWrapperObjectIndex, kV8DOMWrapperTypeIndex};
+    void* values[] = {nullptr, const_cast<WrapperTypeInfo*>(
+                                   &V8HTMLDocument::wrapperTypeInfo)};
+    document_wrapper->SetAlignedPointerInInternalFields(
+        WTF_ARRAY_LENGTH(indices), indices, values);
+
+    // Set the cached accessor for window.document.
+    CHECK(V8PrivateProperty::GetWindowDocumentCachedAccessor(isolate).Set(
+        context->Global(), document_wrapper));
+  }
+
+  for (auto& interface_template : interface_templates) {
+    creator->AddTemplate(interface_template);
+  }
+  creator->AddContext(context, SerializeInternalField);
+
+  V8PerIsolateData::From(isolate)->ClearPersistentsForV8Snapshot();

[haraken, 2017/07/06 13:15:53]

We should call this outside TakeSnapshotForWorld. We won't need to call this
twice.

[Yuki, 2017/07/06 14:12:28]

I don't see a benefit to move this out.

  TakeSnapshotForWorld(world1);
  CleanUp();
  TakeSnapshotForWorld(world2);
  CleanUp();
  TakeSnapshotForWorld(world3);

doesn't look great to me.

Plus, I don't like the following example.

  void foo(world1, world2) {
    TakeSnapshotForWorld(world1);
    CleanUp();
    TakeSnapshotForWorld(world2);
  }

  foo(worldA, worldB);
  foo(worldC, worldD);  // What happens!!?

Moving this out makes it structurally fragile.

[haraken, 2017/07/06 14:22:13]

I'm confused. Why do we need to call CleanUp() every time you call
TakeSnapshotForWorld()?

The snapshot is taken when you call creator->CreateBlob(), isn't it?

[Yuki, 2017/07/06 14:40:09]

My understanding is that:

a) For snapshotting, we don't use the existing v8::Template cache in
V8PerIsolateData because it's using v8::Eternal and v8::SnapshotCreater doesn't
support v8::Eternal.

b) We instead add another v8::Template cache for snapshotting with using
v8::Persistent, instead of v8::Eternal.

c) The v8::Template cache for snapshotting is *NOT* separated for main and
isolated worlds, unlike the existing cache.  It's a single instance of cache and
we re-use the same cache for both of main and isolated worlds.

Thus, the current version of the CL is doing:

  TakeSnapshotForWorld(main_world);
  // This registers v8::Templates for the main world
  // to V8PerIsolateData's cache.  As a result,
  // V8Window::domTemplate(world) returns the template
  // for the main world regardless of |world| because
  // V8PerIsolateData doesn't support world-aware cache.

  // Next, we'd like to take a snapshot for isolated worlds,
  // but V8Window::domTemplate(world) returns the template
  // for the main world...

  // Let's clear the cache out!
  ClearPersistentsForV8Snapshot();
  // Now the cache is empty.

  TakeSnapshotForWorld(isolated_world);
  // V8Window::domTemplate(world) creates a new v8::Template
  // for the given world because of no cache.  :)

[peria, 2017/07/07 06:21:58]

Yes, Yuki's understanding is correct, and c) is the critical
reason to call ClearPersistents here.

+}
+
+}  // namespace blink