Settings Router Refactor: Kill settings-router.

Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOPRESUBMIT=true

Committed: https://crrev.com/5b6b548e6ebc2374818b61e14cd315add717f71a
Cr-Commit-Position: refs/heads/master@{#410228}

[tommycli, 2016-08-04 20:36:23]

Description was changed from

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
==========

to

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tommycli, 2016-08-04 21:24:55]

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

[commit-bot: I haz the power, 2016-08-04 21:25:29]

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-04 21:28:49]

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

[commit-bot: I haz the power, 2016-08-04 21:28:50]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tommycli, 2016-08-04 23:10:49]

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

[commit-bot: I haz the power, 2016-08-04 23:11:29]

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-04 23:14:16]

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

[commit-bot: I haz the power, 2016-08-04 23:14:18]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tommycli, 2016-08-04 23:17:13]

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

[commit-bot: I haz the power, 2016-08-04 23:17:38]

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-04 23:19:37]

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

[commit-bot: I haz the power, 2016-08-04 23:19:38]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[tommycli, 2016-08-04 23:20:52]

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

[commit-bot: I haz the power, 2016-08-04 23:21:23]

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-04 23:29:06]

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

[commit-bot: I haz the power, 2016-08-04 23:29:07]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[tommycli, 2016-08-04 23:38:10]

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

[commit-bot: I haz the power, 2016-08-04 23:38:38]

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-04 23:41:50]

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

[commit-bot: I haz the power, 2016-08-04 23:41:51]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tommycli, 2016-08-04 23:46:05]

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

[commit-bot: I haz the power, 2016-08-04 23:46:21]

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

[tommycli, 2016-08-04 23:52:03]

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

[tommycli, 2016-08-04 23:52:05]

dbeam: PTAL, thanks!

[commit-bot: I haz the power, 2016-08-04 23:53:13]

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

[commit-bot: I haz the power, 2016-08-04 23:53:14]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Dan Beam, 2016-08-05 18:25:19]

