[Bindings] Create and use V8 context snapshots

third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h

Index: third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h
diff --git a/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h b/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h
index a320871ad5fc8b105ac78ca9c743b29e5f60eb2b..b18e278109425c992cf12e2b1b593950d54f28d4 100644
--- a/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h
+++ b/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h
@@ -100,7 +100,7 @@ class PLATFORM_EXPORT V8PerIsolateData {
     virtual ~Data() = default;
   };
 
-  static v8::Isolate* Initialize(WebTaskRunner*);
+  static v8::Isolate* Initialize(WebTaskRunner*, intptr_t*, bool);

[Yuki, 2017/05/12 15:20:10]

Write argument names when they're not trivial.
enum is preferred to bool.

If V8SnapshotCreator::GetExternalReferences() is the standard way to get the
external references, probably we'd like to use it directly without taking it as
an argument.

Ideally, V8SnapshotCreator well encapsulate things and there should be no need
for client code to refer to the external references, i.e. the external
references are preferred being a private member of V8SnapshotCreator.

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

V8SnapshotCreator is under bindings/core and is not referable from
platform/bindings.

 
   static V8PerIsolateData* From(v8::Isolate* isolate) {
     DCHECK(isolate);
@@ -117,6 +117,9 @@ class PLATFORM_EXPORT V8PerIsolateData {
                               std::unique_ptr<gin::V8IdleTaskRunner>);
 
   v8::Isolate* GetIsolate() { return isolate_holder_.isolate(); }
+  v8::SnapshotCreator* GetSnapshotCreator() {
+    return isolate_holder_.snapshot_creator();
+  }
 
   StringCache* GetStringCache() { return string_cache_.get(); }
 
@@ -139,9 +142,16 @@ class PLATFORM_EXPORT V8PerIsolateData {
   // Accessors to the cache of interface templates.
   v8::Local<v8::FunctionTemplate> FindInterfaceTemplate(const DOMWrapperWorld&,
                                                         const void* key);
+  v8::Local<v8::FunctionTemplate> FindInterfaceTemplateTemp(
+      const DOMWrapperWorld&,
+      const void* key);
   void SetInterfaceTemplate(const DOMWrapperWorld&,
                             const void* key,
                             v8::Local<v8::FunctionTemplate>);
+  void SetInterfaceTemplateTemp(const DOMWrapperWorld&,
+                                const void* key,
+                                v8::Local<v8::FunctionTemplate>);
+  void ClearAll();

[Yuki, 2017/05/15 09:37:29]

This function does not actually clear *all*, right?
For example, we don't clear string_cache_ (and no need).  Let's rename the
function name so that it's clear that a) this is designed to be used for
snapshotting, and b) what actually this does.

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

Done.

 
   // Accessor to the cache of cross-origin accessible operation's templates.
   // Created templates get automatically cached.
@@ -222,10 +232,21 @@ class PLATFORM_EXPORT V8PerIsolateData {
     return script_wrappable_visitor_.get();
   }
 
+  bool UseSnapshot() const {

[Yuki, 2017/05/12 15:20:10]

Probably ShouldUseSnapshot()?

This function is a predictor, so names like DoSomething are not good. 
Predictors are expected to be named like IsSomething, ShouldDoSomething, etc.

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

Acknowledged.

+    return isolate_holder_.v8_context_mode() ==
+           gin::IsolateHolder::kUseSnapshot;
+  }
+
  private:
-  explicit V8PerIsolateData(WebTaskRunner*);
+  explicit V8PerIsolateData(WebTaskRunner*, intptr_t*, bool);

[Yuki, 2017/05/12 15:20:10]

s/explicit//

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

Done.

   ~V8PerIsolateData();
 
+  using CopyableTraits = v8::CopyablePersistentTraits<v8::FunctionTemplate>;

[Yuki, 2017/05/12 15:20:10]

Could you let me know why we need CopyablePersistentTraits?

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

This is abused to follow V8FunctionTemplateMap.
I'll change this with ScopedPersistent or something like it.

+  using CopyablePersistent =
+      v8::Persistent<v8::FunctionTemplate, CopyableTraits>;

[Yuki, 2017/05/12 15:20:10]

If these types are only used at the following line, I'd prefer not aliasing
because it's not clear what V8PerIsolateData::CopyableTraits is.  It's not clear
that it's related to V8 snapshotting.

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

Acknowledged.

+  using V8TemporaryFunctionTemplateMap =

[Yuki, 2017/05/12 15:20:10]

"Temporary" doesn't make much sense, or even misleading.

V8FunctionTemplateMapForV8ContextSnapshot for example?

Maybe better to define this type after V8FunctionTemplateMap because most people
read code from top to down.

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

Done.

+      HashMap<const void*, CopyablePersistent>;
+
   typedef HashMap<const void*, v8::Eternal<v8::FunctionTemplate>>
       V8FunctionTemplateMap;
   V8FunctionTemplateMap& SelectInterfaceTemplateMap(const DOMWrapperWorld&);
@@ -239,10 +260,13 @@ class PLATFORM_EXPORT V8PerIsolateData {
 
   gin::IsolateHolder isolate_holder_;
 
-  // m_interfaceTemplateMapFor{,Non}MainWorld holds function templates for
+  // interface_template_map_for_{,non_}main_world holds function templates for
   // the inerface objects.
   V8FunctionTemplateMap interface_template_map_for_main_world_;
   V8FunctionTemplateMap interface_template_map_for_non_main_world_;
+
+  V8TemporaryFunctionTemplateMap interface_template_temp_map_;
+
   // m_operationTemplateMapFor{,Non}MainWorld holds function templates for
   // the cross-origin accessible DOM operations.
   V8FunctionTemplateMap operation_template_map_for_main_world_;