[Bindings] Create and use V8 context snapshots

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

Index: third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
index 930b5bb4ce53414aa4ff352e293632965471173e..c423f15d5d01da2c969a75754baa4f5fbb86cb3e 100644
--- a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
@@ -39,6 +39,7 @@
 #include "bindings/core/v8/V8HTMLDocument.h"
 #include "bindings/core/v8/V8Initializer.h"
 #include "bindings/core/v8/V8PagePopupControllerBinding.h"
+#include "bindings/core/v8/V8SnapshotCreator.h"
 #include "bindings/core/v8/V8Window.h"
 #include "core/dom/Modulator.h"
 #include "core/frame/LocalFrame.h"
@@ -157,26 +158,110 @@ void LocalWindowProxy::Initialize() {
     SetSecurityToken(origin);
   }
 
-  MainThreadDebugger::Instance()->ContextCreated(script_state_.Get(),
-                                                 GetFrame(), origin);
-  GetFrame()->Loader().Client()->DidCreateScriptContext(context,
-                                                        world_->GetWorldId());
-  // If conditional features for window have been queued before the V8 context
-  // was ready, then inject them into the context now
-  if (world_->IsMainWorld()) {
-    InstallConditionalFeaturesOnWindow(script_state_.Get());
+  {
+    TRACE_EVENT1("v8", "notification", "isMainWindow",

[Yuki, 2017/05/10 08:58:42]

nit: I'm not clear about what "main window" is.

Would "isMainFrame" be a better name?
At least, isMainFrame is not confusing against "main-world window".

[peria, 2017/06/01 08:33:31]

Done.

+                 GetFrame()->IsMainFrame());
+    MainThreadDebugger::Instance()->ContextCreated(script_state_.Get(),
+                                                   GetFrame(), origin);
+    GetFrame()->Loader().Client()->DidCreateScriptContext(context,
+                                                          world_->GetWorldId());
+    // If conditional features for window have been queued before the V8 context
+    // was ready, then inject them into the context now
+    if (world_->IsMainWorld()) {
+      InstallConditionalFeaturesOnWindow(script_state_.Get());
+      GetFrame()->Loader().DispatchDidClearWindowObjectInMainWorld();
+    }
   }
+}
 
