[ios Mojo] Supporting code for define WebUI function.

[ios Mojo] Supporting code for define WebUI function.

iOS WebUI will be rebuilt on top of Mojo. All mojo dependencies in JavaScript
are managed via |define| function, which is natively implemented in content//
only for WebUI pages. iOS does not use content, so |define| can only be
implemented in JavaScript. Require.js open source library implements |define| in
almost exactly same way as needed by web//. The only exception is that loading
can not happen from network, because WKWebView does not support custom network
protocols.

This CL implements scripts fetching initiated by require.js library.

https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08Bes/edit#heading=h.yjqp5garsqf9

BUG=567809
Committed: https://crrev.com/174e8ecd336f3582187aeff6deb2bb1a4a9803b9
Cr-Commit-Position: refs/heads/master@{#392212}

[Eugene But (OOO till 7-30), 2016-04-28 18:40:17]

Description was changed from

==========
[ios Mojo] Supporting code for define WebUI function.

BUG=567809
==========

to

==========
[ios Mojo] Supporting code for define WebUI function.

iOS WebUI will be rebuilt on top of Mojo. All mojo dependencies in JavaScript
are managed via |define| function, which is natively implemented in content//
only for WebUI pages. iOS does not use content, so |define| can only be
implemented in JavaScript. Require.js open source library implements |define| in
almost exactly same way as needed by web//. The only exception is that loading
can not happen from network, because WKWebView does not support custom network
protocols.

This CL implements scripts fetching initiated by require.js library.

https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

BUG=567809
==========

[Eugene But (OOO till 7-30), 2016-04-28 18:45:23]

eugenebut@chromium.org changed reviewers:
+ dpapad@chromium.org, jyquinn@chromium.org

[Eugene But (OOO till 7-30), 2016-04-28 18:45:23]

This makes |define| from require.js actually work.

[Jackie Quinn, 2016-04-28 21:50:37]

lgtm

[dpapad, 2016-04-28 22:29:20]

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
File ios/web/webui/resources/web_ui_module_load_notifier.js (right):

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:20: * @return {String}
Identifier for the added context, can be later used for
s/String/string/

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:25: this.lastUsedId_ =
loadId;
I am confused by lines 24-25, it seems that loadId local var is not needed at
all.

this.lastUsedId_++;
var key = name + this.lastUsedId_;
...

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:25: this.lastUsedId_ =
loadId;
I am confused by lines 24-25, it seems that loadId local var is not needed at
all.

this.lastUsedId_++;
var key = name + this.lastUsedId_;
...

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:29: }
Nit: Semicolon missing, here and elsewhere.

[Eugene But (OOO till 7-30), 2016-04-29 00:10:41]

Thanks!

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
File ios/web/webui/resources/web_ui_module_load_notifier.js (right):

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:20: * @return {String}
Identifier for the added context, can be later used for
On 2016/04/28 22:29:20, dpapad wrote:
> s/String/string/

Done.

https://codereview.chromium.org/1929783002/diff/40001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:25: this.lastUsedId_ =
loadId;
On 2016/04/28 22:29:20, dpapad wrote:
> I am confused by lines 24-25, it seems that loadId local var is not needed at
> all.
> 
> this.lastUsedId_++;
> var key = name + this.lastUsedId_;
> ...

Done.

[Eugene But (OOO till 7-30), 2016-05-05 15:40:44]

Demetrios, do you have more comments on this?

[dpapad, 2016-05-05 17:19:59]

LGTM with optional nits.

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
File ios/web/webui/resources/web_ui_module_load_notifier.js (right):

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:13: this.pendingContexts_
= {};
Does native Map
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...)
not work well in IOS yet?

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:19: * @param {Object}
context Require.js context to hold.
It does not seem that null/undefined values should be allowed for context. So
the annotation should be !Object (because according to Closure Compiler,
non-primitive types are nullable by default).

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:26:
this.pendingContexts_[key] = context;
Should there be a check that there is no entry for |key| already in the map?

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:48:
WebUIModuleLoadNotifier.prototype.popContext_ = function(name, loadId) {
Type annotations missing.

[Eugene But (OOO till 7-30), 2016-05-05 18:43:49]

Thanks!

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
File ios/web/webui/resources/web_ui_module_load_notifier.js (right):

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:13: this.pendingContexts_
= {};
On 2016/05/05 17:19:59, dpapad wrote:
> Does native Map
>
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...)
> not work well in IOS yet?
It works. Thanks!

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:19: * @param {Object}
context Require.js context to hold.
On 2016/05/05 17:19:59, dpapad wrote:
> It does not seem that null/undefined values should be allowed for context. So
> the annotation should be !Object (because according to Closure Compiler,
> non-primitive types are nullable by default).

Done.

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:26:
this.pendingContexts_[key] = context;
On 2016/05/05 17:19:59, dpapad wrote:
> Should there be a check that there is no entry for |key| already in the map?
key is always a new string, because |lastUsedId_| is always incremented. So this
check will never return true.