lgtm enjoy your conflicts!

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/device_page/device_page.js (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/device_page/device_page.js:116: /** @protected
*/
can this be @override instead?

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/people_page/people_page.html (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/people_page.html:239:
<settings-sync-page>
nit: unwrap

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/people_page.html:247:
set-modes="{{quickUnlockSetModes}}" profile-name="[[profileName_]]">
nit: indent off

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/people_page/quick_unlock_authenticate.js
(right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/quick_unlock_authenticate.js:20: *
  profile-name="[[profileName_]]">
nit: example indent off ;)

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/people_page/quick_unlock_choose_method.js
(right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/quick_unlock_choose_method.js:76:
if (settings.getCurrentRoute() == settings.Route.QUICK_UNLOCK_CHOOSE_METHOD)
indent off

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/privacy_page/privacy_page.js (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/privacy_page/privacy_page.js:39:
currentRouteChanged: function() {
can this be called at a time where |route| would be falsy?

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_main/settings_main.js:164: var route
= settings.getCurrentRoute();
can |route| be falsy?

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_page/settings_animated_pages.js
(left):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_page/settings_animated_pages.js:49:
settings.navigateTo(this.currentRoute.parent);
all that compilation safety... and we got foiled by null

[michaelpg, 2016-08-05 18:39:28]

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_page/settings_animated_pages.js
(left):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_page/settings_animated_pages.js:49:
settings.navigateTo(this.currentRoute.parent);
On 2016/08/05 18:25:19, Dan Beam wrote:
> all that compilation safety... and we got foiled by null

how do you even fire this w/out a parent? that sounds like a bug in itself

[tommycli, 2016-08-05 18:48:36]

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

[commit-bot: I haz the power, 2016-08-05 18:49:07]

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

[tommycli, 2016-08-05 18:54:06]

dbeam: thanks!

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/device_page/device_page.js (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/device_page/device_page.js:116: /** @protected
*/
On 2016/08/05 18:25:18, Dan Beam wrote:
> can this be @override instead?

Unfortunately, no. RouteObserverBehavior is not detected as a superclass of this
element.

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/people_page/people_page.html (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/people_page.html:239:
<settings-sync-page>
On 2016/08/05 18:25:18, Dan Beam wrote:
> nit: unwrap

Done.

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/people_page.html:247:
set-modes="{{quickUnlockSetModes}}" profile-name="[[profileName_]]">
On 2016/08/05 18:25:18, Dan Beam wrote:
> nit: indent off

Done.

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/people_page/quick_unlock_authenticate.js
(right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/quick_unlock_authenticate.js:20: *
  profile-name="[[profileName_]]">
On 2016/08/05 18:25:18, Dan Beam wrote:
> nit: example indent off ;)

Done.

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/people_page/quick_unlock_choose_method.js
(right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/people_page/quick_unlock_choose_method.js:76:
if (settings.getCurrentRoute() == settings.Route.QUICK_UNLOCK_CHOOSE_METHOD)
On 2016/08/05 18:25:19, Dan Beam wrote:
> indent off

Done.

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/privacy_page/privacy_page.js (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/privacy_page/privacy_page.js:39:
currentRouteChanged: function() {
On 2016/08/05 18:25:19, Dan Beam wrote:
> can this be called at a time where |route| would be falsy?

Done. |route| is actually never falsy now. It's defined as BASIC and then set
from the URL:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/route....

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_main/settings_main.js:164: var route
= settings.getCurrentRoute();
On 2016/08/05 18:25:19, Dan Beam wrote:
> can |route| be falsy?

Done.

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_page/settings_animated_pages.js
(left):

https://codereview.chromium.org/2210933004/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_page/settings_animated_pages.js:49:
settings.navigateTo(this.currentRoute.parent);
On 2016/08/05 18:39:28, michaelpg wrote:
> On 2016/08/05 18:25:19, Dan Beam wrote:
> > all that compilation safety... and we got foiled by null
> 
> how do you even fire this w/out a parent? that sounds like a bug in itself

Done. It could be hypothetically fired when the user is on the BASIC page. But
it shouldn't. I made it an assert.

I'm just going to axe this event in a followup CL.

[commit-bot: I haz the power, 2016-08-05 18:57:18]

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

[commit-bot: I haz the power, 2016-08-05 18:57:20]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[tommycli, 2016-08-05 19:01:27]

Description was changed from

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOPRESUBMIT=true
==========

[tommycli, 2016-08-05 19:05:36]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2016-08-05 19:05:38]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2210933004/#ps160001
(title: "address comments and merge")

[commit-bot: I haz the power, 2016-08-05 19:06:20]

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-05 19:39:58]

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

[commit-bot: I haz the power, 2016-08-05 19:40:03]

Failed to apply patch for
chrome/browser/resources/settings/internet_page/internet_page.html:
While running git apply --index -3 -p1;
  error: patch failed:
chrome/browser/resources/settings/internet_page/internet_page.html:12
  Falling back to three-way merge...
  Applied patch to
'chrome/browser/resources/settings/internet_page/internet_page.html' with
conflicts.
  U chrome/browser/resources/settings/internet_page/internet_page.html

Patch:       chrome/browser/resources/settings/internet_page/internet_page.html
Index: chrome/browser/resources/settings/internet_page/internet_page.html
diff --git a/chrome/browser/resources/settings/internet_page/internet_page.html
b/chrome/browser/resources/settings/internet_page/internet_page.html
index
e39eb35a69a4096cf5bce9e1008921221dc455dd..aea3ba18010188a875a5647cfe8d05b9683e1b10
100644
--- a/chrome/browser/resources/settings/internet_page/internet_page.html
+++ b/chrome/browser/resources/settings/internet_page/internet_page.html
@@ -12,8 +12,7 @@
 <dom-module id="settings-internet-page">
   <template>
     <style include="settings-shared"></style>
-    <settings-animated-pages id="pages" current-route="{{currentRoute}}"
-        section="internet">
+    <settings-animated-pages id="pages" section="internet">
       <neon-animatable id="main">
         <network-summary on-show-detail="onShowDetail_"
             default-network="{{defaultNetwork}}"

[michaelpg, 2016-08-05 19:43:42]

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

[michaelpg, 2016-08-05 19:43:43]

Doesn't seem to break anything (that's not already broken) -- no console errors
or anything like that. :)

[michaelpg, 2016-08-05 19:43:54]

On 2016/08/05 19:43:43, michaelpg wrote:
> Doesn't seem to break anything (that's not already broken) -- no console
errors
> or anything like that. :)

& lgtm

[tommycli, 2016-08-05 22:10:14]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2016-08-05 22:10:15]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org,
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2210933004/#ps200001
(title: "fix merge")

[commit-bot: I haz the power, 2016-08-05 22:10: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-06 00:27:17]

Description was changed from

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOPRESUBMIT=true
==========

to

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOPRESUBMIT=true
==========

[commit-bot: I haz the power, 2016-08-06 00:27:20]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-08-06 00:29:08]

Description was changed from

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOPRESUBMIT=true
==========

to

==========
Settings Router Refactor: Kill settings-router.

Use RouteObserverBehavior everywhere instead.

BUG=632382
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOPRESUBMIT=true

Committed: https://crrev.com/5b6b548e6ebc2374818b61e14cd315add717f71a
Cr-Commit-Position: refs/heads/master@{#410228}
==========

[commit-bot: I haz the power, 2016-08-06 00:29:10]

Patchset 11 (id:??) landed as
https://crrev.com/5b6b548e6ebc2374818b61e14cd315add717f71a
Cr-Commit-Position: refs/heads/master@{#410228}