extensions: Use V8 Maybe APIs in ModuleSystem

extensions/renderer/module_system.cc

 // Copyright 2014 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 "extensions/renderer/module_system.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extensions_client.h"
 #include "extensions/renderer/console.h"
 #include "extensions/renderer/safe_builtins.h"
 #include "extensions/renderer/script_context.h"
+#include "extensions/renderer/v8_maybe_helpers.h"
 #include "gin/modules/module_registry.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
 
 namespace extensions {
 
 namespace {
 
 const char* kModuleSystem = "module_system";
 const char* kModuleName = "module_name";
@@ skipping 31 matching lines @@
 }
 
 void Warn(v8::Isolate* isolate, const std::string& message) {
   console::Warn(isolate->GetCallingContext(), message);
 }
 
 // Default exception handler which logs the exception.
 class DefaultExceptionHandler : public ModuleSystem::ExceptionHandler {
  public:
   explicit DefaultExceptionHandler(ScriptContext* context)
-      : context_(context) {}
+      : ModuleSystem::ExceptionHandler(context) {}
 
   // Fatally dumps the debug info from |try_catch| to the console.
   // Make sure this is never used for exceptions that originate in external
   // code!
   void HandleUncaughtException(const v8::TryCatch& try_catch) override {
     v8::HandleScope handle_scope(context_->isolate());
     std::string stack_trace = "<stack trace unavailable>";
-    if (!try_catch.StackTrace().IsEmpty()) {
-      v8::String::Utf8Value stack_value(try_catch.StackTrace());
+    v8::Local<v8::Value> v8_stack_trace;
+    if (try_catch.StackTrace(context_->v8_context()).ToLocal(&v8_stack_trace)) {
+      v8::String::Utf8Value stack_value(v8_stack_trace);
       if (*stack_value)
         stack_trace.assign(*stack_value, stack_value.length());
       else
         stack_trace = "<could not convert stack trace to string>";
     }
-    Fatal(context_, CreateExceptionString(try_catch) + "{" + stack_trace + "}");
+    Fatal(context_,
+          CreateExceptionString(try_catch) + "{" + stack_trace + "}");
   }
 
  private:
   ScriptContext* context_;
 };
 
 }  // namespace
 
 std::string ModuleSystem::ExceptionHandler::CreateExceptionString(
     const v8::TryCatch& try_catch) {
   v8::Local<v8::Message> message(try_catch.Message());
   if (message.IsEmpty()) {
     return "try_catch has no message";
   }
 
   std::string resource_name = "<unknown resource>";
   if (!message->GetScriptOrigin().ResourceName().IsEmpty()) {
     v8::String::Utf8Value resource_name_v8(
         message->GetScriptOrigin().ResourceName());
     resource_name.assign(*resource_name_v8, resource_name_v8.length());
   }
 
   std::string error_message = "<no error message>";
   if (!message->Get().IsEmpty()) {
     v8::String::Utf8Value error_message_v8(message->Get());
     error_message.assign(*error_message_v8, error_message_v8.length());
   }
 
+  auto maybe = message->GetLineNumber(context_->v8_context());
+  int line_number = maybe.IsJust() ? maybe.FromJust() : 0;
   return base::StringPrintf("%s:%d: %s",
                             resource_name.c_str(),
-                            message->GetLineNumber(),
+                            line_number,
                             error_message.c_str());
 }
 
 ModuleSystem::ModuleSystem(ScriptContext* context, SourceMap* source_map)
     : ObjectBackedNativeHandler(context),
       context_(context),
       source_map_(source_map),
       natives_enabled_(0),
       exception_handler_(new DefaultExceptionHandler(context)),
       weak_factory_(this) {
   RouteFunction(
       "require",
       base::Bind(&ModuleSystem::RequireForJs, base::Unretained(this)));
   RouteFunction(
       "requireNative",
       base::Bind(&ModuleSystem::RequireNative, base::Unretained(this)));
   RouteFunction(
       "requireAsync",
       base::Bind(&ModuleSystem::RequireAsync, base::Unretained(this)));
   RouteFunction("privates",
                 base::Bind(&ModuleSystem::Private, base::Unretained(this)));
 
   v8::Local<v8::Object> global(context->v8_context()->Global());
   v8::Isolate* isolate = context->isolate();
-  global->SetHiddenValue(v8::String::NewFromUtf8(isolate, kModulesField),
+  global->SetHiddenValue(ToV8String(isolate, kModulesField),
                          v8::Object::New(isolate));
-  global->SetHiddenValue(v8::String::NewFromUtf8(isolate, kModuleSystem),
+  global->SetHiddenValue(ToV8String(isolate, kModuleSystem),
                          v8::External::New(isolate, this));
 
   gin::ModuleRegistry::From(context->v8_context())->AddObserver(this);
   if (context_->GetRenderFrame()) {
     context_->GetRenderFrame()->EnsureMojoBuiltinsAreAvailable(
         context->isolate(), context->v8_context());
   }
 }
 
 ModuleSystem::~ModuleSystem() {
 }
 
 void ModuleSystem::Invalidate() {
   // Clear the module system properties from the global context. It's polite,
   // and we use this as a signal in lazy handlers that we no longer exist.
   {
     v8::HandleScope scope(GetIsolate());
     v8::Local<v8::Object> global = context()->v8_context()->Global();
-    global->DeleteHiddenValue(
-        v8::String::NewFromUtf8(GetIsolate(), kModulesField));
-    global->DeleteHiddenValue(
-        v8::String::NewFromUtf8(GetIsolate(), kModuleSystem));
+    global->DeleteHiddenValue(ToV8String(GetIsolate(), kModulesField));
+    global->DeleteHiddenValue(ToV8String(GetIsolate(), kModuleSystem));
   }
 
   // Invalidate all active and clobbered NativeHandlers we own.
   for (const auto& handler : native_handler_map_)
     handler.second->Invalidate();
   for (const auto& clobbered_handler : clobbered_native_handlers_)
     clobbered_handler->Invalidate();
 
   ObjectBackedNativeHandler::Invalidate();
 }
 
 ModuleSystem::NativesEnabledScope::NativesEnabledScope(
     ModuleSystem* module_system)
     : module_system_(module_system) {
   module_system_->natives_enabled_++;
 }
 
 ModuleSystem::NativesEnabledScope::~NativesEnabledScope() {
   module_system_->natives_enabled_--;
   CHECK_GE(module_system_->natives_enabled_, 0);
 }
 
 void ModuleSystem::HandleException(const v8::TryCatch& try_catch) {
   exception_handler_->HandleUncaughtException(try_catch);
 }
 
 v8::Local<v8::Value> ModuleSystem::Require(const std::string& module_name) {
+  if (module_name.size() >= v8::String::kMaxLength)
+    return v8::Undefined(GetIsolate());
   v8::EscapableHandleScope handle_scope(GetIsolate());
   return handle_scope.Escape(RequireForJsInner(
-      v8::String::NewFromUtf8(GetIsolate(), module_name.c_str())));
+      ToV8String(GetIsolate(), module_name.c_str())));
 }
 
 void ModuleSystem::RequireForJs(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::Local<v8::String> module_name = args[0]->ToString(args.GetIsolate());
+  if (!args[0]->IsString())
+    return;

[not at google - send to devlin, 2015/06/12 16:51:36]

If this isn't a string it means that JS called require(something) where
something isn't a string. That's almost certainly a Chrome programmer error, so
I'd rather that not be a silent failure. It would be nice to throw a JS
exception then (or just LOG(FATAL); they amount to the the same thing) but there
exists a bug where a third party did manage to call require(), so perhaps that
isn't wise.

Well, the worst they can do is crash themselves, I suppose. Something to think
about.

How about:

if (!args[0]->IsString()) {
  NOTREACHED() << "require() called with a non-string argument";
  return;
}

to at least catch Chrome programmer error to some degree.

[bashi, 2015/06/16 03:12:46]

Done.

+  v8::Local<v8::String> module_name = args[0].As<v8::String>();
   args.GetReturnValue().Set(RequireForJsInner(module_name));
 }
 
 v8::Local<v8::Value> ModuleSystem::RequireForJsInner(
     v8::Local<v8::String> module_name) {
   v8::EscapableHandleScope handle_scope(GetIsolate());
-  v8::Context::Scope context_scope(context()->v8_context());
+  v8::Local<v8::Context> v8_context = context()->v8_context();
+  v8::Context::Scope context_scope(v8_context);
 
   v8::Local<v8::Object> global(context()->v8_context()->Global());
 
   // The module system might have been deleted. This can happen if a different
   // context keeps a reference to us, but our frame is destroyed (e.g.
   // background page keeps reference to chrome object in a closed popup).
   v8::Local<v8::Value> modules_value = global->GetHiddenValue(
-      v8::String::NewFromUtf8(GetIsolate(), kModulesField));
+      ToV8String(GetIsolate(), kModulesField));
   if (modules_value.IsEmpty() || modules_value->IsUndefined()) {
     Warn(GetIsolate(), "Extension view no longer exists");
     return v8::Undefined(GetIsolate());
   }
 
   v8::Local<v8::Object> modules(v8::Local<v8::Object>::Cast(modules_value));
-  v8::Local<v8::Value> exports(modules->Get(module_name));
-  if (!exports->IsUndefined())
+  v8::Local<v8::Value> exports;
+  if (!modules->Get(v8_context, module_name).ToLocal(&exports) ||
+      !exports->IsUndefined())
     return handle_scope.Escape(exports);
 
   exports = LoadModule(*v8::String::Utf8Value(module_name));
-  modules->Set(module_name, exports);
+  SetProperty(v8_context, modules, module_name, exports);
   return handle_scope.Escape(exports);
 }
 
 v8::Local<v8::Value> ModuleSystem::CallModuleMethod(
     const std::string& module_name,
     const std::string& method_name) {
   v8::EscapableHandleScope handle_scope(GetIsolate());
   v8::Local<v8::Value> no_args;
   return handle_scope.Escape(
       CallModuleMethod(module_name, method_name, 0, &no_args));
@@ skipping 13 matching lines @@
     int argc,
     v8::Local<v8::Value> argv[]) {
   TRACE_EVENT2("v8",
                "v8.callModuleMethod",
                "module_name",
                module_name,
                "method_name",
                method_name);
 
   v8::EscapableHandleScope handle_scope(GetIsolate());
-  v8::Context::Scope context_scope(context()->v8_context());
+  v8::Local<v8::Context> v8_context = context()->v8_context();
+  v8::Context::Scope context_scope(v8_context);
+
+  if (module_name.size() >= v8::String::kMaxLength ||

[not at google - send to devlin, 2015/06/12 16:51:36]

It would be nice to build a version of ToV8String that takes these checks into
account, otherwise it'll probably regress. Possibly another version of
ToV8String that takes a pointer to a handle and returns a bool?

v8::Local<v8::Context> v8_module_name, v8_method_name;
if (!ToV8String(GetIsolate(), module_name, &v8_module_name) ||
    !ToV8String(GetIsolate(), method_name, &v8_method_name)) {
  return ...;
}

Overloading ToV8String in that way is probably a bad idea, so I'd either get rid
of the handle-returning version, or have a name like ToV8StringUnsafe maybe?

[bashi, 2015/06/16 03:12:46]

Added.
Renamed ToV8String() to ToV8StringUnsafe().

+      method_name.size() >= v8::String::kMaxLength)
+    return handle_scope.Escape(
+        v8::Local<v8::Primitive>(v8::Undefined(GetIsolate())));

[not at google - send to devlin, 2015/06/12 16:51:36]

Both the condition and body of this function are multiple lines, I think the
body should be wrapped in {}.

[bashi, 2015/06/16 03:12:47]

Done.

 
   v8::Local<v8::Value> module;
   {
     NativesEnabledScope natives_enabled(this);
-    module = RequireForJsInner(
-        v8::String::NewFromUtf8(GetIsolate(), module_name.c_str()));
+    module = RequireForJsInner(ToV8String(GetIsolate(), module_name.c_str()));
   }
 
   if (module.IsEmpty() || !module->IsObject()) {
     Fatal(context_,
           "Failed to get module " + module_name + " to call " + method_name);
     return handle_scope.Escape(
         v8::Local<v8::Primitive>(v8::Undefined(GetIsolate())));
   }
 
-  v8::Local<v8::Value> value = v8::Local<v8::Object>::Cast(module)->Get(
-      v8::String::NewFromUtf8(GetIsolate(), method_name.c_str()));
-  if (value.IsEmpty() || !value->IsFunction()) {
+  v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(module);
+  v8::Local<v8::String> v8_method_name =
+      ToV8String(GetIsolate(), method_name.c_str());
+  v8::Local<v8::Value> value;
+  if (!object->Get(v8_context, v8_method_name).ToLocal(&value) ||
+      !value->IsFunction()) {
     Fatal(context_, module_name + "." + method_name + " is not a function");
     return handle_scope.Escape(
         v8::Local<v8::Primitive>(v8::Undefined(GetIsolate())));
   }
 
   v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(value);
   v8::Local<v8::Value> result;
   {
-    v8::TryCatch try_catch;
+    v8::TryCatch try_catch(GetIsolate());
     try_catch.SetCaptureMessage(true);
     result = context_->CallFunction(func, argc, argv);
-    if (try_catch.HasCaught())
+    if (try_catch.HasCaught()) {
       HandleException(try_catch);
+      result = v8::Local<v8::Primitive>(v8::Undefined(GetIsolate()));

[not at google - send to devlin, 2015/06/12 16:51:36]

Arghhhh so much verbosity. Why do methods like v8::Undefined and v8::Object::Get
not automatically return a v8::Local rather than a v8::Handle?

[bashi, 2015/06/16 03:12:47]

Not sure, but maybe a historical reason?

+    }
   }
   return handle_scope.Escape(result);
 }
 
 void ModuleSystem::RegisterNativeHandler(
     const std::string& name,
     scoped_ptr<NativeHandler> native_handler) {
   ClobberExistingNativeHandler(name);
   native_handler_map_[name] =
       linked_ptr<NativeHandler>(native_handler.release());
 }
 
 void ModuleSystem::OverrideNativeHandlerForTest(const std::string& name) {
   ClobberExistingNativeHandler(name);
   overridden_native_handlers_.insert(name);
 }
 
 void ModuleSystem::RunString(const std::string& code, const std::string& name) {
   v8::HandleScope handle_scope(GetIsolate());
-  RunString(v8::String::NewFromUtf8(GetIsolate(), code.c_str()),
-            v8::String::NewFromUtf8(GetIsolate(), name.c_str()));
+  if (code.size() >= v8::String::kMaxLength ||
+      name.size() >= v8::String::kMaxLength) {
+    Warn(GetIsolate(), "Too long code or name.");
+    return;
+  }
+  RunString(ToV8String(GetIsolate(), code.c_str()),
+            ToV8String(GetIsolate(), name.c_str()));
 }
 
 // static
 void ModuleSystem::NativeLazyFieldGetter(
-    v8::Local<v8::String> property,
+    v8::Local<v8::Name> property,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
-  LazyFieldGetterInner(property, info, &ModuleSystem::RequireNativeFromString);
+  LazyFieldGetterInner(property.As<v8::String>(), info,
+                       &ModuleSystem::RequireNativeFromString);
 }
 
 // static
 void ModuleSystem::LazyFieldGetter(
-    v8::Local<v8::String> property,
+    v8::Local<v8::Name> property,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
-  LazyFieldGetterInner(property, info, &ModuleSystem::Require);
+  LazyFieldGetterInner(property.As<v8::String>(), info, &ModuleSystem::Require);
 }
 
 // static
 void ModuleSystem::LazyFieldGetterInner(
     v8::Local<v8::String> property,
     const v8::PropertyCallbackInfo<v8::Value>& info,
     RequireFunction require_function) {
   CHECK(!info.Data().IsEmpty());
   CHECK(info.Data()->IsObject());
   v8::HandleScope handle_scope(info.GetIsolate());
   v8::Local<v8::Object> parameters = v8::Local<v8::Object>::Cast(info.Data());
   // This context should be the same as context()->v8_context().
   v8::Local<v8::Context> context = parameters->CreationContext();
   v8::Local<v8::Object> global(context->Global());
   v8::Local<v8::Value> module_system_value = global->GetHiddenValue(
-      v8::String::NewFromUtf8(info.GetIsolate(), kModuleSystem));
+      ToV8String(info.GetIsolate(), kModuleSystem));
   if (module_system_value.IsEmpty() || !module_system_value->IsExternal()) {
     // ModuleSystem has been deleted.
     // TODO(kalman): See comment in header file.
     Warn(info.GetIsolate(),
          "Module system has been deleted, does extension view exist?");
     return;
   }
 
   ModuleSystem* module_system = static_cast<ModuleSystem*>(
       v8::Local<v8::External>::Cast(module_system_value)->Value());
 
-  std::string name = *v8::String::Utf8Value(parameters->Get(
-      v8::String::NewFromUtf8(info.GetIsolate(), kModuleName)));
+  v8::Local<v8::String> v8_module_name =
+      ToV8String(info.GetIsolate(), kModuleName);
+  v8::Local<v8::Value> v8_name;

[not at google - send to devlin, 2015/06/12 16:51:36]

This should be the one called v8_module_name (with name --> module_name), versus
what's currently called |v8_module_name| instead called |v8_k_module_name| or
just inlined. It should always be safe to cast to a v8 value, it would be crazy
not to be.

[bashi, 2015/06/16 03:12:47]

Done.

+  if (!parameters->Get(context, v8_module_name).ToLocal(&v8_name)) {
+    Warn(info.GetIsolate(), "Cannot find module.");
+    return;
+  }
+  std::string name = *v8::String::Utf8Value(v8_name);
 
   // Switch to our v8 context because we need functions created while running
   // the require()d module to belong to our context, not the current one.
   v8::Context::Scope context_scope(context);
   NativesEnabledScope natives_enabled_scope(module_system);
 
-  v8::TryCatch try_catch;
+  v8::TryCatch try_catch(info.GetIsolate());
   v8::Local<v8::Value> module_value = (module_system->*require_function)(name);
   if (try_catch.HasCaught()) {
     module_system->HandleException(try_catch);
     return;
   }
   if (module_value.IsEmpty() || !module_value->IsObject()) {
     // require_function will have already logged this, we don't need to.
     return;
   }
 
   v8::Local<v8::Object> module = v8::Local<v8::Object>::Cast(module_value);
-  v8::Local<v8::String> field =
-      parameters->Get(v8::String::NewFromUtf8(info.GetIsolate(), kModuleField))
-          ->ToString(info.GetIsolate());
+  v8::Local<v8::Value> field_value;
+  if (!parameters->Get(context, ToV8String(info.GetIsolate(), kModuleField))
+           .ToLocal(&field_value)) {
+    module_system->HandleException(try_catch);
+    return;
+  }
+  v8::Local<v8::String> field;
+  if (!field_value->ToString(context).ToLocal(&field)) {
+    module_system->HandleException(try_catch);
+    return;
+  }
 
-  if (!module->Has(field)) {
+  if (!CheckV8Call(module->Has(context, field))) {
     std::string field_str = *v8::String::Utf8Value(field);
     Fatal(module_system->context_,
           "Lazy require of " + name + "." + field_str + " did not set the " +
               field_str + " field");
     return;
   }
 
-  v8::Local<v8::Value> new_field = module->Get(field);
-  if (try_catch.HasCaught()) {
+  v8::Local<v8::Value> new_field;
+  if (!module->Get(context, field).ToLocal(&new_field)) {
     module_system->HandleException(try_catch);
     return;
   }
 
   // Ok for it to be undefined, among other things it's how bindings signify
   // that the extension doesn't have permission to use them.
   CHECK(!new_field.IsEmpty());
 
   // Delete the getter and set this field to |new_field| so the same object is
   // returned every time a certain API is accessed.
   v8::Local<v8::Value> val = info.This();
   if (val->IsObject()) {
     v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(val);
-    object->Delete(property);
-    object->Set(property, new_field);
+    object->Delete(context, property);
+    SetProperty(context, object, property, new_field);
   } else {
     NOTREACHED();
   }
   info.GetReturnValue().Set(new_field);
 }
 
 void ModuleSystem::SetLazyField(v8::Local<v8::Object> object,
                                 const std::string& field,
                                 const std::string& module_name,
                                 const std::string& module_field) {
   SetLazyField(
       object, field, module_name, module_field, &ModuleSystem::LazyFieldGetter);
 }
 
 void ModuleSystem::SetLazyField(v8::Local<v8::Object> object,
                                 const std::string& field,
                                 const std::string& module_name,
                                 const std::string& module_field,
-                                v8::AccessorGetterCallback getter) {
+                                v8::AccessorNameGetterCallback getter) {
+  CHECK(field.size() < v8::String::kMaxLength);
+  CHECK(module_name.size() < v8::String::kMaxLength);
+  CHECK(module_field.size() < v8::String::kMaxLength);
   v8::HandleScope handle_scope(GetIsolate());
   v8::Local<v8::Object> parameters = v8::Object::New(GetIsolate());
-  parameters->Set(v8::String::NewFromUtf8(GetIsolate(), kModuleName),
-                  v8::String::NewFromUtf8(GetIsolate(), module_name.c_str()));
-  parameters->Set(v8::String::NewFromUtf8(GetIsolate(), kModuleField),
-                  v8::String::NewFromUtf8(GetIsolate(), module_field.c_str()));
-  object->SetAccessor(v8::String::NewFromUtf8(GetIsolate(), field.c_str()),
-                      getter,
-                      NULL,
-                      parameters);
+  v8::Local<v8::Context> context = context_->v8_context();
+  SetProperty(context, parameters, ToV8String(GetIsolate(), kModuleName),
+              ToV8String(GetIsolate(), module_name.c_str()));
+  SetProperty(context, parameters, ToV8String(GetIsolate(), kModuleField),
+              ToV8String(GetIsolate(), module_field.c_str()));
+  auto maybe =
+      object->SetAccessor(context, ToV8String(GetIsolate(), field.c_str()),
+                          getter, NULL, parameters);
+  DCHECK(maybe.IsJust() && maybe.FromJust());
 }
 
 void ModuleSystem::SetNativeLazyField(v8::Local<v8::Object> object,
                                       const std::string& field,
                                       const std::string& module_name,
                                       const std::string& module_field) {
   SetLazyField(object,
                field,
                module_name,
                module_field,
                &ModuleSystem::NativeLazyFieldGetter);
 }
 
 v8::Local<v8::Value> ModuleSystem::RunString(v8::Local<v8::String> code,
                                              v8::Local<v8::String> name) {
   v8::EscapableHandleScope handle_scope(GetIsolate());
-  v8::Context::Scope context_scope(context()->v8_context());
+  v8::Local<v8::Context> v8_context = context()->v8_context();
+  v8::Context::Scope context_scope(v8_context);
 
   // Prepend extensions:: to |name| so that internal code can be differentiated
   // from external code in stack traces. This has no effect on behaviour.
   std::string internal_name =
       base::StringPrintf("extensions::%s", *v8::String::Utf8Value(name));
 
+  if (internal_name.size() >= v8::String::kMaxLength)

[not at google - send to devlin, 2015/06/12 16:51:36]

Generally I think many of these failures should have a NOTREACHED or LOG(FATAL)
in their body, so as to not hide mysterious failures.

[bashi, 2015/06/16 03:12:47]

Done.

+    return v8::Undefined(GetIsolate());
+
   blink::WebScopedMicrotaskSuppression suppression;
-  v8::TryCatch try_catch;
+  v8::TryCatch try_catch(GetIsolate());
   try_catch.SetCaptureMessage(true);
-  v8::Local<v8::Script> script(v8::Script::Compile(
-      code, v8::String::NewFromUtf8(GetIsolate(), internal_name.c_str(),
-                                    v8::String::kNormalString,
-                                    internal_name.size())));
-  if (try_catch.HasCaught()) {
+  v8::ScriptOrigin origin(ToV8String(GetIsolate(), internal_name.c_str()));
+  v8::Local<v8::Script> script;
+  if (!v8::Script::Compile(v8_context, code, &origin).ToLocal(&script)) {
     HandleException(try_catch);
     return v8::Undefined(GetIsolate());
   }
 
-  v8::Local<v8::Value> result = script->Run();
-  if (try_catch.HasCaught()) {
+  v8::Local<v8::Value> result;
+  if (!script->Run(v8_context).ToLocal(&result)) {
     HandleException(try_catch);
     return v8::Undefined(GetIsolate());
   }
 
   return handle_scope.Escape(result);
 }
 
 v8::Local<v8::Value> ModuleSystem::GetSource(const std::string& module_name) {
   v8::EscapableHandleScope handle_scope(GetIsolate());
   if (!source_map_->Contains(module_name))
@@ skipping 10 matching lines @@
 }
 
 v8::Local<v8::Value> ModuleSystem::RequireNativeFromString(
     const std::string& native_name) {
   if (natives_enabled_ == 0) {
     // HACK: if in test throw exception so that we can test the natives-disabled
     // logic; however, under normal circumstances, this is programmer error so
     // we could crash.
     if (exception_handler_) {
       return GetIsolate()->ThrowException(
-          v8::String::NewFromUtf8(GetIsolate(), "Natives disabled"));
+          ToV8String(GetIsolate(), "Natives disabled"));
     }
     Fatal(context_, "Natives disabled for requireNative(" + native_name + ")");
     return v8::Undefined(GetIsolate());
   }
 
   if (overridden_native_handlers_.count(native_name) > 0u) {
     return RequireForJsInner(
-        v8::String::NewFromUtf8(GetIsolate(), native_name.c_str()));
+        ToV8String(GetIsolate(), native_name.c_str()));
   }
 
   NativeHandlerMap::iterator i = native_handler_map_.find(native_name);
   if (i == native_handler_map_.end()) {
     Fatal(context_,
           "Couldn't find native for requireNative(" + native_name + ")");
     return v8::Undefined(GetIsolate());
   }
   return i->second->NewInstance();
 }
 
 void ModuleSystem::RequireAsync(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   CHECK_EQ(1, args.Length());
   std::string module_name = *v8::String::Utf8Value(args[0]);
+  v8::Local<v8::Context> v8_context = context_->v8_context();
   v8::Local<v8::Promise::Resolver> resolver(
-      v8::Promise::Resolver::New(GetIsolate()));
+      v8::Promise::Resolver::New(v8_context).ToLocalChecked());
   args.GetReturnValue().Set(resolver->GetPromise());
   scoped_ptr<v8::Global<v8::Promise::Resolver>> global_resolver(
       new v8::Global<v8::Promise::Resolver>(GetIsolate(), resolver));
   gin::ModuleRegistry* module_registry =
-      gin::ModuleRegistry::From(context_->v8_context());
+      gin::ModuleRegistry::From(v8_context);
   if (!module_registry) {
     Warn(GetIsolate(), "Extension view no longer exists");
-    resolver->Reject(v8::Exception::Error(v8::String::NewFromUtf8(
+    resolver->Reject(v8_context, v8::Exception::Error(ToV8String(
         GetIsolate(), "Extension view no longer exists")));
     return;
   }
   module_registry->LoadModule(
       GetIsolate(), module_name,
       base::Bind(&ModuleSystem::OnModuleLoaded, weak_factory_.GetWeakPtr(),
                  base::Passed(&global_resolver)));
   if (module_registry->available_modules().count(module_name) == 0)
     LoadModule(module_name);
 }
 
 v8::Local<v8::String> ModuleSystem::WrapSource(v8::Local<v8::String> source) {
   v8::EscapableHandleScope handle_scope(GetIsolate());
   // Keep in order with the arguments in RequireForJsInner.
-  v8::Local<v8::String> left = v8::String::NewFromUtf8(
+  v8::Local<v8::String> left = ToV8String(
       GetIsolate(),
       "(function(define, require, requireNative, requireAsync, exports, "
       "console, privates,"
       "$Array, $Function, $JSON, $Object, $RegExp, $String, $Error) {"
       "'use strict';");
-  v8::Local<v8::String> right = v8::String::NewFromUtf8(GetIsolate(), "\n})");
+  v8::Local<v8::String> right = ToV8String(GetIsolate(), "\n})");
   return handle_scope.Escape(v8::Local<v8::String>(
       v8::String::Concat(left, v8::String::Concat(source, right))));
 }
 
 void ModuleSystem::Private(const v8::FunctionCallbackInfo<v8::Value>& args) {
   CHECK_EQ(1, args.Length());
   if (!args[0]->IsObject() || args[0]->IsNull()) {
     GetIsolate()->ThrowException(
-        v8::Exception::TypeError(v8::String::NewFromUtf8(GetIsolate(),
+        v8::Exception::TypeError(ToV8String(GetIsolate(),
             args[0]->IsUndefined()
                 ? "Method called without a valid receiver (this). "
                   "Did you forget to call .bind()?"
                 : "Invalid invocation: receiver is not an object!")));
     return;
   }
   v8::Local<v8::Object> obj = args[0].As<v8::Object>();
-  v8::Local<v8::String> privates_key =
-      v8::String::NewFromUtf8(GetIsolate(), "privates");
+  v8::Local<v8::String> privates_key = ToV8String(GetIsolate(), "privates");
   v8::Local<v8::Value> privates = obj->GetHiddenValue(privates_key);
   if (privates.IsEmpty()) {
     privates = v8::Object::New(args.GetIsolate());
     if (privates.IsEmpty()) {
       GetIsolate()->ThrowException(
-          v8::String::NewFromUtf8(GetIsolate(), "Failed to create privates"));
+          ToV8String(GetIsolate(), "Failed to create privates"));
       return;
     }
     obj->SetHiddenValue(privates_key, privates);
   }
   args.GetReturnValue().Set(privates);
 }
 
 v8::Local<v8::Value> ModuleSystem::LoadModule(const std::string& module_name) {
   v8::EscapableHandleScope handle_scope(GetIsolate());
-  v8::Context::Scope context_scope(context()->v8_context());
+  v8::Local<v8::Context> v8_context = context()->v8_context();
+  v8::Context::Scope context_scope(v8_context);
 
   v8::Local<v8::Value> source(GetSource(module_name));
   if (source.IsEmpty() || source->IsUndefined()) {
     Fatal(context_, "No source for require(" + module_name + ")");
     return v8::Undefined(GetIsolate());
   }
   v8::Local<v8::String> wrapped_source(
       WrapSource(v8::Local<v8::String>::Cast(source)));
+  if (module_name.size() >= v8::String::kMaxLength) {
+    Fatal(context_, "Module name is too long");
+    return v8::Undefined(GetIsolate());
+  }
   // Modules are wrapped in (function(){...}) so they always return functions.
   v8::Local<v8::Value> func_as_value =
       RunString(wrapped_source,
-                v8::String::NewFromUtf8(GetIsolate(), module_name.c_str()));
+                ToV8String(GetIsolate(), module_name.c_str()));
   if (func_as_value.IsEmpty() || func_as_value->IsUndefined()) {
     Fatal(context_, "Bad source for require(" + module_name + ")");
     return v8::Undefined(GetIsolate());
   }
 
   v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(func_as_value);
 
   v8::Local<v8::Object> define_object = v8::Object::New(GetIsolate());
   gin::ModuleRegistry::InstallGlobals(GetIsolate(), define_object);
 
   v8::Local<v8::Value> exports = v8::Object::New(GetIsolate());
   v8::Local<v8::Object> natives(NewInstance());
-  CHECK(!natives.IsEmpty());  // this can happen if v8 has issues
+  CHECK(!natives.IsEmpty());  // this can fail if v8 has issues
 
   // These must match the argument order in WrapSource.
   v8::Local<v8::Value> args[] = {
       // AMD.
-      define_object->Get(v8::String::NewFromUtf8(GetIsolate(), "define")),
+      UnsafeGet(v8_context, define_object, ToV8String(GetIsolate(), "define")),
       // CommonJS.
-      natives->Get(v8::String::NewFromUtf8(GetIsolate(), "require",
-                                           v8::String::kInternalizedString)),
-      natives->Get(v8::String::NewFromUtf8(GetIsolate(), "requireNative",
-                                           v8::String::kInternalizedString)),
-      natives->Get(v8::String::NewFromUtf8(GetIsolate(), "requireAsync",
-                                           v8::String::kInternalizedString)),
+      UnsafeGet(v8_context, natives,
+                ToV8String(GetIsolate(), "require",
+                           v8::NewStringType::kInternalized)),
+      UnsafeGet(v8_context, natives,
+                ToV8String(GetIsolate(), "requireNative",
+                           v8::NewStringType::kInternalized)),
+      UnsafeGet(v8_context, natives,
+                ToV8String(GetIsolate(), "requireAsync",
+                           v8::NewStringType::kInternalized)),
       exports,
       // Libraries that we magically expose to every module.
       console::AsV8Object(GetIsolate()),
-      natives->Get(v8::String::NewFromUtf8(GetIsolate(), "privates",
-                                           v8::String::kInternalizedString)),
+      UnsafeGet(v8_context, natives,
+                ToV8String(GetIsolate(), "privates",
+                           v8::NewStringType::kInternalized)),
       // Each safe builtin. Keep in order with the arguments in WrapSource.
       context_->safe_builtins()->GetArray(),
       context_->safe_builtins()->GetFunction(),
       context_->safe_builtins()->GetJSON(),
       context_->safe_builtins()->GetObjekt(),
       context_->safe_builtins()->GetRegExp(),
       context_->safe_builtins()->GetString(),
       context_->safe_builtins()->GetError(),
   };
   {
-    v8::TryCatch try_catch;
+    v8::TryCatch try_catch(GetIsolate());
     try_catch.SetCaptureMessage(true);
     context_->CallFunction(func, arraysize(args), args);
     if (try_catch.HasCaught()) {
       HandleException(try_catch);
       return v8::Undefined(GetIsolate());
     }
   }
   return handle_scope.Escape(exports);
 }
 
@@ skipping 19 matching lines @@
 }
 
 void ModuleSystem::OnModuleLoaded(
     scoped_ptr<v8::Global<v8::Promise::Resolver>> resolver,
     v8::Local<v8::Value> value) {
   if (!is_valid())
     return;
   v8::HandleScope handle_scope(GetIsolate());
   v8::Local<v8::Promise::Resolver> resolver_local(
       v8::Local<v8::Promise::Resolver>::New(GetIsolate(), *resolver));
-  resolver_local->Resolve(value);
+  resolver_local->Resolve(GetIsolate()->GetCurrentContext(), value);

[not at google - send to devlin, 2015/06/12 16:51:36]

Can this be context() rather than GetCurrentContext()?

[bashi, 2015/06/16 03:12:47]

Done.

 }
 
 void ModuleSystem::ClobberExistingNativeHandler(const std::string& name) {
   NativeHandlerMap::iterator existing_handler = native_handler_map_.find(name);
   if (existing_handler != native_handler_map_.end()) {
     clobbered_native_handlers_.push_back(existing_handler->second);
     native_handler_map_.erase(existing_handler);
   }
 }
 
 }  // namespace extensions