https://codereview.chromium.org/1929783002/diff/60001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:48:
WebUIModuleLoadNotifier.prototype.popContext_ = function(name, loadId) {
On 2016/05/05 17:19:59, dpapad wrote:
> Type annotations missing.

Done.

[dpapad, 2016-05-05 18:48:21]

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
File ios/web/webui/resources/web_ui_module_load_notifier.js (right):

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:26:
this.pendingContexts_[key] = context;
If you are using Map, this should be
this.pendingContexts_.set(key, context);

Currently you are not actually using the Map as a Map, simply augmenting the JS
object just like before.

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:57: delete
this.pendingContexts_[key];
Same here. If you are using a map this should be
this.pendingContexts_.delete(key);

[dpapad, 2016-05-05 18:53:29]

I suggest double checking that Map is supported in IOS, when using it as a Map,
not as a plain JS object.

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
File ios/web/webui/resources/web_ui_module_load_notifier.js (right):

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:26:
this.pendingContexts_[key] = context;
On 2016/05/05 at 18:48:21, dpapad wrote:
> If you are using Map, this should be
> this.pendingContexts_.set(key, context);
> 
> Currently you are not actually using the Map as a Map, simply augmenting the
JS object just like before.

To clarify this, try the following in the dev console
var m = new Map();
m['foo'] = 'bar';
console.log(m.size); // returns 0 because 'foo' was not added to the map, it
just augmented the object.
console.log(m.get('foo')); // returns undefined

m.set('foo', 'bar');
console.log(m.size); // returns 1
console.log(m.get('foo')); // returns 'bar'

[Eugene But (OOO till 7-30), 2016-05-05 20:46:52]

All done. Thanks!

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
File ios/web/webui/resources/web_ui_module_load_notifier.js (right):

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:26:
this.pendingContexts_[key] = context;
On 2016/05/05 18:48:21, dpapad wrote:
> If you are using Map, this should be
> this.pendingContexts_.set(key, context);
> 
> Currently you are not actually using the Map as a Map, simply augmenting the
JS
> object just like before.

Done.

https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
ios/web/webui/resources/web_ui_module_load_notifier.js:57: delete
this.pendingContexts_[key];
On 2016/05/05 18:48:21, dpapad wrote:
> Same here. If you are using a map this should be
> this.pendingContexts_.delete(key);

Done.

[dpapad, 2016-05-05 20:51:48]

On 2016/05/05 at 20:46:52, eugenebut wrote:
> All done. Thanks!
> 
>
https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
> File ios/web/webui/resources/web_ui_module_load_notifier.js (right):
> 
>
https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
> ios/web/webui/resources/web_ui_module_load_notifier.js:26:
this.pendingContexts_[key] = context;
> On 2016/05/05 18:48:21, dpapad wrote:
> > If you are using Map, this should be
> > this.pendingContexts_.set(key, context);
> > 
> > Currently you are not actually using the Map as a Map, simply augmenting the
JS
> > object just like before.
> 
> Done.
> 
>
https://codereview.chromium.org/1929783002/diff/80001/ios/web/webui/resources...
> ios/web/webui/resources/web_ui_module_load_notifier.js:57: delete
this.pendingContexts_[key];
> On 2016/05/05 18:48:21, dpapad wrote:
> > Same here. If you are using a map this should be
> > this.pendingContexts_.delete(key);
> 
> Done.

LGTM.

[Eugene But (OOO till 7-30), 2016-05-06 23:07:08]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2016-05-06 23:07:09]

The patchset sent to the CQ was uploaded after l-g-t-m from jyquinn@chromium.org
Link to the patchset: https://codereview.chromium.org/1929783002/#ps100001
(title: "Actually use Map API")

[commit-bot: I haz the power, 2016-05-06 23:07:28]

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

[commit-bot: I haz the power, 2016-05-07 00:13:14]

Description was changed from

==========
[ios Mojo] Supporting code for define WebUI function.

iOS WebUI will be rebuilt on top of Mojo. All mojo dependencies in JavaScript
are managed via |define| function, which is natively implemented in content//
only for WebUI pages. iOS does not use content, so |define| can only be
implemented in JavaScript. Require.js open source library implements |define| in
almost exactly same way as needed by web//. The only exception is that loading
can not happen from network, because WKWebView does not support custom network
protocols.

This CL implements scripts fetching initiated by require.js library.

https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

BUG=567809
==========

to

==========
[ios Mojo] Supporting code for define WebUI function.

iOS WebUI will be rebuilt on top of Mojo. All mojo dependencies in JavaScript
are managed via |define| function, which is natively implemented in content//
only for WebUI pages. iOS does not use content, so |define| can only be
implemented in JavaScript. Require.js open source library implements |define| in
almost exactly same way as needed by web//. The only exception is that loading
can not happen from network, because WKWebView does not support custom network
protocols.

This CL implements scripts fetching initiated by require.js library.

https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

BUG=567809
==========

[commit-bot: I haz the power, 2016-05-07 00:13:15]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-07 00:15:00]

Description was changed from

==========
[ios Mojo] Supporting code for define WebUI function.

iOS WebUI will be rebuilt on top of Mojo. All mojo dependencies in JavaScript
are managed via |define| function, which is natively implemented in content//
only for WebUI pages. iOS does not use content, so |define| can only be
implemented in JavaScript. Require.js open source library implements |define| in
almost exactly same way as needed by web//. The only exception is that loading
can not happen from network, because WKWebView does not support custom network
protocols.

This CL implements scripts fetching initiated by require.js library.

https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

BUG=567809
==========

to

==========
[ios Mojo] Supporting code for define WebUI function.

iOS WebUI will be rebuilt on top of Mojo. All mojo dependencies in JavaScript
are managed via |define| function, which is natively implemented in content//
only for WebUI pages. iOS does not use content, so |define| can only be
implemented in JavaScript. Require.js open source library implements |define| in
almost exactly same way as needed by web//. The only exception is that loading
can not happen from network, because WKWebView does not support custom network
protocols.

This CL implements scripts fetching initiated by require.js library.

https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

BUG=567809

Committed: https://crrev.com/174e8ecd336f3582187aeff6deb2bb1a4a9803b9
Cr-Commit-Position: refs/heads/master@{#392212}
==========

[commit-bot: I haz the power, 2016-05-07 00:15:02]

Patchset 6 (id:??) landed as
https://crrev.com/174e8ecd336f3582187aeff6deb2bb1a4a9803b9
Cr-Commit-Position: refs/heads/master@{#392212}