MD Settings: Add helper method for responding to cr.sendWithPromise.

MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resolved with a single argument (this is also more consistent
   with Mojo-based communication).
 - Adding additional test cases.

BUG=580633
Committed: https://crrev.com/b60b1f03ab1b8681132ecdf384e02967eee63694
Cr-Commit-Position: refs/heads/master@{#371726}

[dpapad, 2016-01-26 19:42:55]

Description was changed from

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

BUG=580633
==========

to

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

Also restricting C++ response to JS to a single argument, since Promises
can only be resorved with a single argument (this is also more consistent
with Mojo-based communication).

BUG=580633
==========

[dpapad, 2016-01-26 19:44:20]

Description was changed from

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

Also restricting C++ response to JS to a single argument, since Promises
can only be resorved with a single argument (this is also more consistent
with Mojo-based communication).

BUG=580633
==========

to

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Also restricting C++ response to JS to a single argument, since Promises
   can only be resorved with a single argument (this is also more consistent
   with Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

[dpapad, 2016-01-26 19:46:48]

Description was changed from

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Also restricting C++ response to JS to a single argument, since Promises
   can only be resorved with a single argument (this is also more consistent
   with Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

to

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Also restricting C++ response to JS to a single argument, since Promises
   can only be resorved with a single argument (this is also more consistent
   with eventual goal of Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

[dpapad, 2016-01-26 19:47:07]

Description was changed from

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Also restricting C++ response to JS to a single argument, since Promises
   can only be resorved with a single argument (this is also more consistent
   with eventual goal of Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

to

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resorved with a single argument (this is also more consistent
   with eventual goal of Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

[dpapad, 2016-01-26 22:40:44]

Description was changed from

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resorved with a single argument (this is also more consistent
   with eventual goal of Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

to

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resolved with a single argument (this is also more consistent
   with eventual goal of Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

[dpapad, 2016-01-26 23:02:55]

Description was changed from

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resolved with a single argument (this is also more consistent
   with eventual goal of Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

to

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resolved with a single argument (this is also more consistent
   with Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

[dpapad, 2016-01-26 23:04:13]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2016-01-27 00:23:57]

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/appearance_handler.cc (right):

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/appearance_handler.cc:75: const base::Value*
callbackId;
callback_id

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/md_settings_ui.cc (right):

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.cc:47:
webui->CallJavascriptFunction("cr.webUIResponse", callbackId, response);
why not just web_ui()?

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/md_settings_ui.h (right):

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.h:31: void
CallJavascriptCallback(content::WebUI* webui,
web_ui

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.h:32: const base::Value&
callbackId,
callback_id

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.h:33: const base::Value&
response);
\n

https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
File ui/webui/resources/js/cr.js (left):

https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
ui/webui/resources/js/cr.js:342:
resolverFn(Array.prototype.slice.call(arguments, 1));
why did you change this?

[dpapad, 2016-01-27 00:45:24]

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/appearance_handler.cc (right):

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/appearance_handler.cc:75: const base::Value*
callbackId;
On 2016/01/27 at 00:23:57, Dan Beam wrote:
> callback_id

Done.

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/md_settings_ui.cc (right):

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.cc:47:
webui->CallJavascriptFunction("cr.webUIResponse", callbackId, response);
On 2016/01/27 at 00:23:57, Dan Beam wrote:
> why not just web_ui()?

Ah, fixed.

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/md_settings_ui.h (right):

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.h:31: void
CallJavascriptCallback(content::WebUI* webui,
On 2016/01/27 at 00:23:57, Dan Beam wrote:
> web_ui

Removed.

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.h:32: const base::Value&
callbackId,
On 2016/01/27 at 00:23:57, Dan Beam wrote:
> callback_id

Done.

https://codereview.chromium.org/1635673003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_ui.h:33: const base::Value&
response);
On 2016/01/27 at 00:23:57, Dan Beam wrote:
> \n

Done.

https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
File ui/webui/resources/js/cr.js (left):

https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
ui/webui/resources/js/cr.js:342:
resolverFn(Array.prototype.slice.call(arguments, 1));
On 2016/01/27 at 00:23:57, Dan Beam wrote:
> why did you change this?

Explained better in CL description. On top of that, if we were to support
variable number of args in CallJavascriptCallback helper, we would have to add
multiple signatures (similar to CallJavaScriptFunction at
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...),
I don't see a good reason to do this.

[Dan Beam, 2016-01-27 02:30:44]

lgtm

https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
File ui/webui/resources/js/cr.js (left):

https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
ui/webui/resources/js/cr.js:342:
resolverFn(Array.prototype.slice.call(arguments, 1));
On 2016/01/27 00:45:24, dpapad wrote:
> On 2016/01/27 at 00:23:57, Dan Beam wrote:
> > why did you change this?
> 
> Explained better in CL description.

I saw that

> On top of that, if we were to support
> variable number of args in CallJavascriptCallback helper, we would have to add
> multiple signatures (similar to CallJavaScriptFunction at
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...),

i understand that

> I don't see a good reason to do this.

I think the currently proposed mechanism will inspire lots of extra C++
base::ListValues to wrap things in, which is likely more verbose than this
0-line gain in JS.

we'll see!

[dpapad, 2016-01-27 03:17:17]

On 2016/01/27 at 02:30:44, dbeam wrote:
> lgtm
> 
>
https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
> File ui/webui/resources/js/cr.js (left):
> 
>
https://codereview.chromium.org/1635673003/diff/40001/ui/webui/resources/js/c...
> ui/webui/resources/js/cr.js:342:
resolverFn(Array.prototype.slice.call(arguments, 1));
> On 2016/01/27 00:45:24, dpapad wrote:
> > On 2016/01/27 at 00:23:57, Dan Beam wrote:
> > > why did you change this?
> > 
> > Explained better in CL description.
> 
> I saw that
> 
> > On top of that, if we were to support
> > variable number of args in CallJavascriptCallback helper, we would have to
add
> > multiple signatures (similar to CallJavaScriptFunction at
> >
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...),
> 
> i understand that
> 
> > I don't see a good reason to do this.
> 
> I think the currently proposed mechanism will inspire lots of extra C++
base::ListValues to wrap things in, which is likely more verbose than this
0-line gain in JS.
> 
> we'll see!

I still think that it is better having C++ side package all response info to a
single JS object, than having JS code pull out inhomogeneous response arguments
from a wrapping array, for example
{hello: 'world', foo: true, baz: 1} vs  ['world', true, 1]. We'll see.

[dpapad, 2016-01-27 03:18:07]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-01-27 03:18:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1635673003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1635673003/60001

[commit-bot: I haz the power, 2016-01-27 04:39:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-27 04:40:34]

Description was changed from

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resolved with a single argument (this is also more consistent
   with Mojo-based communication).
 - Adding additional test cases.

BUG=580633
==========

to

==========
MD Settings: Add helper method for responding to cr.sendWithPromise.

 - Stop passing the 'cr.webUIResponse' string from JS to C++ and again back to
JS.
 - Restricting C++ response to JS to a single argument, since Promises
   can only be resolved with a single argument (this is also more consistent
   with Mojo-based communication).
 - Adding additional test cases.

BUG=580633

Committed: https://crrev.com/b60b1f03ab1b8681132ecdf384e02967eee63694
Cr-Commit-Position: refs/heads/master@{#371726}
==========

[commit-bot: I haz the power, 2016-01-27 04:40:35]

Patchset 4 (id:??) landed as
https://crrev.com/b60b1f03ab1b8681132ecdf384e02967eee63694
Cr-Commit-Position: refs/heads/master@{#371726}