-  if (world_->IsMainWorld())
-    GetFrame()->Loader().DispatchDidClearWindowObjectInMainWorld();
+namespace {
+
+struct DataForDeserializer {

[Yuki, 2017/05/10 08:58:43]

You don't want to expose this kind of hack outside V8SnapshotCreater.

[peria, 2017/05/30 08:25:41]

Done.

+  STACK_ALLOCATED();
+  Member<Document> document;
+};
+
+const WrapperTypeInfo* FieldTypeToWrapperTypeInfo(

[Yuki, 2017/05/10 08:58:43]

V8SnapshotCreater should be responsible about the mapping between enum FieldType
and WrapperTypeInfo.  Why don't you define this function in V8SnapshotCreater?

[peria, 2017/05/30 08:25:41]

Done.

+    V8SnapshotCreator::FieldType type) {
+  switch (type) {
+    case V8SnapshotCreator::kNodeType:
+      return &V8Node::wrapperTypeInfo;
+    case V8SnapshotCreator::kDocumentType:
+      return &V8Document::wrapperTypeInfo;
+    case V8SnapshotCreator::kHTMLDocumentType:
+      return &V8HTMLDocument::wrapperTypeInfo;
+    case V8SnapshotCreator::kHTMLDocumentObject:
+      return &V8HTMLDocument::wrapperTypeInfo;
+    case V8SnapshotCreator::kNone:
+      NOTREACHED();
+      break;
+  }
+  NOTREACHED();
+  return nullptr;
 }
 
+void deserialize(v8::Local<v8::Object> wrapper,
+                 int index,
+                 v8::StartupData payload,
+                 void* ptr) {
+  CHECK_EQ(payload.raw_size,
+           static_cast<int>(sizeof(V8SnapshotCreator::FieldType)));
+  V8SnapshotCreator::FieldType type =
+      *reinterpret_cast<const V8SnapshotCreator::FieldType*>(payload.data);
+
+  const WrapperTypeInfo* wrapper_type_info = FieldTypeToWrapperTypeInfo(type);
+  switch (type) {
+    case V8SnapshotCreator::kNodeType:

[Yuki, 2017/05/10 08:58:42]

nit: Probably it's cleaner to use bit flags here.

kInterfaceObject = 1 << n1,
kPlatformObject = 1 << n2,

kNodeInterface = kInterfaceObject | 1 << 0,
kDocumentInterface = kInterfaceObject | 1 << 1,
...

kHTMLDocumentInstance = kPlatformObject | 1 << 0,

for example.

[peria, 2017/05/30 08:25:41]

Acknowledged.

+    case V8SnapshotCreator::kDocumentType:
+    case V8SnapshotCreator::kHTMLDocumentType: {
+      CHECK_EQ(index, kV8DOMWrapperTypeIndex);
+      wrapper->SetAlignedPointerInInternalField(
+          index, const_cast<WrapperTypeInfo*>(wrapper_type_info));

[Yuki, 2017/05/10 08:58:43]

I'm not confident, but we wouldn't need to set an internal field on an interface
object nor prototype object.

I know that we are setting internal fields on prototype objects, but are we
really setting them on interface objects, too?  If so, can we stop setting them?

I vaguely guess that we could stop setting internal fields on prototype objects,
too.

[peria, 2017/05/30 08:25:41]

Acknowledged.

+      wrapper_type_info->WrapperCreated();

[Yuki, 2017/05/10 08:58:43]

IIUC, interface objects are not considered as "wrapper objects", or at least no
need to call WrapperCreated().

Plus, WrapperCreated() is now a static member function.  No need to use
wrapper_type_info, here.

[peria, 2017/05/30 08:25:41]

Done.

+      break;
+    }
+    case V8SnapshotCreator::kHTMLDocumentObject: {
+      CHECK_EQ(index, kV8DOMWrapperObjectIndex);
+      v8::Isolate* isolate = v8::Isolate::GetCurrent();
+      DataForDeserializer* data = static_cast<DataForDeserializer*>(ptr);
+      ScriptWrappable* document = data->document;
+
+      // Make reference from wrapper to document
+      wrapper->SetAlignedPointerInInternalField(index, document);
+      // Make reference from document to wrapper
+      CHECK(document->SetWrapper(isolate, wrapper_type_info, wrapper));

[Yuki, 2017/05/10 08:58:42]

I think that you should use document->AssociateWithWrapper.
ScriptWrappable::SetWrapper only works for the main world, plus it doesn't take
care of WrapperCreated() nor SetNativeInfo().

[peria, 2017/05/30 08:25:41]

This routine is run during creating v8::Context, so it has no context nor
ScriptState to be used in AssociateWithWrapper().

+      break;
+    }
+    case V8SnapshotCreator::kNone:
+      NOTREACHED();
+      break;
+  }
+}
+
+}  // namespace
+
 void LocalWindowProxy::CreateContext() {
-  // Create a new v8::Context with the window object as the global object
-  // (aka the inner global). Reuse the outer global proxy if it already exists.
-  v8::Local<v8::ObjectTemplate> global_template =
-      V8Window::domTemplate(GetIsolate(), *world_)->InstanceTemplate();
-  CHECK(!global_template.IsEmpty());
+  TRACE_EVENT1("v8", "LocalWindowProxy::CreateContext", "isMainWindow",
+               GetFrame()->IsMainFrame());
+
+  if (V8PerIsolateData::From(GetIsolate())->UseSnapshot()) {

[Yuki, 2017/05/10 08:58:43]

This kind of code should live hidden in V8SnapshotCreater, and CreateContext()'d
better call an API of V8SnapshotCreater.

[peria, 2017/05/30 08:25:41]

Done.
This part should be run once per context.
I put a TODO comment to do it.

+    // To store function templates into V8PerIsolateData.
+    v8::Local<v8::FunctionTemplate> eventTargetFunctionTemplate =
+        V8EventTarget::domTemplate(GetIsolate(), World());
+    CHECK(!eventTargetFunctionTemplate.IsEmpty());
+    v8::Local<v8::FunctionTemplate> windowFunctionTemplate =
+        V8Window::domTemplate(GetIsolate(), World());
+    CHECK(!windowFunctionTemplate.IsEmpty());
+    v8::Local<v8::FunctionTemplate> nodeFunctionTemplate =
+        V8Node::domTemplate(GetIsolate(), World());
+    CHECK(!nodeFunctionTemplate.IsEmpty());
+    v8::Local<v8::FunctionTemplate> documentFunctionTemplate =
+        V8Document::domTemplate(GetIsolate(), World());
+    CHECK(!documentFunctionTemplate.IsEmpty());
+    v8::Local<v8::FunctionTemplate> htmlDocumentFunctionTemplate =
+        V8HTMLDocument::domTemplate(GetIsolate(), World());
+    CHECK(!htmlDocumentFunctionTemplate.IsEmpty());
+  }
 
   Vector<const char*> extension_names;
   // Dynamically tell v8 about our extensions now.
@@ -191,11 +276,33 @@ void LocalWindowProxy::CreateContext() {
 
   v8::Local<v8::Context> context;
   {
+    V8PerIsolateData* per_isolate_data = V8PerIsolateData::From(GetIsolate());
     V8PerIsolateData::UseCounterDisabledScope use_counter_disabled(
         V8PerIsolateData::From(GetIsolate()));
-    context =
-        v8::Context::New(GetIsolate(), &extension_configuration,
-                         global_template, global_proxy_.NewLocal(GetIsolate()));
+    TRACE_EVENT1("v8", "contextCreation", "isMainWindow",
+                 GetFrame()->IsMainFrame());
+
+    v8::Local<v8::Object> global_proxy = global_proxy_.NewLocal(GetIsolate());
+    Document* document = GetFrame()->GetDocument();
+
+    if (per_isolate_data->UseSnapshot() &&
+        (!World().IsMainWorld() || (document && document->IsHTMLDocument()))) {

[Yuki, 2017/05/10 08:58:42]

I don't understand these conditions.

I thought that we always have |document|.  In what cases don't we have
|document|?

Also, why the world matters?

[peria, 2017/05/30 08:25:41]

First of all, we need at least two types of contexts in a snapshot;
one is for main world, and the other is for non-main world.
They can have some different callback functions.

And in practice, we assume to have a HTMLDocument wrapper in the context for
main world.

+      const int index = World().IsMainWorld() ? 0 : 1;

[Yuki, 2017/05/10 08:58:43]

Where these magic numbers 0 and 1 come from?

You may not want to expose these magic numbers outside V8SnapshotCreater.

[peria, 2017/05/30 08:25:41]

Done.

+      DataForDeserializer data{document};
+      CHECK(v8::Context::FromSnapshot(
+                GetIsolate(), index,
+                v8::DeserializeInternalFieldsCallback(&deserialize, &data),
+                &extension_configuration, global_proxy)
+                .ToLocal(&context));
+    }
+
+    if (context.IsEmpty()) {

[Yuki, 2017/05/10 08:58:43]

Should this simply be
  } else {
?

It seems that you're not expecting any failure of Context::FromSnapshot.

[peria, 2017/05/30 08:25:41]

Done.

+      v8::Local<v8::ObjectTemplate> global_template =
+          V8Window::domTemplate(GetIsolate(), *world_)->InstanceTemplate();
+      CHECK(!global_template.IsEmpty());
+      context = v8::Context::New(GetIsolate(), &extension_configuration,
+                                 global_template, global_proxy);
+    }
   }
   CHECK(!context.IsEmpty());
 
@@ -212,6 +319,9 @@ void LocalWindowProxy::CreateContext() {
 }
 
 void LocalWindowProxy::SetupWindowPrototypeChain() {
+  TRACE_EVENT1("v8", "LocalWindowProxy::setupWindowPrototypeChain",

[Yuki, 2017/05/10 08:58:43]

nit: s/setup/Setup/

[peria, 2017/05/30 08:25:41]

Done.

+               "isMainWindow", GetFrame()->IsMainFrame());
+
   // Associate the window wrapper object and its prototype chain with the
   // corresponding native DOMWindow object.
   DOMWindow* window = GetFrame()->DomWindow();
@@ -248,6 +358,14 @@ void LocalWindowProxy::SetupWindowPrototypeChain() {
   V8DOMWrapper::SetNativeInfo(GetIsolate(), window_properties,
                               wrapper_type_info, window);
 
+  if (V8PerIsolateData::From(GetIsolate())->UseSnapshot()) {
+    v8::Local<v8::Function> window_interface =
+        V8Window::domTemplate(GetIsolate(), World())->GetFunction();
+    V8Window::installV8WindowRuntimeEnabledFunction(
+        GetIsolate(), World(), window_wrapper, window_prototype,
+        window_interface);
+  }
+
   // TODO(keishi): Remove installPagePopupController and implement
   // PagePopupController in another way.
   V8PagePopupControllerBinding::InstallPagePopupController(context,
@@ -256,12 +374,29 @@ void LocalWindowProxy::SetupWindowPrototypeChain() {
 
 void LocalWindowProxy::UpdateDocumentProperty() {
   DCHECK(world_->IsMainWorld());
+  TRACE_EVENT1("v8", "LocalWindowProxy::updateDocumentProperty", "isMainWindow",

[Yuki, 2017/05/10 08:58:43]

nit: s/update/Update/

[peria, 2017/05/30 08:25:41]

Done.

+               GetFrame()->IsMainFrame());
 
   ScriptState::Scope scope(script_state_.Get());
   v8::Local<v8::Context> context = script_state_->GetContext();
   v8::Local<v8::Value> document_wrapper =
       ToV8(GetFrame()->GetDocument(), context->Global(), GetIsolate());
   DCHECK(document_wrapper->IsObject());
+
+  if (V8PerIsolateData::From(GetIsolate())->UseSnapshot()) {
+    v8::Local<v8::Object> wrapper = document_wrapper.As<v8::Object>();

[Yuki, 2017/05/10 08:58:42]

s/wrapper/htmldocument_wrapper/
and make it clear that you're following
  platform object of type HTMLDocument
  => HTMLDocument.prototype
  => Document.prototype

Use of htmldocument_prototype would be good.

[peria, 2017/05/30 08:25:41]

Done.

+    v8::Local<v8::Object> document_prototype = wrapper->GetPrototype()
+                                                   .As<v8::Object>()
+                                                   ->GetPrototype()
+                                                   .As<v8::Object>();
+    V8Document::installRuntimeEnabledFeatures(GetIsolate(), World(), wrapper,

[Yuki, 2017/05/10 08:58:43]

Although it's noop, we do have
{V8HTMLDocument,V8Node,...}::installRuntimeEnabledFeatures.  You should call
them, too, or it will be fragile or surprising in future.

Also, you'd like to hide all this kind of things in V8SnapshotCreater.  My
understanding is that V8SnapshotCreater is responsible to take a snapshot of
what things, which interface objects and which platform objects. 
LocalWindowProxy must not need to know which interface or platform objects are
taken into a snapshot.

[peria, 2017/05/30 08:25:41]

Done.

+                                              document_prototype,
+                                              v8::Local<v8::Function>());
+    V8Document::preparePrototypeAndInterfaceObject(
+        context, World(), document_prototype, v8::Local<v8::Function>(),
+        v8::Local<v8::FunctionTemplate>());
+  }
+
   // Update the cached accessor for window.document.
   CHECK(V8PrivateProperty::GetWindowDocumentCachedAccessor(GetIsolate())
             .Set(context->Global(), document_wrapper));
@@ -305,8 +440,8 @@ void LocalWindowProxy::SetSecurityToken(SecurityOrigin* origin) {
     String frame_security_token = frame_security_origin->ToString();
     // We need to check the return value of domainWasSetInDOM() on the
     // frame's SecurityOrigin because, if that's the case, only
-    // SecurityOrigin::m_domain would have been modified.
-    // m_domain is not used by SecurityOrigin::toString(), so we would end
+    // SecurityOrigin::domain_ would have been modified.
+    // domain_ is not used by SecurityOrigin::toString(), so we would end
     // up generating the same token that was already set.
     if (frame_security_origin->DomainWasSetInDOM() ||
         frame_security_token.IsEmpty() || frame_security_token == "null") {