Add support for asynchronously loading modules from the background page.

Add support for asynchronously loading modules from the background page.

This is necessary to share Mojo services such as serial connections
among all windows in an app as Mojo services created in a particular
context are only usable while that context exists.

BUG=389016
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290744

[not at google - send to devlin, 2014-08-18 21:22:11]

https://codereview.chromium.org/481853002/diff/60001/extensions/renderer/reso...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/481853002/diff/60001/extensions/renderer/reso...
extensions/renderer/resources/utils.js:162:
chrome.runtime.getBackgroundPage(resolve);
Promise is neat, but strictly speaking you need to watch out for
chrome.runtime.lastError here and reject the Promise if it's set. It looks like
it can indeed fail:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

I think it would help this problem - and just compose better in general - if
there were a utils.promise function which promise-fies an API call? I did
something similar here:

https://github.com/kalman/chrome-extensions/blob/master/snippets/promise.js

but it would be simpler than that. Once you have that function you could do (at
the call site, not in here):

promise(chrome.runtime.getBackgroundPage).then(
  function(bg) {
    return context.GetModuleSystem(bg || window).requireAsync('serial');
  },
  function(err) {
    ???
  }
);

of course it would be nice to support this as a first-class citizen in the
bindings, but that shouldn't block this work :)

This also seems similar to what createAsyncProxy is doing, but that's a little
harder to use (in this context, anyway). You could implement createAsyncProxy in
terms of promise pretty easily.

[Sam McNally, 2014-08-19 01:44:56]

https://codereview.chromium.org/481853002/diff/60001/extensions/renderer/reso...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/481853002/diff/60001/extensions/renderer/reso...
extensions/renderer/resources/utils.js:162:
chrome.runtime.getBackgroundPage(resolve);
On 2014/08/18 21:22:11, kalman wrote:
> Promise is neat, but strictly speaking you need to watch out for
> chrome.runtime.lastError here and reject the Promise if it's set. It looks
like
> it can indeed fail:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
> 
> I think it would help this problem - and just compose better in general - if
> there were a utils.promise function which promise-fies an API call? I did
> something similar here:
> 
> https://github.com/kalman/chrome-extensions/blob/master/snippets/promise.js
> 
> but it would be simpler than that. Once you have that function you could do
(at
> the call site, not in here):
> 
> promise(chrome.runtime.getBackgroundPage).then(
>   function(bg) {
>     return context.GetModuleSystem(bg || window).requireAsync('serial');
>   },
>   function(err) {
>     ???
>   }
> );
> 
> of course it would be nice to support this as a first-class citizen in the
> bindings, but that shouldn't block this work :)
> 
> This also seems similar to what createAsyncProxy is doing, but that's a little
> harder to use (in this context, anyway). You could implement createAsyncProxy
in
> terms of promise pretty easily.

Done. I'd prefer to keep requireAsyncFromBackgroundPage here so we can have a
drop-in replacement for requireAsync.

The async proxy is to skip one side of the promise glue. Instead of something
like
function doSomething() {
  return fooPromise.then(function(foo) {
    return foo.doSomething();
  });
}
we can just refer to "fooProxy.doSomething".

[not at google - send to devlin, 2014-08-19 19:23:38]

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:138: * Wrap an asynchronous API call in a
promise.
Explain how the call happens. I presume that |arg| manipulating is so that you
could do something like:

promise(chrome.storage.get, 'a').then(function(result) {
  console.log('got ' + result);
});

so... explain that. You might also want to DCHECK (logging.DCHECK) that the
first argument is indeed a function.

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:151: resolve($Array.slice(arguments));
This doesn't seem right; so if there are multiple results, they're put in an
array? Should you be calling $Function.apply(resolve, null, arguments)?

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:159: * background page if necessary.
You might actually want to explain why you need this method at all. Why would
you want to require from the background page rather than the current page? Seems
very specialised.

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:175: function
createAsyncProxy(targetPromise, functionNames) {
I'm having a hard time grasping what this method does, and I think it's the
name, it's very generic. Maybe it would help if you called it
createPromiseProxy? Even the "proxy" part is hard to parse. Maybe toPromiseAPI?

[Sam McNally, 2014-08-20 01:27:18]

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:138: * Wrap an asynchronous API call in a
promise.
On 2014/08/19 19:23:38, kalman wrote:
> Explain how the call happens. I presume that |arg| manipulating is so that you
> could do something like:
> 
> promise(chrome.storage.get, 'a').then(function(result) {
>   console.log('got ' + result);
> });
> 
> so... explain that. You might also want to DCHECK (logging.DCHECK) that the
> first argument is indeed a function.

Done.

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:151: resolve($Array.slice(arguments));
On 2014/08/19 19:23:38, kalman wrote:
> This doesn't seem right; so if there are multiple results, they're put in an
> array? Should you be calling $Function.apply(resolve, null, arguments)?

Promises only support one argument.

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:159: * background page if necessary.
On 2014/08/19 19:23:38, kalman wrote:
> You might actually want to explain why you need this method at all. Why would
> you want to require from the background page rather than the current page?
Seems
> very specialised.

Done and moved it into serial_custom_bindings.

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:175: function
createAsyncProxy(targetPromise, functionNames) {
On 2014/08/19 19:23:38, kalman wrote:
> I'm having a hard time grasping what this method does, and I think it's the
> name, it's very generic. Maybe it would help if you called it
> createPromiseProxy? Even the "proxy" part is hard to parse. Maybe
toPromiseAPI?

Fair enough. Removed it and inlined the uses.

[not at google - send to devlin, 2014-08-20 01:47:22]

lgtm

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/481853002/diff/120001/extensions/renderer/res...
extensions/renderer/resources/utils.js:151: resolve($Array.slice(arguments));
On 2014/08/20 01:27:18, Sam McNally wrote:
> On 2014/08/19 19:23:38, kalman wrote:
> > This doesn't seem right; so if there are multiple results, they're put in an
> > array? Should you be calling $Function.apply(resolve, null, arguments)?
> 
> Promises only support one argument.

Yikes. THE MORE YOU KNOW.

[Sam McNally, 2014-08-20 02:26:43]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2014-08-20 02:27:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sammc@chromium.org/481853002/160001

[commit-bot: I haz the power, 2014-08-20 03:38:38]

Committed patchset #4 (160001) as 290744