Add on_startup_page/compiled_resources.gyp

Add on_startup_page/compiled_resources.gyp

Includes Some minor fixes to the JS

BUG=none

Committed: https://crrev.com/b49dedaca74c27f728c2e0365d540c59302ce41b
Cr-Commit-Position: refs/heads/master@{#348790}

[stevenjb, 2015-09-11 17:53:23]

stevenjb@chromium.org changed reviewers:
+ dschuyler@chromium.org

[stevenjb, 2015-09-14 19:37:07]

PTAL

[dschuyler, 2015-09-14 21:08:28]

https://codereview.chromium.org/1341463002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js
(right):

https://codereview.chromium.org/1341463002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js:163:
return;
This seems like something different from the other files in the CL.  I'm missing
how this relates to on_startup settings.

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp
(right):

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp:23:
'depends': [
I don't know closure well enough to review this imo.  I am getting the code
build it, as a kind of sanity check.

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/search_page/search_page.js (right):

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/search_page/search_page.js:27: type: Array,
After chatting with Tommy, he suggested this should be an Object.
See
https://codereview.chromium.org/1321033004/diff/1/chrome/browser/resources/se...

[dschuyler, 2015-09-14 21:10:13]

https://codereview.chromium.org/1341463002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js
(right):

https://codereview.chromium.org/1341463002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js:163:
return;
On 2015/09/14 21:08:28, dschuyler wrote:
> This seems like something different from the other files in the CL.  I'm
missing
> how this relates to on_startup settings.

Please ignore this.  This file was removed after the comment was made.

[dschuyler, 2015-09-14 21:46:52]

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp
(right):

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp:23:
'depends': [
On 2015/09/14 21:08:28, dschuyler wrote:
> I don't know closure well enough to review this imo.  I am getting the code
> build it, as a kind of sanity check.

I've patched with this CL and tried out the on_startup page and ran the closure
compiler.  It looks good from that point of view.  I also read some about the
closure compiler to see what depends, externs, and includes were about.  It all
looks ok as far as I see.

[dschuyler, 2015-09-14 21:50:20]

On 2015/09/14 21:46:52, dschuyler wrote:
>
https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
> File chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp
> (right):
> 
>
https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
> chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp:23:
> 'depends': [
> On 2015/09/14 21:08:28, dschuyler wrote:
> > I don't know closure well enough to review this imo.  I am getting the code
> > build it, as a kind of sanity check.
> 
> I've patched with this CL and tried out the on_startup page and ran the
closure
> compiler.  It looks good from that point of view.  I also read some about the
> closure compiler to see what depends, externs, and includes were about.  It
all
> looks ok as far as I see.

LGTM

[dschuyler, 2015-09-14 21:50:57]

On 2015/09/14 21:50:20, dschuyler wrote:
> On 2015/09/14 21:46:52, dschuyler wrote:
> >
>
https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
> > File
chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp
> > (right):
> > 
> >
>
https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
> > chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp:23:
> > 'depends': [
> > On 2015/09/14 21:08:28, dschuyler wrote:
> > > I don't know closure well enough to review this imo.  I am getting the
code
> > > build it, as a kind of sanity check.
> > 
> > I've patched with this CL and tried out the on_startup page and ran the
> closure
> > compiler.  It looks good from that point of view.  I also read some about
the
> > closure compiler to see what depends, externs, and includes were about.  It
> all
> > looks ok as far as I see.
> 
> LGTM

Err, with the nit to follow-up on the route Object vs Array thing with Tommy.

[stevenjb, 2015-09-14 23:12:34]

stevenjb@chromium.org changed reviewers:
+ tommycli@chromium.org

[stevenjb, 2015-09-14 23:12:35]

PTAL
+tommycli@

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp
(right):

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp:23:
'depends': [
On 2015/09/14 21:08:28, dschuyler wrote:
> I don't know closure well enough to review this imo.  I am getting the code
> build it, as a kind of sanity check.

Acknowledged.

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/on_startup_page/compiled_resources.gyp:23:
'depends': [
On 2015/09/14 21:46:52, dschuyler wrote:
> On 2015/09/14 21:08:28, dschuyler wrote:
> > I don't know closure well enough to review this imo.  I am getting the code
> > build it, as a kind of sanity check.
> 
> I've patched with this CL and tried out the on_startup page and ran the
closure
> compiler.  It looks good from that point of view.  I also read some about the
> closure compiler to see what depends, externs, and includes were about.  It
all
> looks ok as far as I see.

Acknowledged.

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/search_page/search_page.js (right):

https://codereview.chromium.org/1341463002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/search_page/search_page.js:27: type: Array,
On 2015/09/14 21:08:28, dschuyler wrote:
> After chatting with Tommy, he suggested this should be an Object.
> See
>
https://codereview.chromium.org/1321033004/diff/1/chrome/browser/resources/se...
> 

Yes, you're right, this came up elsewhere, I was propagating the wrong change.
Fixed throughout.

[tommycli, 2015-09-14 23:16:18]

stevenjb: Cool. Assuming this compiles with clojure, LGTM. Good to see we are
starting to clojure this code.

[stevenjb, 2015-09-14 23:19:12]

On 2015/09/14 23:16:18, tommycli wrote:
> stevenjb: Cool. Assuming this compiles with clojure, LGTM. Good to see we are
> starting to clojure this code.

It does, that was indeed the point :)
Cheers.

[stevenjb, 2015-09-14 23:19:15]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2015-09-14 23:19:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org
Link to the patchset: https://codereview.chromium.org/1341463002/#ps60001
(title: "currentRoute -> Object")

[commit-bot: I haz the power, 2015-09-14 23:20:59]

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

[commit-bot: I haz the power, 2015-09-15 00:36:26]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-15 00:37:06]

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

[commit-bot: I haz the power, 2015-09-23 12:40:15]

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