WebUI: Replace cr.sendWithCallback with cr.sendWithPromise.

ui/webui/resources/js/cr.js

Index: ui/webui/resources/js/cr.js
diff --git a/ui/webui/resources/js/cr.js b/ui/webui/resources/js/cr.js
index a38c59f84732e820c00407287b3ef00c37b41bb2..448cb85c025f8bf490697e353d35d1646ebf75aa 100644
--- a/ui/webui/resources/js/cr.js
+++ b/ui/webui/resources/js/cr.js
@@ -313,48 +313,52 @@ var cr = function() {
   }
 
   /**
-   * The mapping used by the sendWithCallback mechanism to tie the callback
-   * supplied to an invocation of sendWithCallback with the WebUI response
-   * sent by the browser in response to the chrome.send call. The mapping is
-   * from ID to callback function; the ID is generated by sendWithCallback and
-   * is unique across all invocations of said method.
-   * @type {!Object<Function>}
+   * The mapping used by the sendWithPromise mechanism to tie the Promise
+   * returned to callers with the corresponding WebUI response. The mapping is
+   * from ID to the Promise resolver function; the ID is generated by
+   * sendWithPromise and is unique across all invocations of said method.
+   * @type {!Object<!Function>}
    */
-  var chromeSendCallbackMap = Object.create(null);
+  var chromeSendResolverMap = {};
 
   /**
    * The named method the WebUI handler calls directly in response to a
-   * chrome.send call that expects a callback. The handler requires no knowledge
+   * chrome.send call that expects a response. The handler requires no knowledge
    * of the specific name of this method, as the name is passed to the handler
    * as the first argument in the arguments list of chrome.send. The handler
    * must pass the ID, also sent via the chrome.send arguments list, as the
    * first argument of the JS invocation; additionally, the handler may
-   * supply any number of other arguments that will be forwarded to the
-   * callback.
-   * @param {string} id The unique ID identifying the callback method this
-   *    response is tied to.
+   * supply any number of other arguments that will be included in the response.
+   * @param {string} id The unique ID identifying the Promise this response is
+   *     tied to.
+   * @param {...*} var_args Variable number of arguments to be included in the
+   *     response.
    */
-  function webUIResponse(id) {
-    chromeSendCallbackMap[id].apply(
-        null, Array.prototype.slice.call(arguments, 1));
-    delete chromeSendCallbackMap[id];
+  function webUIResponse(id, var_args) {
+    var resolverFn = chromeSendResolverMap[id];
+    delete chromeSendResolverMap[id];
+    // If no response arguments were passed from C++, resolving the Promise

[Dan Beam, 2016/01/25 18:48:37]

resolve

[dpapad, 2016/01/25 19:10:36]

Done.

+    // without any arguments, else wraping all response arguments in an array

[Dan Beam, 2016/01/25 18:48:37]

wrap

[dpapad, 2016/01/25 19:10:36]

Done.

+    // since Promise#resolve accepts only one value.
+    var responseArgs = Array.prototype.slice.call(arguments, 1);
+    resolverFn(responseArgs.length == 0 ? undefined : responseArgs);

[Dan Beam, 2016/01/25 18:48:37]

why not just resolve with an empty array?

[dpapad, 2016/01/25 19:10:36]

Partly to avoid creating empty arrays unnecessarily and partly because I thought
it makes it clearer that no arguments were passed from C++.

Another alternative I am considering is to restrict C++ calls of webUIResponse()
to only pass the callback ID plus a single argument (primitive value or
DictionaryValue/ListValue) such that we don't need to wrap multiple response
aguments to an Array ourselves here. I think that is better since it is the most
flexible. As of now even a primitive (say boolean) arrives at the caller within
an array of length 1, see change in appearance_page.js.

[Dan Beam, 2016/01/25 23:13:38]

responseArgs is already created
ehhhhh, I think it's more annoying to have to check the type of args.  counting
on it always being the same type (at least the wrapper) is less error prone, IMO
arguments <-> list manipulation is pretty common in JS, so I don't think one is
preferred to the other.  chrome.send() could easily be made to do
chrome.send('msg', param1, param2) instead of chrome.send('msg', [param1,
param2]), but it's not really a game changer

[dpapad, 2016/01/25 23:54:44]

Changed it such that the array is always forwarded to the Promise.

It is far more common to send multiple arguments in the request, than receive
multiple arguments in the response. Wrapping arguments passed from C++ (which is
the response) in an array seems unnecessary when in most cases C++ has already
pre-packaged the response to an array or object such that only one argument is
passed back. Either way, I think we can refine this point in a follow up CL if
necessary.


On 2016/01/25 at 23:13:38, Dan Beam wrote:

   }
 
   /**
-   * A variation of chrome.send which allows the client to receive a direct
-   * callback without requiring the handler to have specific knowledge of any
-   * JS internal method names or state. The callback will be removed from the
-   * mapping once it has fired.
+   * A variation of chrome.send, suitable for messages that expect a single
+   * response from C++.
    * @param {string} methodName The name of the WebUI handler API.
-   * @param {Array|undefined} args Arguments for the method call sent to the
-   *     WebUI handler. Pass undefined if no args should be sent to the handler.
-   * @param {Function} callback A callback function which is called (indirectly)
-   *     by the WebUI handler.
+   * @param {...*} var_args Varibale number of arguments to be forwarded to the
+   *     C++ call.
+   * @return {!Promise}
    */
-  function sendWithCallback(methodName, args, callback) {
-    var id = methodName + createUid();
-    chromeSendCallbackMap[id] = callback;
-    chrome.send(methodName, ['cr.webUIResponse', id].concat(args || []));
+  function sendWithPromise(methodName, var_args) {
+    var args = Array.prototype.slice.call(arguments, 1);
+    return new Promise(function(resolve, reject) {
+      var id = methodName + '_' + createUid();

[dpapad, 2016/01/25 18:34:22]

FYI, added an '_' symbol between methodName and the uid, because previous code
had a corner case that would break it. Imagine the following scenario.

Two chrome.send() message names exist 'foo1' and 'foo'.

sendWithPromise('foo1') // inserts key 'foo11' to the map object.
Now calling sendWithPromise('foo') 10 times also inserts key 'foo11' to the map
(possibly overwriting the entry that corresponds to message name 'foo1'. New
code would not have a problem since it would insert
foo_11 instead.

[Dan Beam, 2016/01/25 18:48:37]

Acknowledged.

+      chromeSendResolverMap[id] = resolve;
+      chrome.send(methodName, ['cr.webUIResponse', id].concat(args));
+    });
   }
 
   /**
@@ -403,7 +407,7 @@ var cr = function() {
     getUid: getUid,
     makePublic: makePublic,
     webUIResponse: webUIResponse,
-    sendWithCallback: sendWithCallback,
+    sendWithPromise: sendWithPromise,
     webUIListenerCallback: webUIListenerCallback,
     addWebUIListener: addWebUIListener,
     PropertyKind: PropertyKind,