extensions: Use V8 Maybe APIs in ModuleSystem

Replacing DEPRECATE_SOON APIs.

BUG=479065

Review URL: https://codereview.chromium.org/1185443004

Cr-Commit-Position: refs/heads/master@{#335181}

6 files changed, 283 insertions, 107 deletions

diff --git a/extensions/extensions.gypi b/extensions/extensions.gypi
index bd0d294..ac72ad8 100644
--- a/extensions/extensions.gypi
+++ b/extensions/extensions.gypi
@@ -1004,6 +1004,7 @@
       'renderer/utils_native_handler.h',
       'renderer/v8_context_native_handler.cc',
       'renderer/v8_context_native_handler.h',
+      'renderer/v8_helpers.h',
       'renderer/v8_schema_registry.cc',
       'renderer/v8_schema_registry.h',
       'renderer/web_ui_injection_host.cc',
diff --git a/extensions/renderer/module_system.cc b/extensions/renderer/module_system.cc
index 05b51b2..64e3502 100644
--- a/extensions/renderer/module_system.cc
+++ b/extensions/renderer/module_system.cc
@@ -18,12 +18,15 @@
 #include "extensions/renderer/console.h"
 #include "extensions/renderer/safe_builtins.h"
 #include "extensions/renderer/script_context.h"
+#include "extensions/renderer/v8_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 {
 
+using namespace v8_helpers;
+
 namespace {
 
 const char* kModuleSystem = "module_system";
@@ -69,7 +72,7 @@ void Warn(v8::Isolate* isolate, const std::string& message) {
 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
@@ -77,8 +80,9 @@ class DefaultExceptionHandler : public ModuleSystem::ExceptionHandler {
   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
@@ -113,9 +117,11 @@ std::string ModuleSystem::ExceptionHandler::CreateExceptionString(
     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());
 }
 
@@ -140,9 +146,9 @@ ModuleSystem::ModuleSystem(ScriptContext* context, SourceMap* source_map)
 
   v8::Local<v8::Object> global(context->v8_context()->Global());
   v8::Isolate* isolate = context->isolate();
-  global->SetHiddenValue(v8::String::NewFromUtf8(isolate, kModulesField),
+  global->SetHiddenValue(ToV8StringUnsafe(isolate, kModulesField),
                          v8::Object::New(isolate));
-  global->SetHiddenValue(v8::String::NewFromUtf8(isolate, kModuleSystem),
+  global->SetHiddenValue(ToV8StringUnsafe(isolate, kModuleSystem),
                          v8::External::New(isolate, this));
 
   gin::ModuleRegistry::From(context->v8_context())->AddObserver(this);
@@ -161,10 +167,8 @@ void ModuleSystem::Invalidate() {
   {
     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(ToV8StringUnsafe(GetIsolate(), kModulesField));
+    global->DeleteHiddenValue(ToV8StringUnsafe(GetIsolate(), kModuleSystem));
   }
 
   // Invalidate all active and clobbered NativeHandlers we own.
@@ -192,21 +196,28 @@ void ModuleSystem::HandleException(const v8::TryCatch& 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())));
+      ToV8StringUnsafe(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()) {
+    NOTREACHED() << "require() called with a non-string argument";
+    return;
+  }
+  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());
 
@@ -214,19 +225,20 @@ v8::Local<v8::Value> ModuleSystem::RequireForJsInner(
   // 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));
+      ToV8StringUnsafe(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 (!GetProperty(v8_context, modules, module_name, &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);
 }
 
@@ -260,38 +272,46 @@ v8::Local<v8::Value> ModuleSystem::CallModuleMethod(
                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);
+
+  v8::Local<v8::String> v8_module_name;
+  v8::Local<v8::String> v8_method_name;
+  if (!ToV8String(GetIsolate(), module_name.c_str(), &v8_module_name) ||
+      !ToV8String(GetIsolate(), method_name.c_str(), &v8_method_name)) {
+    return handle_scope.Escape(v8::Undefined(GetIsolate()));
+  }
 
   v8::Local<v8::Value> module;
   {
     NativesEnabledScope natives_enabled(this);
-    module = RequireForJsInner(
-        v8::String::NewFromUtf8(GetIsolate(), module_name.c_str()));
+    module = RequireForJsInner(v8_module_name);
   }
 
   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())));
+    return handle_scope.Escape(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::Value> value;
+  if (!GetProperty(v8_context, object, v8_method_name, &value) ||
+      !value->IsFunction()) {
     Fatal(context_, module_name + "." + method_name + " is not a function");
-    return handle_scope.Escape(
-        v8::Local<v8::Primitive>(v8::Undefined(GetIsolate())));
+    return handle_scope.Escape(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::Undefined(GetIsolate());
+    }
   }
   return handle_scope.Escape(result);
 }
@@ -311,22 +331,29 @@ void ModuleSystem::OverrideNativeHandlerForTest(const std::string& 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()));
+  v8::Local<v8::String> v8_code;
+  v8::Local<v8::String> v8_name;
+  if (!ToV8String(GetIsolate(), code.c_str(), &v8_code) ||
+      !ToV8String(GetIsolate(), name.c_str(), &v8_name)) {
+    Warn(GetIsolate(), "Too long code or name.");
+    return;
+  }
+  RunString(v8_code, v8_name);
 }
 
 // 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
