extensions: Use V8 Maybe APIs in NativeHandler

extensions/renderer/binding_generating_native_handler.cc

Index: extensions/renderer/binding_generating_native_handler.cc
diff --git a/extensions/renderer/binding_generating_native_handler.cc b/extensions/renderer/binding_generating_native_handler.cc
index 2f5dc6f442d48ee282e389dff302a45e69c92bd5..00aaabff61bccfae7b3d56936cd3bff6f7a15717 100644
--- a/extensions/renderer/binding_generating_native_handler.cc
+++ b/extensions/renderer/binding_generating_native_handler.cc
@@ -5,40 +5,69 @@
 #include "extensions/renderer/binding_generating_native_handler.h"
 
 #include "extensions/renderer/module_system.h"
+#include "extensions/renderer/v8_helpers.h"
 
 namespace extensions {
 
+using namespace v8_helpers;
+
 BindingGeneratingNativeHandler::BindingGeneratingNativeHandler(
     ModuleSystem* module_system,
     const std::string& api_name,
     const std::string& bind_to)
     : module_system_(module_system), api_name_(api_name), bind_to_(bind_to) {}
 
-v8::Local<v8::Object> BindingGeneratingNativeHandler::NewInstance() {
+v8::MaybeLocal<v8::Object> BindingGeneratingNativeHandler::NewInstance() {

[not at google - send to devlin, 2015/06/19 17:23:54]

NewInstance doesn't run any third-party code so it should be safe to return a
Local and CHECK that it's never a Maybe value. If our internal code throws an
exception we're screwed. It would make for a nicer interface.

[bashi, 2015/06/23 00:10:49]

Done.

   v8::Isolate* isolate = module_system_->GetIsolate();
   v8::EscapableHandleScope scope(isolate);
-  v8::Local<v8::Object> binding_module =
-      module_system_->Require("binding")->ToObject(isolate);
-  v8::Local<v8::Object> binding =
-      binding_module->Get(v8::String::NewFromUtf8(isolate, "Binding"))
-          ->ToObject(isolate);
+
+  v8::Local<v8::String> v8_api_name;
+  v8::Local<v8::String> v8_bind_to;
+  if (!ToV8String(isolate, api_name_.c_str(), &v8_api_name) ||
+      !ToV8String(isolate, bind_to_.c_str(), &v8_bind_to))
+    return v8::MaybeLocal<v8::Object>();
+
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();

[not at google - send to devlin, 2015/06/19 17:23:54]

Use the context() property rather than the current context?

[bashi, 2015/06/23 00:10:49]

BindingGeneratingNativeHandler doesn't have context() property. Get a context
from ModuleSystem? We need to expose context() if so.

[not at google - send to devlin, 2015/06/23 00:36:48]

Ah right. It should be pretty straightforward to change
BindingGeneratingNativeHandler to be constructed with a ScriptContext instead of
a ModuleSystem.

[bashi, 2015/06/23 00:49:41]

Done. Thanks for the suggestion!

+  v8::Local<v8::Object> binding_module;
+  if (!module_system_->Require("binding")->ToObject(context).ToLocal(
+          &binding_module))
+    return v8::MaybeLocal<v8::Object>();
+
+  v8::Local<v8::Value> binding_value;
+  v8::Local<v8::Object> binding;
+  if (!GetProperty(context, binding_module, "Binding", &binding_value) ||
+      !binding_value->ToObject(context).ToLocal(&binding))
+    return v8::MaybeLocal<v8::Object>();
+
+  v8::Local<v8::Value> create_binding_value;
+  if (!GetProperty(context, binding, "create", &create_binding_value) ||
+      !create_binding_value->IsFunction())
+    return v8::MaybeLocal<v8::Object>();
   v8::Local<v8::Function> create_binding =
-      binding->Get(v8::String::NewFromUtf8(isolate, "create"))
-          .As<v8::Function>();
-  v8::Local<v8::Value> argv[] = {
-      v8::String::NewFromUtf8(isolate, api_name_.c_str())};
-  v8::Local<v8::Object> binding_instance =
-      create_binding->Call(binding, arraysize(argv), argv)->ToObject(isolate);
-  v8::Local<v8::Function> generate =
-      binding_instance->Get(v8::String::NewFromUtf8(isolate, "generate"))
-          .As<v8::Function>();
+      create_binding_value.As<v8::Function>();
+
+  v8::Local<v8::Value> argv[] = {v8_api_name};
+  v8::Local<v8::Value> binding_instance_value;
+  v8::Local<v8::Object> binding_instance;
+  if (!CallFunction(context, create_binding, binding, arraysize(argv), argv,
+                    &binding_instance_value) ||
+      !binding_instance_value->ToObject(context).ToLocal(&binding_instance))
+    return v8::MaybeLocal<v8::Object>();
+
+  v8::Local<v8::Value> generate_value;
+  if (!GetProperty(context, binding_instance, "generate", &generate_value) ||
+      !generate_value->IsFunction())
+    return v8::MaybeLocal<v8::Object>();
+  v8::Local<v8::Function> generate = generate_value.As<v8::Function>();
+
   v8::Local<v8::Object> object = v8::Object::New(isolate);
-  v8::Local<v8::Value> compiled_schema =
-      generate->Call(binding_instance, 0, NULL);
-  if (!compiled_schema.IsEmpty()) {
-    object->Set(v8::String::NewFromUtf8(isolate, bind_to_.c_str()),
-                compiled_schema);
-  }
+  v8::Local<v8::Value> compiled_schema;
+  if (!CallFunction(context, generate, binding_instance, 0, nullptr,
+                    &compiled_schema))
+    return v8::MaybeLocal<v8::Object>();
+
+  if (!SetProperty(context, object, v8_bind_to, compiled_schema))
+    return v8::MaybeLocal<v8::Object>();
   return scope.Escape(object);
 }