[ios Mojo] Committed forked version of require.js.

[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Changes made in require.js:
 - changed code for loading the modules;
 - removed workarounds non needed for WebKit;
 - removed code which is not used for WebUI;

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08Bes/edit#heading=h.yjqp5garsqf9

BUG=567809
Committed: https://crrev.com/703f8f2899ceccc0c05d05b6e3930b2e6c23aeb9
Cr-Commit-Position: refs/heads/master@{#392194}

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

Description was changed from

==========
[ios Mojo] Committed forked version of require.js.

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

to

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

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

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

Description was changed from

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

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

to

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

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

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

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

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

jyquinn@ for iOS WebUI
dpapad@ for JavaScript
xam@ for OSTPR

[chromium-reviews, 2016-04-28 19:24:16]

LGTM for OSTPR

On Thu, Apr 28, 2016 at 11:42 AM,  <eugenebut@chromium.org> wrote:
> Reviewers: Jackie Quinn, dpapad, xam
> CL: https://codereview.chromium.org/1931833004/
>
> Message:
> jyquinn@ for iOS WebUI
> dpapad@ for JavaScript
> xam@ for OSTPR
>
>
> Description:
> [ios Mojo] Forked version of require.js needed for WebUI Mojo.
>
> 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 require.js file is a forked and stripped down version of open-source
> require.js. Fetching the scripts is delegated to native code by sending
> loadMojo message.
>
> Mojo for WebUI design doc:
>
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...
>
> BUG=567809
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+1102, -2 lines):
> A + ios/third_party/requirejs/LICENSE
> A + ios/third_party/requirejs/OWNERS
> A ios/third_party/requirejs/README.chromium
> A ios/third_party/requirejs/require.js
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

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

On 2016/04/28 19:24:16, chromium-reviews wrote:
> LGTM for OSTPR
> 
> On Thu, Apr 28, 2016 at 11:42 AM,  <mailto:eugenebut@chromium.org> wrote:
> > Reviewers: Jackie Quinn, dpapad, xam
> > CL: https://codereview.chromium.org/1931833004/
> >
> > Message:
> > jyquinn@ for iOS WebUI
> > dpapad@ for JavaScript
> > xam@ for OSTPR
> >
> >
> > Description:
> > [ios Mojo] Forked version of require.js needed for WebUI Mojo.
> >
> > 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 require.js file is a forked and stripped down version of open-source
> > require.js. Fetching the scripts is delegated to native code by sending
> > loadMojo message.
> >
> > Mojo for WebUI design doc:
> >
>
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...
> >
> > BUG=567809
> >
> > Base URL: https://chromium.googlesource.com/chromium/src.git@master
> >
> > Affected files (+1102, -2 lines):
> > A + ios/third_party/requirejs/LICENSE
> > A + ios/third_party/requirejs/OWNERS
> > A ios/third_party/requirejs/README.chromium
> > A ios/third_party/requirejs/require.js
> >
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.
> 

lgtm for WebUI

[dpapad, 2016-04-28 21:28:07]

+rockot just as FYI.

@rockot: You mentioned in the past (IIRC) that Mojo team was not super happy
with define() and could possibly change in the future. Since IOS is going to
have a different define() implementation, transitioning away from it will be
more involved.

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/O...
File ios/third_party/requirejs/OWNERS (right):

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/O...
ios/third_party/requirejs/OWNERS:1: eugenebut@chromium.org
According to
https://www.chromium.org/developers/adding-3rd-party-libraries#TOC-Add-OWNERS,
there should be at least 2 owners.

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/R...
File ios/third_party/requirejs/README.chromium (right):

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/R...
ios/third_party/requirejs/README.chromium:3: Version: 2.1.22
Can we use the latest 2.2.0 version instead? See
http://requirejs.org/docs/release/2.2.0/comments/require.js and
https://github.com/requirejs/requirejs/blob/master/require.js

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/r...
File ios/third_party/requirejs/require.js (right):

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/r...
ios/third_party/requirejs/require.js:14:
goog.require('__crWeb.webUIModuleLoadNotifier');
Are those goog statements added on purpose? Is IOS using Closure Compiler?

[Eugene But (OOO till 7-30), 2016-04-29 18:04:42]

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/O...
File ios/third_party/requirejs/OWNERS (right):

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/O...
ios/third_party/requirejs/OWNERS:1: eugenebut@chromium.org
On 2016/04/28 21:28:07, dpapad wrote:
> According to
> https://www.chromium.org/developers/adding-3rd-party-libraries#TOC-Add-OWNERS,
> there should be at least 2 owners.
Jackie, are you willing to be an owner for this?

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/R...
File ios/third_party/requirejs/README.chromium (right):

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/R...
ios/third_party/requirejs/README.chromium:3: Version: 2.1.22
On 2016/04/28 21:28:07, dpapad wrote:
> Can we use the latest 2.2.0 version instead? See
> http://requirejs.org/docs/release/2.2.0/comments/require.js and
> https://github.com/requirejs/requirejs/blob/master/require.js
Sure. I merged changes from the new library. Most of them however do not apply
as forked version contain less than a half of original code.

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/r...
File ios/third_party/requirejs/require.js (right):

https://codereview.chromium.org/1931833004/diff/1/ios/third_party/requirejs/r...
ios/third_party/requirejs/require.js:14:
goog.require('__crWeb.webUIModuleLoadNotifier');
On 2016/04/28 21:28:07, dpapad wrote:
> Are those goog statements added on purpose? Is IOS using Closure Compiler?
Yes, here: https://codereview.chromium.org/1929783002/patch/60001/70010

[dpapad, 2016-04-29 21:02:22]

lgtm

[Eugene But (OOO till 7-30), 2016-04-29 21:03:19]

Thanks everyone!

[Eugene But (OOO till 7-30), 2016-04-29 21:03:25]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2016-04-29 21:03:26]

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/1931833004/#ps20001
(title: "Addressed review comments")