@@ -342,7 +369,7 @@ void ModuleSystem::LazyFieldGetterInner(
   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));
+      ToV8StringUnsafe(info.GetIsolate(), kModuleSystem));
   if (module_system_value.IsEmpty() || !module_system_value->IsExternal()) {
     // ModuleSystem has been deleted.
     // TODO(kalman): See comment in header file.
@@ -354,15 +381,19 @@ void ModuleSystem::LazyFieldGetterInner(
   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::Value> v8_module_name;
+  if (!GetProperty(context, parameters, kModuleName, &v8_module_name)) {
+    Warn(info.GetIsolate(), "Cannot find module.");
+    return;
+  }
+  std::string name = *v8::String::Utf8Value(v8_module_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);
@@ -374,11 +405,18 @@ void ModuleSystem::LazyFieldGetterInner(
   }
 
   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 (!GetProperty(context, parameters, kModuleField, &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 (!IsTrue(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 " +
@@ -386,8 +424,8 @@ void ModuleSystem::LazyFieldGetterInner(
     return;
   }
 
-  v8::Local<v8::Value> new_field = module->Get(field);
-  if (try_catch.HasCaught()) {
+  v8::Local<v8::Value> new_field;
+  if (!GetProperty(context, module, field, &new_field)) {
     module_system->HandleException(try_catch);
     return;
   }
@@ -401,8 +439,8 @@ void ModuleSystem::LazyFieldGetterInner(
   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();
   }
@@ -421,17 +459,21 @@ 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, kModuleName,
+              ToV8StringUnsafe(GetIsolate(), module_name.c_str()));
+  SetProperty(context, parameters, kModuleField,
+              ToV8StringUnsafe(GetIsolate(), module_field.c_str()));
+  auto maybe = object->SetAccessor(
+      context, ToV8StringUnsafe(GetIsolate(), field.c_str()), getter, NULL,
+      parameters);
+  CHECK(IsTrue(maybe));
 }
 
 void ModuleSystem::SetNativeLazyField(v8::Local<v8::Object> object,
@@ -448,27 +490,32 @@ void ModuleSystem::SetNativeLazyField(v8::Local<v8::Object> object,
 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) {
+    NOTREACHED() << "internal_name is too long.";
+    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(
+      ToV8StringUnsafe(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());
   }
@@ -499,7 +546,7 @@ v8::Local<v8::Value> ModuleSystem::RequireNativeFromString(
     // we could crash.
     if (exception_handler_) {
       return GetIsolate()->ThrowException(
-          v8::String::NewFromUtf8(GetIsolate(), "Natives disabled"));
+          ToV8StringUnsafe(GetIsolate(), "Natives disabled"));
     }
     Fatal(context_, "Natives disabled for requireNative(" + native_name + ")");
     return v8::Undefined(GetIsolate());
@@ -507,7 +554,7 @@ v8::Local<v8::Value> ModuleSystem::RequireNativeFromString(
 
   if (overridden_native_handlers_.count(native_name) > 0u) {
     return RequireForJsInner(
-        v8::String::NewFromUtf8(GetIsolate(), native_name.c_str()));
+        ToV8StringUnsafe(GetIsolate(), native_name.c_str()));
   }
 
   NativeHandlerMap::iterator i = native_handler_map_.find(native_name);
@@ -523,16 +570,17 @@ 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(ToV8StringUnsafe(
         GetIsolate(), "Extension view no longer exists")));
     return;
   }
@@ -547,13 +595,13 @@ void ModuleSystem::RequireAsync(
 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 = ToV8StringUnsafe(
       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 = ToV8StringUnsafe(GetIsolate(), "\n})");
   return handle_scope.Escape(v8::Local<v8::String>(
       v8::String::Concat(left, v8::String::Concat(source, right))));
 }
@@ -562,7 +610,7 @@ 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(ToV8StringUnsafe(GetIsolate(),
             args[0]->IsUndefined()
                 ? "Method called without a valid receiver (this). "
                   "Did you forget to call .bind()?"
@@ -571,13 +619,13 @@ void ModuleSystem::Private(const v8::FunctionCallbackInfo<v8::Value>& args) {
   }
   v8::Local<v8::Object> obj = args[0].As<v8::Object>();
   v8::Local<v8::String> privates_key =
-      v8::String::NewFromUtf8(GetIsolate(), "privates");
+      ToV8StringUnsafe(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"));
+          ToV8StringUnsafe(GetIsolate(), "Failed to create privates"));
       return;
     }
     obj->SetHiddenValue(privates_key, privates);
@@ -587,7 +635,8 @@ void ModuleSystem::Private(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
 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()) {
@@ -596,10 +645,14 @@ v8::Local<v8::Value> ModuleSystem::LoadModule(const std::string& module_name) {
   }
   v8::Local<v8::String> wrapped_source(
       WrapSource(v8::Local<v8::String>::Cast(source)));
+  v8::Local<v8::String> v8_module_name;
+  if (!ToV8String(GetIsolate(), module_name.c_str(), &v8_module_name)) {
+    NOTREACHED() << "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()));
+      RunString(wrapped_source, v8_module_name);
   if (func_as_value.IsEmpty() || func_as_value->IsUndefined()) {
     Fatal(context_, "Bad source for require(" + module_name + ")");
     return v8::Undefined(GetIsolate());
@@ -612,24 +665,24 @@ v8::Local<v8::Value> ModuleSystem::LoadModule(const std::string& module_name) {
 
   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")),
+      GetPropertyUnsafe(v8_context, define_object, "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)),
+      GetPropertyUnsafe(v8_context, natives, "require",
+                        v8::NewStringType::kInternalized),
+      GetPropertyUnsafe(v8_context, natives, "requireNative",
+                        v8::NewStringType::kInternalized),
+      GetPropertyUnsafe(v8_context, natives, "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)),
+      GetPropertyUnsafe(v8_context, natives, "privates",
+                        v8::NewStringType::kInternalized),
       // Each safe builtin. Keep in order with the arguments in WrapSource.
       context_->safe_builtins()->GetArray(),
       context_->safe_builtins()->GetFunction(),
@@ -640,7 +693,7 @@ v8::Local<v8::Value> ModuleSystem::LoadModule(const std::string& module_name) {
       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()) {
@@ -680,7 +733,7 @@ void ModuleSystem::OnModuleLoaded(
   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(context()->v8_context(), value);
 }
 
 void ModuleSystem::ClobberExistingNativeHandler(const std::string& name) {
diff --git a/extensions/renderer/module_system.h b/extensions/renderer/module_system.h
index 82474b6..895aa56 100644
--- a/extensions/renderer/module_system.h
+++ b/extensions/renderer/module_system.h
@@ -51,12 +51,16 @@ class ModuleSystem : public ObjectBackedNativeHandler,
 
   class ExceptionHandler {
    public:
+    explicit ExceptionHandler(ScriptContext* context) : context_(context) {}
     virtual ~ExceptionHandler() {}
     virtual void HandleUncaughtException(const v8::TryCatch& try_catch) = 0;
 
    protected:
     // Formats |try_catch| as a nice string.
     std::string CreateExceptionString(const v8::TryCatch& try_catch);
+    // A script context associated with this handler. Owned by the module
+    // system.
+    ScriptContext* context_;
   };
 
   // Enables native bindings for the duration of its lifetime.
@@ -126,7 +130,7 @@ class ModuleSystem : public ObjectBackedNativeHandler,
                     const std::string& field,
                     const std::string& module_name,
                     const std::string& module_field,
-                    v8::AccessorGetterCallback getter);
+                    v8::AccessorNameGetterCallback getter);
 
   // Make |object|.|field| lazily evaluate to the result of
   // requireNative(|module_name|)[|module_field|].
@@ -150,12 +154,12 @@ class ModuleSystem : public ObjectBackedNativeHandler,
   typedef std::map<std::string, linked_ptr<NativeHandler> > NativeHandlerMap;
 
   // Retrieves the lazily defined field specified by |property|.
-  static void LazyFieldGetter(v8::Local<v8::String> property,
+  static void LazyFieldGetter(v8::Local<v8::Name> property,
                               const v8::PropertyCallbackInfo<v8::Value>& info);
   // Retrieves the lazily defined field specified by |property| on a native
   // object.
   static void NativeLazyFieldGetter(
-      v8::Local<v8::String> property,
+      v8::Local<v8::Name> property,
       const v8::PropertyCallbackInfo<v8::Value>& info);
 
   // Called when an exception is thrown but not caught.
diff --git a/extensions/renderer/module_system_test.cc b/extensions/renderer/module_system_test.cc
index 04de2b1..50b6416 100644
--- a/extensions/renderer/module_system_test.cc
+++ b/extensions/renderer/module_system_test.cc
@@ -27,6 +27,7 @@ namespace {
 
 class FailsOnException : public ModuleSystem::ExceptionHandler {
  public:
+  FailsOnException() : ModuleSystem::ExceptionHandler(nullptr) {}
   void HandleUncaughtException(const v8::TryCatch& try_catch) override {
     FAIL() << "Uncaught exception: " << CreateExceptionString(try_catch);
   }
@@ -129,10 +130,10 @@ ModuleSystemTestEnvironment::ModuleSystemTestEnvironment(v8::Isolate* isolate)
   context_holder_->SetContext(v8::Context::New(
       isolate, g_v8_extension_configurator.Get().GetConfiguration()));
   context_.reset(new ScriptContext(context_holder_->context(),
-                                   NULL,  // WebFrame
-                                   NULL,  // Extension
+                                   nullptr,  // WebFrame
+                                   nullptr,  // Extension
                                    Feature::BLESSED_EXTENSION_CONTEXT,
-                                   NULL,  // Effective Extension
+                                   nullptr,  // Effective Extension
                                    Feature::BLESSED_EXTENSION_CONTEXT));
   context_->v8_context()->Enter();
   assert_natives_ = new AssertNatives(context_.get());
diff --git a/extensions/renderer/module_system_unittest.cc b/extensions/renderer/module_system_unittest.cc
index e687841..c5c916e 100644
--- a/extensions/renderer/module_system_unittest.cc
+++ b/extensions/renderer/module_system_unittest.cc
@@ -34,7 +34,8 @@ class CounterNatives : public ObjectBackedNativeHandler {
 
 class TestExceptionHandler : public ModuleSystem::ExceptionHandler {
  public:
-  TestExceptionHandler() : handled_exception_(false) {}
+  TestExceptionHandler()
+      : ModuleSystem::ExceptionHandler(nullptr), handled_exception_(false) {}
 
   void HandleUncaughtException(const v8::TryCatch& try_catch) override {
     handled_exception_ = true;
@@ -404,11 +405,14 @@ TEST_F(ModuleSystemTest, TestRequireAsyncFromAnotherContext) {
                             "    return 'pong';"
                             "  }"
                             "});");
-  gin::ModuleRegistry::From(env()->context()->v8_context())->AddBuiltinModule(
-      env()->isolate(), "natives", other_env->module_system()->NewInstance());
+  gin::ModuleRegistry::From(env()->context()->v8_context())
+      ->AddBuiltinModule(
+          env()->isolate(), "natives",
+          other_env->module_system()->NewInstance());
   gin::ModuleRegistry::From(other_env->context()->v8_context())
       ->AddBuiltinModule(
-          env()->isolate(), "natives", env()->module_system()->NewInstance());
+          env()->isolate(), "natives",
+          env()->module_system()->NewInstance());
   env()->module_system()->Require("test");
   RunResolvedPromises();
 }
@@ -438,11 +442,14 @@ TEST_F(ModuleSystemTest, TestRequireAsyncBetweenContexts) {
                             "    return natives.requireAsync('pong');"
                             "  }"
                             "});");
-  gin::ModuleRegistry::From(env()->context()->v8_context())->AddBuiltinModule(
-      env()->isolate(), "natives", other_env->module_system()->NewInstance());
+  gin::ModuleRegistry::From(env()->context()->v8_context())
+      ->AddBuiltinModule(
+          env()->isolate(), "natives",
+          other_env->module_system()->NewInstance());
   gin::ModuleRegistry::From(other_env->context()->v8_context())
       ->AddBuiltinModule(
-          env()->isolate(), "natives", env()->module_system()->NewInstance());
+          env()->isolate(), "natives",
+          env()->module_system()->NewInstance());
   env()->module_system()->Require("test");
   RunResolvedPromises();
 }
@@ -461,8 +468,10 @@ TEST_F(ModuleSystemTest, TestRequireAsyncFromContextWithNoModuleRegistry) {
                         "  });"
                         "});");
   scoped_ptr<ModuleSystemTestEnvironment> other_env = CreateEnvironment();
-  gin::ModuleRegistry::From(env()->context()->v8_context())->AddBuiltinModule(
-      env()->isolate(), "natives", other_env->module_system()->NewInstance());
+  gin::ModuleRegistry::From(env()->context()->v8_context())
+      ->AddBuiltinModule(
+          env()->isolate(), "natives",
+          other_env->module_system()->NewInstance());
   other_env->ShutdownGin();
   env()->module_system()->Require("test");
   RunResolvedPromises();
@@ -477,8 +486,10 @@ TEST_F(ModuleSystemTest, TestRequireAsyncFromContextWithNoModuleSystem) {
                         "      natives.requireAsync('foo') === undefined);"
                         "});");
   scoped_ptr<ModuleSystemTestEnvironment> other_env = CreateEnvironment();
-  gin::ModuleRegistry::From(env()->context()->v8_context())->AddBuiltinModule(
-      env()->isolate(), "natives", other_env->module_system()->NewInstance());
+  gin::ModuleRegistry::From(env()->context()->v8_context())
+      ->AddBuiltinModule(
+          env()->isolate(), "natives",
+          other_env->module_system()->NewInstance());
   other_env->ShutdownModuleSystem();
   env()->module_system()->Require("test");
   RunResolvedPromises();
diff --git a/extensions/renderer/v8_helpers.h b/extensions/renderer/v8_helpers.h
new file mode 100644
index 0000000..385490f
--- /dev/null
+++ b/extensions/renderer/v8_helpers.h
@@ -0,0 +1,106 @@
+// Copyright 2015 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.
+
+#ifndef EXTENSIONS_RENDERER_V8_HELPERS_H_
+#define EXTENSIONS_RENDERER_V8_HELPERS_H_
+
+#include <string.h>
+
+#include "v8/include/v8.h"
+
+namespace extensions {
+namespace v8_helpers {
+
+// Helper functions for V8 APIs.
+
+// Converts |str| to a V8 string. Returns true on success.
+inline bool ToV8String(v8::Isolate* isolate,
+                       const char* str,
+                       v8::Local<v8::String>* out) {
+  return v8::String::NewFromUtf8(isolate, str, v8::NewStringType::kNormal)
+      .ToLocal(out);
+}
+
+// Converts |str| to a V8 string.
+// This crashes when strlen(str) > v8::String::kMaxLength.
+inline v8::Local<v8::String> ToV8StringUnsafe(
+    v8::Isolate* isolate,
+    const char* str,
+    v8::NewStringType string_type = v8::NewStringType::kNormal) {
+  DCHECK(strlen(str) <= v8::String::kMaxLength);
+  return v8::String::NewFromUtf8(isolate, str, string_type)
+      .ToLocalChecked();
+}
+
+// Returns true if |maybe| is both a value, and that value is true.
+inline bool IsTrue(v8::Maybe<bool> maybe) {
+  return maybe.IsJust() && maybe.FromJust();
+}
+
+// SetProperty() family wraps V8::Object::Set(). Returns true on success.
+inline bool SetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        v8::Local<v8::Value> key,
+                        v8::Local<v8::Value> value) {
+  return IsTrue(object->Set(context, key, value));
+}
+
+inline bool SetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        uint32_t index,
+                        v8::Local<v8::Value> value) {
+  return IsTrue(object->Set(context, index, value));
+}
+
+inline bool SetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        const char* key,
+                        v8::Local<v8::Value> value) {
+  v8::Local<v8::String> v8_key;
+  if (!ToV8String(context->GetIsolate(), key, &v8_key))
+    return false;
+  return SetProperty(context, object, v8_key, value);
+}
+
+// GetProperty() family calls V8::Object::Get() and extracts a value from
+// returned MaybeLocal. Returns true on success.
+inline bool GetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        v8::Local<v8::Value> key,
+                        v8::Local<v8::Value>* out) {
+  return object->Get(context, key).ToLocal(out);
+}
+
+inline bool GetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        const char* key,
+                        v8::Local<v8::Value>* out) {
+  v8::Local<v8::String> v8_key;
+  if (!ToV8String(context->GetIsolate(), key, &v8_key))
+    return false;
+  return GetProperty(context, object, v8_key, out);
+}
+
+// GetPropertyUnsafe() family wraps v8::Object::Get(). They crash when an
+// exception is thrown.
+inline v8::Local<v8::Value> GetPropertyUnsafe(v8::Local<v8::Context> context,
+                                              v8::Local<v8::Object> object,
+                                              v8::Local<v8::Value> key) {
+  return object->Get(context, key).ToLocalChecked();
+}
+
+inline v8::Local<v8::Value> GetPropertyUnsafe(
+    v8::Local<v8::Context> context,
+    v8::Local<v8::Object> object,
+    const char* key,
+    v8::NewStringType string_type = v8::NewStringType::kNormal) {
+  return object->Get(context,
+                     ToV8StringUnsafe(context->GetIsolate(), key, string_type))
+      .ToLocalChecked();
+}
+
+}  // namespace v8_helpers
+}  // namespace extensions
+
+#endif  // EXTENSIONS_RENDERER_V8_HELPERS_H_