MD Settings: methods to get root pages and sections

MD Settings: methods to get root pages and sections

BUG=634582
R=dschuyler@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[michaelpg, 2016-08-10 03:50:37]

Description was changed from

==========
MD Settings: methods to get root pages and sections

BUG=634582
R=dschuyler@chromium.org
==========

to

==========
MD Settings: methods to get root pages and sections

BUG=634582
R=dschuyler@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-08-10 03:53:01]

PTAL -- some minor ergonomic cleanup.

[dschuyler, 2016-08-10 17:16:18]

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_main/settings_main.js:191: getPage_:
function(route) {
I'd like to hear more about why this function helps.

It seems to query the about page with each call
(which is unlikely to have the element in question).

Is it useful to separate the data types of the return
values?

It also seems like a lot of extra code to read.

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_page/main_page_behavior.js
(right):

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_page/main_page_behavior.js:243:
this.$$('[section=' + section + ']'));
Removing the query from the other file loses the 
quotes in the section= query. Let's add them here
if we are removing the other:
'settings-section[section="' + route.section + '"]'
They are needed if we want to search for the empty
section ("").

[michaelpg, 2016-08-15 16:41:58]

The CQ bit was checked by michaelpg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-15 16:42:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-15 17:43:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-15 17:43:12]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2016-08-15 18:08:11]

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_main/settings_main.js:191: getPage_:
function(route) {
On 2016/08/10 17:16:17, dschuyler wrote:
> I'd like to hear more about why this function helps.
> 
> It seems to query the about page with each call
> (which is unlikely to have the element in question).

No, it only queries the page for the current route -- so if the current route is
or descends from ABOUT, it will return the about page.

> 
> Is it useful to separate the data types of the return
> values?

Separate, no; have, yes. The type is useful because MainPageBehavior has
functions like getSection which are useful to settings-main (rather than
re-implementing them here). We have to specify all the types because Closure
isn't smart enough to accept a behavior as a type.

> 
> It also seems like a lot of extra code to read.


We could try duplicating all of the public member docs from MainPageBehavior
onto an @interface, and claim to Closure that each of our main pages @implements
that interface, but that would be more duplication.

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_page/main_page_behavior.js
(right):

https://codereview.chromium.org/2230753002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_page/main_page_behavior.js:243:
this.$$('[section=' + section + ']'));
On 2016/08/10 17:16:17, dschuyler wrote:
> Removing the query from the other file loses the 
> quotes in the section= query. Let's add them here
> if we are removing the other:
> 'settings-section[section="' + route.section + '"]'
> They are needed if we want to search for the empty
> section ("").

Done.

[michaelpg, 2016-08-17 22:32:32]

Patchset #3 (id:40001) has been deleted

[michaelpg, 2016-08-17 22:32:43]

friendly ping, blocking other lg'd CLs

[dschuyler, 2016-08-18 01:16:03]

lgtm

[michaelpg, 2016-08-18 21:00:35]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-08-18 21:00:35]

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/2230753002/#ps60001
(title: "defensive rebase")

[commit-bot: I haz the power, 2016-08-18 21:01:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-18 23:16:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 23:16:02]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[michaelpg, 2016-08-18 23:43:29]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2016-08-18 23:44:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 02:05:39]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-08-19 02:09:42]

Description was changed from

==========
MD Settings: methods to get root pages and sections

BUG=634582
R=dschuyler@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: methods to get root pages and sections

BUG=634582
R=dschuyler@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-08-19 02:09:43]

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