[Extensions Bindings] Add lastError utilities to APIBindingJSUtil

extensions/renderer/api_binding_js_util.cc

Index: extensions/renderer/api_binding_js_util.cc
diff --git a/extensions/renderer/api_binding_js_util.cc b/extensions/renderer/api_binding_js_util.cc
index d554bb54d1006e7e65d8d89e8f3b2e741cfbc491..90806f2d5384ec65ec03bd6feaf3267d39f77664 100644
--- a/extensions/renderer/api_binding_js_util.cc
+++ b/extensions/renderer/api_binding_js_util.cc
@@ -18,10 +18,12 @@ gin::WrapperInfo APIBindingJSUtil::kWrapperInfo = {gin::kEmbedderNativeGin};
 
 APIBindingJSUtil::APIBindingJSUtil(const APITypeReferenceMap* type_refs,
                                    APIRequestHandler* request_handler,
-                                   APIEventHandler* event_handler)
+                                   APIEventHandler* event_handler,
+                                   const binding::RunJSFunction& run_js)
     : type_refs_(type_refs),
       request_handler_(request_handler),
-      event_handler_(event_handler) {}
+      event_handler_(event_handler),
+      run_js_(run_js) {}
 
 APIBindingJSUtil::~APIBindingJSUtil() {}
 
@@ -32,7 +34,12 @@ gin::ObjectTemplateBuilder APIBindingJSUtil::GetObjectTemplateBuilder(
       .SetMethod("registerEventArgumentMassager",
                  &APIBindingJSUtil::RegisterEventArgumentMassager)
       .SetMethod("createCustomEvent", &APIBindingJSUtil::CreateCustomEvent)
-      .SetMethod("invalidateEvent", &APIBindingJSUtil::InvalidateEvent);
+      .SetMethod("invalidateEvent", &APIBindingJSUtil::InvalidateEvent)
+      .SetMethod("setLastError", &APIBindingJSUtil::SetLastError)
+      .SetMethod("clearLastError", &APIBindingJSUtil::ClearLastError)
+      .SetMethod("hasLastError", &APIBindingJSUtil::HasLastError)
+      .SetMethod("runCallbackWithLastError",
+                 &APIBindingJSUtil::RunCallbackWithLastError);
 }
 
 void APIBindingJSUtil::SendRequest(
@@ -114,4 +121,56 @@ void APIBindingJSUtil::InvalidateEvent(gin::Arguments* arguments,
   event_handler_->InvalidateCustomEvent(context, event);
 }
 
+void APIBindingJSUtil::SetLastError(gin::Arguments* arguments,
+                                    const std::string& error) {
+  v8::Isolate* isolate = arguments->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Object> holder;
+  CHECK(arguments->GetHolder(&holder));
+  v8::Local<v8::Context> context = holder->CreationContext();
+
+  request_handler_->last_error()->SetError(context, error);
+}
+
+void APIBindingJSUtil::ClearLastError(gin::Arguments* arguments) {
+  v8::Isolate* isolate = arguments->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Object> holder;
+  CHECK(arguments->GetHolder(&holder));
+  v8::Local<v8::Context> context = holder->CreationContext();
+
+  bool report_if_unchecked = false;
+  request_handler_->last_error()->ClearError(context, report_if_unchecked);
+}
+
+void APIBindingJSUtil::HasLastError(gin::Arguments* arguments) {
+  v8::Isolate* isolate = arguments->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Object> holder;
+  CHECK(arguments->GetHolder(&holder));
+  v8::Local<v8::Context> context = holder->CreationContext();
+
+  bool has_last_error = request_handler_->last_error()->HasError(context);
+  arguments->Return(has_last_error);
+}
+
+void APIBindingJSUtil::RunCallbackWithLastError(
+    gin::Arguments* arguments,
+    const std::string& error,
+    v8::Local<v8::Function> callback,
+    std::vector<v8::Local<v8::Value>> callback_args) {

[jbroman, 2017/03/20 20:14:49]

const vector&: gin doesn't move the arguments into the callee (maybe it ought
to, but it doesn't at present), so this currently does an unnecessary copy

[Devlin, 2017/03/20 23:10:40]

Moot now with no |callback_args|, but this was actually because the various
RunJS() functions accept a v8::Local<v8::Value>[] - and you can't bind that  to
std::vector<v8::Local<v8::Value>>::data().  I was hoping that I could have taken
a non-const &, but gin barfed on that idea.  We could also hack our way out of
it with various casts, but given v8::Locals are pretty cheap to copy, this
seemed like the most appropriate.

+  v8::Isolate* isolate = arguments->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Object> holder;
+  CHECK(arguments->GetHolder(&holder));
+  v8::Local<v8::Context> context = holder->CreationContext();
+
+  request_handler_->last_error()->SetError(context, error);
+  if (!callback->IsNull() && !callback->IsUndefined())

[jbroman, 2017/03/20 20:14:49]

1. What's the point of calling this will null or undefined anyhow, since it just
sets the error and then shouts (since it will obviously never be checked if that
branch is taken)?

2. Null and undefined aren't v8::Functions, so it would be a type error to get
to this point. (specifically, the gin::Converter logic before this point should
throw an exception if either of those values is provided)

[Devlin, 2017/03/20 23:10:40]

1.  If we mimic the JS api, the function is often a curried callback from an
extension API call, which can be undefined/null if the extension is uninterested
in the result.  And in that case, immediately yelling is the right thing to do. 
But, if we change the signature to accept a bindings-supplied function, this
should never happen.

2.  D'oh.

Done.

+    run_js_.Run(callback, context, callback_args.size(), callback_args.data());
+
+  bool report_if_unchecked = true;
+  request_handler_->last_error()->ClearError(context, report_if_unchecked);
+}
+
 }  // namespace extensions