[Extensions Bindings] Allow custom hooks to return values, throw

extensions/renderer/api_binding_hooks.h

 // Copyright 2016 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_API_BINDING_HOOKS_H_
 #define EXTENSIONS_RENDERER_API_BINDING_HOOKS_H_
 
 #include <map>
 #include <memory>
 #include <string>
@@ skipping 31 matching lines @@
   // them) and the passed arguments. The handler is responsible for returning,
   // which depending on the API could mean either returning synchronously
   // through gin::Arguments::Return or asynchronously through a passed callback.
   // TODO(devlin): As we continue expanding the hooks interface, we should allow
   // handlers to register a request so that they don't have to maintain a
   // reference to the callback themselves.
   using HandleRequestHook =
       base::Callback<RequestResult(const APISignature*,
                                    v8::Local<v8::Context> context,
                                    std::vector<v8::Local<v8::Value>>*,
-                                   const ArgumentSpec::RefMap&)>;
+                                   const ArgumentSpec::RefMap&,
+                                   v8::Local<v8::Value>* return_value)>;
 
   explicit APIBindingHooks(const binding::RunJSFunctionSync& run_js);
   ~APIBindingHooks();
 
   // Register a custom binding to handle requests.
   void RegisterHandleRequest(const std::string& method_name,
                              const HandleRequestHook& hook);
 
   // Registers a JS script to be compiled and run in order to initialize any JS
   // hooks within a v8 context.
   void RegisterJsSource(v8::Global<v8::String> source,
                         v8::Global<v8::String> resource_name);
 
   // Initializes JS hooks within a context.
   void InitializeInContext(v8::Local<v8::Context> context,
                            const std::string& api_name);
 
   // Looks for a custom hook to handle the given request and, if one exists,
-  // runs it. Returns the result of trying to run the hook, or NOT_HANDLED if no
-  // hook was found.
+  // runs it. Returns the result of running the hook, if any. Populates
+  // |return_value| with a value to return, if any.
   RequestResult HandleRequest(const std::string& api_name,
                               const std::string& method_name,
                               v8::Local<v8::Context> context,
                               const APISignature* signature,
                               std::vector<v8::Local<v8::Value>>* arguments,
-                              const ArgumentSpec::RefMap& type_refs);
+                              const ArgumentSpec::RefMap& type_refs,
+                              v8::Local<v8::Value>* return_value);

[jbroman, 2017/01/02 20:05:46]

nit: This argument list is getting a little long. WDYT of at wrapping this up
with the result enum:

struct RequestResult {
  enum ResultCode { HANDLED, INVALID_INVOCATION, ... };
  ResultCode code;
  v8::Local<v8::Value> value;  // if code == HANDLED
};

[Devlin, 2017/01/04 18:38:07]

Hmm... it makes it seem a little more verbose to me, but I don't feel too
strongly.  I've done this in the latest patch - if you prefer it, let's keep it.

 
   // Returns a JS interface that can be used to register hooks.
   v8::Local<v8::Object> GetJSHookInterface(const std::string& api_name,
                                            v8::Local<v8::Context> context);
 
  private:
   // Updates the |arguments| by running |function| and settings arguments to the
   // returned result.
   bool UpdateArguments(v8::Local<v8::Function> function,
                        v8::Local<v8::Context> context,
@@ skipping 16 matching lines @@
   // it's in direct response to JS calling in. There should be no reason that
   // script is disabled.
   binding::RunJSFunctionSync run_js_;
 
   DISALLOW_COPY_AND_ASSIGN(APIBindingHooks);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_RENDERER_API_BINDING_HOOKS_H_