[Extensions Bindings] Add support for updateArgumentsPreValidate

extensions/renderer/native_extension_bindings_system.cc

Index: extensions/renderer/native_extension_bindings_system.cc
diff --git a/extensions/renderer/native_extension_bindings_system.cc b/extensions/renderer/native_extension_bindings_system.cc
index f04aa91a2ccedfef21a1bdb069684ec3ca22283b..46d09a09959daad77479237ccc59b3728237cc93 100644
--- a/extensions/renderer/native_extension_bindings_system.cc
+++ b/extensions/renderer/native_extension_bindings_system.cc
@@ -82,6 +82,52 @@ void CallJsFunction(v8::Local<v8::Function> function,
   script_context->SafeCallFunction(function, argc, argv);
 }
 
+// A helper class for calling JS function synchronously.
+class CallJsSyncHelper {
+ public:
+  CallJsSyncHelper(v8::Isolate* isolate) : isolate_(isolate),
+                                           weak_factory_(this) {}
+  ~CallJsSyncHelper() {}
+
+  ScriptInjectionCallback::CompleteCallback GetCallback() {
+    return base::Bind(&CallJsSyncHelper::OnJsComplete,
+                      weak_factory_.GetWeakPtr());
+  }
+
+  bool did_complete() const { return did_complete_; }
+  v8::Global<v8::Value> take_result() { return std::move(result_); }
+
+ private:
+  void OnJsComplete(const std::vector<v8::Local<v8::Value>>& results) {
+    did_complete_ = true;
+    // The locals are released after the callback is executed, so we need to
+    // grab a persistent handle.
+    if (!results.empty() && !results[0].IsEmpty())
+      result_.Reset(isolate_, results[0]);
+  }
+
+  bool did_complete_ = false;
+  v8::Isolate* isolate_;
+  v8::Global<v8::Value> result_;
+  base::WeakPtrFactory<CallJsSyncHelper> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallJsSyncHelper);
+};
+
+v8::Global<v8::Value> CallJsFunctionSync(v8::Local<v8::Function> function,
+                                         v8::Local<v8::Context> context,
+                                         int argc,
+                                         v8::Local<v8::Value> argv[]) {
+  CallJsSyncHelper helper(context->GetIsolate());
+  ScriptContext* script_context =
+      ScriptContextSet::GetContextByV8Context(context);
+  CHECK(script_context);
+  script_context->SafeCallFunction(function, argc, argv, helper.GetCallback());

[jbroman, 2017/01/02 19:46:38]

WDYT of this, which seems to do the same but be somewhat shorter than the helper
class:

bool did_complete = false;
v8::Global<v8::Value> result;
auto callback = base::Bind([](v8::Isolate* isolate,
                              bool* did_complete_out,
                              v8::Local<v8::Value>* result_out,
                              const std::vector<v8::Local<v8::Value>>& results)
{
  *did_complete_out = true;
  // The locals are released after the callback is executed, so we need to
  // grab a persistent handle.
  if (!results.empty() && !results[0].IsEmpty())
    result_out->Reset(isolate, results[0]);
}, base::Unretained(isolate), base::Unretained(&did_complete),
base::Unretained(&result));

ScriptContext* script_context =
    ScriptContextSet::GetContextByV8Context(context);
CHECK(script_context);
script_context->SafeCallFunction(function, argc, argv, callback);
CHECK(did_complete) << "expected script to execute synchronously";
return result;

I don't feel strongly; just a thought.

We already CHECK if the callback isn't invoked before the helper could die, so I
don't think the weak-ptr indirection helps.

(I'm not thrilled with passing around Global here as Local is usually used to
pass things around on the stack, but I can see why it arises from the way
SafeCallFunction works. :/)

[Devlin, 2017/01/04 17:57:02]

I like it.  Done.

I've also added a TODO to express the desire to use Local instead (in the
comment in the types.h file).

+  // If the JS did not, in fact, execute synchronously, there's a big problem.
+  CHECK(helper.did_complete());
+  return helper.take_result();
+}
+
 // Returns the API schema indicated by |api_name|.
 const base::DictionaryValue& GetAPISchema(const std::string& api_name) {
   const base::DictionaryValue* schema =
@@ -103,6 +149,7 @@ NativeExtensionBindingsSystem::NativeExtensionBindingsSystem(
     const SendIPCMethod& send_ipc)
     : send_ipc_(send_ipc),
       api_system_(base::Bind(&CallJsFunction),
+                  base::Bind(&CallJsFunctionSync),
                   base::Bind(&GetAPISchema),
                   base::Bind(&NativeExtensionBindingsSystem::SendRequest,
                              base::Unretained(this))),