[dpapad, 2016-04-29 21:03:41]

https://codereview.chromium.org/1931833004/diff/20001/ios/third_party/require...
File ios/third_party/requirejs/README.chromium (right):

https://codereview.chromium.org/1931833004/diff/20001/ios/third_party/require...
ios/third_party/requirejs/README.chromium:12: and quality of your code.
Can you add a "Local modifications" section here mentioning briefly all the
stuff that has been removed? Similar to
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/chaijs....

[commit-bot: I haz the power, 2016-04-29 21:03:44]

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

[dpapad, 2016-04-29 21:05:49]

Actually I am revoking my LGTM for now, because it is not clear to me whether
the entire process described at
https://www.chromium.org/developers/adding-3rd-party-libraries has been
followed. Specifically, did you get an OK from chrome-eng reviewer?

[Eugene But (OOO till 7-30), 2016-04-29 21:15:32]

The CQ bit was unchecked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2016-04-29 21:42:57]

>> Specifically, did you get an OK from chrome-eng reviewer?
Yes. See the second message in this thread (LGTM for OSTPR).

https://codereview.chromium.org/1931833004/diff/20001/ios/third_party/require...
File ios/third_party/requirejs/README.chromium (right):

https://codereview.chromium.org/1931833004/diff/20001/ios/third_party/require...
ios/third_party/requirejs/README.chromium:12: and quality of your code.
On 2016/04/29 21:03:41, dpapad wrote:
> Can you add a "Local modifications" section here mentioning briefly all the
> stuff that has been removed? Similar to
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/chaijs....
Sure.

[Eugene But (OOO till 7-30), 2016-04-29 22:02:22]

Description was changed from

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

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

to

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Changes made in require.js:
 - changed code for loading the modules;
 - removed workarounds non needed for WebKit;
 - removed code which is not used for WebUI;

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

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

[Eugene But (OOO till 7-30), 2016-04-29 22:02:33]

eugenebut@chromium.org changed reviewers:
+ pinkerton@chromium.org, rsesek@chromium.org

[Eugene But (OOO till 7-30), 2016-04-29 22:02:34]

Mike, could you please do Chrome Eng Review for require.js, which is needed by
Mojo. CL description contains the design doc and explanation why the library is
needed.

Robert, could you please sign off on security.

Thanks!

[pink (ping after 24hrs), 2016-05-02 12:52:39]

You can bring it to chrome eng review yourself. Probably best for you to do
so because you know all the details. I'm on the review group so I can help
get any of their questions answered quickly.

On Fri, Apr 29, 2016 at 6:02 PM, <eugenebut@chromium.org> wrote:

> Mike, could you please do Chrome Eng Review for require.js, which is
> needed by
> Mojo. CL description contains the design doc and explanation why the
> library is
> needed.
>
> Robert, could you please sign off on security.
>
> Thanks!
>
> https://codereview.chromium.org/1931833004/
>



-- 
Mike Pinkerton
Mac Weenie
pinkerton@google.com

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Robert Sesek, 2016-05-02 22:00:36]

Please take this to eng review first, and then email security@chromium.org for
3p inclusion.

[Robert Sesek, 2016-05-02 22:00:49]

rsesek@chromium.org changed reviewers:
- rsesek@chromium.org

[brettw, 2016-05-03 18:06:59]

brettw@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2016-05-03 18:07:00]

LGTM from eng-review. You should get legal sign-off before landing. You can
probably land before final security sign-off as long as it's not shipped yet.

[brettw, 2016-05-03 18:12:38]

On 2016/05/03 18:07:00, brettw wrote:
> LGTM from eng-review. You should get legal sign-off before landing. You can
> probably land before final security sign-off as long as it's not shipped yet.

Ah, OSTPR is the legal one, so you should be OK there.

[Martin Barbella, 2016-05-05 21:15:42]

mbarbella@chromium.org changed reviewers:
+ mbarbella@chromium.org

[Martin Barbella, 2016-05-05 21:15:42]

Still trying to find a security reviewer, but I'll take a look tomorrow if no
one else steps up. I'm not the best reviewer for js.

https://codereview.chromium.org/1931833004/diff/40001/ios/third_party/require...
File ios/third_party/requirejs/README.chromium (right):

https://codereview.chromium.org/1931833004/diff/40001/ios/third_party/require...
ios/third_party/requirejs/README.chromium:6: Security Critical: no
If this ships, it's security critical.

[Eugene But (OOO till 7-30), 2016-05-05 21:27:01]

https://codereview.chromium.org/1931833004/diff/40001/ios/third_party/require...
File ios/third_party/requirejs/README.chromium (right):

https://codereview.chromium.org/1931833004/diff/40001/ios/third_party/require...
ios/third_party/requirejs/README.chromium:6: Security Critical: no
On 2016/05/05 21:15:42, Martin Barbella wrote:
> If this ships, it's security critical.

Done.

[Martin Barbella, 2016-05-06 22:08:45]

LGTM from security.

I don't think that including this library has any inherent problems, but the
feature using it will still need a security review of its own. I should point
out that local modifications do make it more difficult to uptake fixes, but in
this case it seems reasonable since it mostly sounds like code removal (which is
a win), and I think this is fairly low-risk.

[Eugene But (OOO till 7-30), 2016-05-06 22:12:32]

Thanks everyone!

[Eugene But (OOO till 7-30), 2016-05-06 22:12:39]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2016-05-06 22:12:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
jyquinn@chromium.org, dpapad@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1931833004/#ps60001
(title: "Marked security critical.")

[commit-bot: I haz the power, 2016-05-06 22:12:59]

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

[commit-bot: I haz the power, 2016-05-06 23:01:12]

Description was changed from

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Changes made in require.js:
 - changed code for loading the modules;
 - removed workarounds non needed for WebKit;
 - removed code which is not used for WebUI;

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

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

to

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Changes made in require.js:
 - changed code for loading the modules;
 - removed workarounds non needed for WebKit;
 - removed code which is not used for WebUI;

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

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

[commit-bot: I haz the power, 2016-05-06 23:01:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-06 23:03:12]

Description was changed from

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Changes made in require.js:
 - changed code for loading the modules;
 - removed workarounds non needed for WebKit;
 - removed code which is not used for WebUI;

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

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

to

==========
[ios Mojo] Forked version of require.js needed for WebUI Mojo.

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 require.js file is a forked and stripped down version of open-source
require.js. Fetching the scripts is delegated to native code by sending
loadMojo message.

Changes made in require.js:
 - changed code for loading the modules;
 - removed workarounds non needed for WebKit;
 - removed code which is not used for WebUI;

Mojo for WebUI design doc:
https://docs.google.com/document/d/1iUJXX_zoY2BbnecpfxjznOeasf29KUzl_9S_LG08B...

BUG=567809

Committed: https://crrev.com/703f8f2899ceccc0c05d05b6e3930b2e6c23aeb9
Cr-Commit-Position: refs/heads/master@{#392194}
==========

[commit-bot: I haz the power, 2016-05-06 23:03:14]

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