[snapshot] revisit snapshot API.

This part of the snapshot API should not be in use yet, so we can still
change this. The motivation for this change is:
- Use MaybeHandle where reasonable.
- Remove ambiguity: when we use index to create context from snapshot,
  we should not have a silent fallback if snapshot is not available.
- Symmetry: rename to Context::FromSnapshot to mirror templates.

R=jochen@chromium.org
BUG=chromium:617892

Review-Url: https://codereview.chromium.org/2100073002
Cr-Commit-Position: refs/heads/master@{#37334}

include/v8.h

@@ -4490,7 +4490,8 @@ class V8_EXPORT FunctionTemplate : public Template {
       Local<Signature> signature = Local<Signature>(), int length = 0);
 
   /** Get a template included in the snapshot by index. */
-  static Local<FunctionTemplate> FromSnapshot(Isolate* isolate, size_t index);
+  static MaybeLocal<FunctionTemplate> FromSnapshot(Isolate* isolate,
+                                                   size_t index);
 
   /**
    * Creates a function template with a fast handler. If a fast handler is set,
@@ -4668,7 +4669,8 @@ class V8_EXPORT ObjectTemplate : public Template {
   static V8_DEPRECATED("Use isolate version", Local<ObjectTemplate> New());
 
   /** Get a template included in the snapshot by index. */
-  static Local<ObjectTemplate> FromSnapshot(Isolate* isolate, size_t index);
+  static MaybeLocal<ObjectTemplate> FromSnapshot(Isolate* isolate,
+                                                 size_t index);
 
   /** Creates a new instance of this template.*/
   V8_DEPRECATE_SOON("Use maybe version", Local<Object> NewInstance());
@@ -7141,8 +7143,13 @@ class V8_EXPORT Context {
   static Local<Context> New(
       Isolate* isolate, ExtensionConfiguration* extensions = NULL,
       Local<ObjectTemplate> global_template = Local<ObjectTemplate>(),
-      Local<Value> global_object = Local<Value>(),
-      size_t context_snapshot_index = 0);
+      Local<Value> global_object = Local<Value>());
+
+  static MaybeLocal<Context> FromSnapshot(
+      Isolate* isolate, size_t context_snapshot_index,
+      ExtensionConfiguration* extensions = NULL,
+      Local<ObjectTemplate> global_template = Local<ObjectTemplate>(),
+      Local<Value> global_object = Local<Value>());
 
   /**
    * Sets the security token for the context.  To access an object in

src/api.cc

@@ -1200,13 +1200,13 @@ Local<FunctionTemplate> FunctionTemplate::New(Isolate* isolate,
                              length, false);
 }
 
-Local<FunctionTemplate> FunctionTemplate::FromSnapshot(Isolate* isolate,
+MaybeLocal<FunctionTemplate> FunctionTemplate::FromSnapshot(Isolate* isolate,
                                                             size_t index) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::FixedArray* templates = i_isolate->heap()->serialized_templates();
   int int_index = static_cast<int>(index);
   if (int_index < templates->length()) {
-    i::Object* info = i_isolate->heap()->serialized_templates()->get(int_index);
+    i::Object* info = templates->get(int_index);
     if (info->IsFunctionTemplateInfo()) {
       return Utils::ToLocal(i::Handle<i::FunctionTemplateInfo>(
           i::FunctionTemplateInfo::cast(info)));
@@ -1425,13 +1425,13 @@ Local<ObjectTemplate> ObjectTemplate::New(
   return ObjectTemplateNew(isolate, constructor, false);
 }
 
-Local<ObjectTemplate> ObjectTemplate::FromSnapshot(Isolate* isolate,
+MaybeLocal<ObjectTemplate> ObjectTemplate::FromSnapshot(Isolate* isolate,
                                                         size_t index) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::FixedArray* templates = i_isolate->heap()->serialized_templates();
   int int_index = static_cast<int>(index);
   if (int_index < templates->length()) {
-    i::Object* info = i_isolate->heap()->serialized_templates()->get(int_index);
+    i::Object* info = templates->get(int_index);
     if (info->IsObjectTemplateInfo()) {
       return Utils::ToLocal(
           i::Handle<i::ObjectTemplateInfo>(i::ObjectTemplateInfo::cast(info)));
@@ -5684,7 +5684,7 @@ static i::Handle<i::Context> CreateEnvironment(
   return env;
 }
 
-Local<Context> v8::Context::New(v8::Isolate* external_isolate,
+Local<Context> NewContext(v8::Isolate* external_isolate,
                           v8::ExtensionConfiguration* extensions,
                           v8::Local<ObjectTemplate> global_template,
                           v8::Local<Value> global_object,
@@ -5706,6 +5706,26 @@ Local<Context> v8::Context::New(v8::Isolate* external_isolate,
   return Utils::ToLocal(scope.CloseAndEscape(env));
 }
 
+Local<Context> v8::Context::New(v8::Isolate* external_isolate,
+                                v8::ExtensionConfiguration* extensions,
+                                v8::Local<ObjectTemplate> global_template,
+                                v8::Local<Value> global_object) {
+  return NewContext(external_isolate, extensions, global_template,
+                    global_object, 0);
+}
+
+MaybeLocal<Context> v8::Context::FromSnapshot(
+    v8::Isolate* external_isolate, size_t context_snapshot_index,
+    v8::ExtensionConfiguration* extensions,
+    v8::Local<ObjectTemplate> global_template, v8::Local<Value> global_object) {
+  if (!i::Snapshot::HasContextSnapshot(
+          reinterpret_cast<i::Isolate*>(external_isolate),
+          context_snapshot_index)) {
+    return MaybeLocal<Context>();
+  }
+  return NewContext(external_isolate, extensions, global_template,
+                    global_object, context_snapshot_index);
+}
 
 void v8::Context::SetSecurityToken(Local<Value> token) {
   i::Handle<i::Context> env = Utils::OpenHandle(this);

src/snapshot/snapshot-common.cc

@@ -22,11 +22,13 @@ bool Snapshot::SnapshotIsValid(v8::StartupData* snapshot_blob) {
 }
 #endif  // DEBUG
 
-
-bool Snapshot::HaveASnapshotToStartFrom(Isolate* isolate) {
+bool Snapshot::HasContextSnapshot(Isolate* isolate, size_t index) {
   // Do not use snapshots if the isolate is used to create snapshots.
-  return isolate->snapshot_blob() != NULL &&
-         isolate->snapshot_blob()->data != NULL;
+  const v8::StartupData* blob = isolate->snapshot_blob();
+  if (blob == nullptr) return false;
+  if (blob->data == nullptr) return false;
+  size_t num_contexts = static_cast<size_t>(ExtractNumContexts(blob));
+  return index < num_contexts;
 }
 
 

src/snapshot/snapshot.h

@@ -63,6 +63,8 @@ class Snapshot : public AllStatic {
 
   static bool HaveASnapshotToStartFrom(Isolate* isolate);
 
+  static bool HasContextSnapshot(Isolate* isolate, size_t index);
+
   static bool EmbedsScript(Isolate* isolate);
 
   static uint32_t SizeOfFirstPage(Isolate* isolate, AllocationSpace space);

test/cctest/heap/test-heap.cc

@@ -4890,7 +4890,7 @@ TEST(NoWeakHashTableLeakWithIncrementalMarking) {
   Isolate* isolate = CcTest::i_isolate();
 
   // Do not run for no-snap builds.
-  if (!i::Snapshot::HaveASnapshotToStartFrom(isolate)) return;
+  if (!i::Snapshot::HasContextSnapshot(isolate, 0)) return;
 
   v8::internal::Heap* heap = CcTest::heap();
 
@@ -6029,7 +6029,7 @@ TEST(BootstrappingExports) {
   v8::Isolate* isolate = CcTest::isolate();
   LocalContext env;
 
-  if (Snapshot::HaveASnapshotToStartFrom(CcTest::i_isolate())) return;
+  if (Snapshot::HasContextSnapshot(CcTest::i_isolate(), 0)) return;
 
   utils_has_been_collected = false;
 

test/cctest/test-serialize.cc

@@ -1921,27 +1921,24 @@ TEST(SnapshotCreatorMultipleContexts) {
   v8::Isolate* isolate = v8::Isolate::New(params);
   {
     v8::Isolate::Scope isolate_scope(isolate);
-    v8::ExtensionConfiguration* no_extension = nullptr;
-    v8::Local<v8::ObjectTemplate> no_template = v8::Local<v8::ObjectTemplate>();
-    v8::Local<v8::Value> no_object = v8::Local<v8::Value>();
     {
       v8::HandleScope handle_scope(isolate);
       v8::Local<v8::Context> context =
-          v8::Context::New(isolate, no_extension, no_template, no_object, 0);
+          v8::Context::FromSnapshot(isolate, 0).ToLocalChecked();
       v8::Context::Scope context_scope(context);
       ExpectInt32("f()", 1);
     }
     {
       v8::HandleScope handle_scope(isolate);
       v8::Local<v8::Context> context =
-          v8::Context::New(isolate, no_extension, no_template, no_object, 1);
+          v8::Context::FromSnapshot(isolate, 1).ToLocalChecked();
       v8::Context::Scope context_scope(context);
       ExpectInt32("f()", 2);
     }
     {
       v8::HandleScope handle_scope(isolate);
       v8::Local<v8::Context> context =
-          v8::Context::New(isolate, no_extension, no_template, no_object, 2);
+          v8::Context::FromSnapshot(isolate, 2).ToLocalChecked();
       v8::Context::Scope context_scope(context);
       ExpectUndefined("this.f");
     }
@@ -1999,12 +1996,8 @@ TEST(SnapshotCreatorExternalReferences) {
     {
       v8::Isolate::Scope isolate_scope(isolate);
       v8::HandleScope handle_scope(isolate);
-      v8::ExtensionConfiguration* no_extension = nullptr;
-      v8::Local<v8::ObjectTemplate> no_template =
-          v8::Local<v8::ObjectTemplate>();
-      v8::Local<v8::Value> no_object = v8::Local<v8::Value>();
       v8::Local<v8::Context> context =
-          v8::Context::New(isolate, no_extension, no_template, no_object, 0);
+          v8::Context::FromSnapshot(isolate, 0).ToLocalChecked();
       v8::Context::Scope context_scope(context);
       ExpectInt32("f()", 42);
     }
@@ -2021,12 +2014,8 @@ TEST(SnapshotCreatorExternalReferences) {
     {
       v8::Isolate::Scope isolate_scope(isolate);
       v8::HandleScope handle_scope(isolate);
-      v8::ExtensionConfiguration* no_extension = nullptr;
-      v8::Local<v8::ObjectTemplate> no_template =
-          v8::Local<v8::ObjectTemplate>();
-      v8::Local<v8::Value> no_object = v8::Local<v8::Value>();
       v8::Local<v8::Context> context =
-          v8::Context::New(isolate, no_extension, no_template, no_object, 0);
+          v8::Context::FromSnapshot(isolate, 0).ToLocalChecked();
       v8::Context::Scope context_scope(context);
       ExpectInt32("f()", 1337);
     }
@@ -2072,18 +2061,14 @@ TEST(SnapshotCreatorTemplates) {
       {
         // Create a new context without a new object template.
         v8::HandleScope handle_scope(isolate);
-        v8::ExtensionConfiguration* no_extension = nullptr;
-        v8::Local<v8::ObjectTemplate> no_template =
-            v8::Local<v8::ObjectTemplate>();
-        v8::Local<v8::Value> no_object = v8::Local<v8::Value>();
         v8::Local<v8::Context> context =
-            v8::Context::New(isolate, no_extension, no_template, no_object, 0);
+            v8::Context::FromSnapshot(isolate, 0).ToLocalChecked();
         v8::Context::Scope context_scope(context);
         ExpectInt32("f()", 42);
 
         // Retrieve the snapshotted object template.
         v8::Local<v8::ObjectTemplate> obj_template =
-            v8::ObjectTemplate::FromSnapshot(isolate, 1);
+            v8::ObjectTemplate::FromSnapshot(isolate, 1).ToLocalChecked();
         CHECK(!obj_template.IsEmpty());
         v8::Local<v8::Object> object =
             obj_template->NewInstance(context).ToLocalChecked();
@@ -2094,7 +2079,7 @@ TEST(SnapshotCreatorTemplates) {
 
         // Retrieve the snapshotted function template.
         v8::Local<v8::FunctionTemplate> fun_template =
-            v8::FunctionTemplate::FromSnapshot(isolate, 0);
+            v8::FunctionTemplate::FromSnapshot(isolate, 0).ToLocalChecked();
         CHECK(!fun_template.IsEmpty());
         v8::Local<v8::Function> fun =
             fun_template->GetFunction(context).ToLocalChecked();
@@ -2102,6 +2087,11 @@ TEST(SnapshotCreatorTemplates) {
         ExpectInt32("g()", 42);
         // Check that it instantiates to the same prototype.
         ExpectTrue("g.prototype === f.prototype");
+
+        // Accessing out of bound returns empty MaybeHandle.
+        CHECK(v8::ObjectTemplate::FromSnapshot(isolate, 2).IsEmpty());
+        CHECK(v8::FunctionTemplate::FromSnapshot(isolate, 2).IsEmpty());
+        CHECK(v8::Context::FromSnapshot(isolate, 2).IsEmpty());
       }
 
       {
@@ -2114,9 +2104,9 @@ TEST(SnapshotCreatorTemplates) {
         global_template->Set(
             v8_str("g"),
             v8::FunctionTemplate::New(isolate, SerializedCallbackReplacement));
-        v8::Local<v8::Value> no_object = v8::Local<v8::Value>();
-        v8::Local<v8::Context> context = v8::Context::New(
-            isolate, no_extension, global_template, no_object, 0);
+        v8::Local<v8::Context> context =
+            v8::Context::FromSnapshot(isolate, 0, no_extension, global_template)
+                .ToLocalChecked();
         v8::Context::Scope context_scope(context);
         ExpectInt32("g()", 1337);
         ExpectInt32("f()", 42);