Settings: Fix Site Details subpage routing

Goal is: A site details subpage that's accessible both from /siteSettings/all, /siteSettings/bluetooth, /siteSettings/plugins, etc. (all the categories).

Previous way it was done was: Separate route for each category, so there was /siteSettings/all/details, /siteSettings/bluetooth/details, /siteSettings/plugins/details.

And it relied on a quirk of how settings-animated-pages used to work to coalesce all those routes to a single card.

New way I'm proposing: One single /siteSettings/siteDetails route.

But here are some extra needed changes:
 - settings-animated-pages must slide right / left based on route depth instead of based on .contains()
 - subpage-back button needs to call window.history.back() whenever there is a previous route, even if the previous route doesn't strictly contain the current route.

I think the above two changes are harmless and the new version is overall an improvement.

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

Review-Url: https://codereview.chromium.org/2249873003
Cr-Commit-Position: refs/heads/master@{#412605}

chrome/browser/resources/settings/privacy_page/privacy_page.html

@@ -225,7 +225,7 @@
           </site-settings-category>
         </settings-subpage>
       </template>
-      <template is="dom-if" name="protocol-handlers" no-search>
+      <template is="dom-if" route-path="/siteSettings/handlers" no-search>
         <settings-subpage page-title="$i18n{siteSettingsCategoryHandlers}">
           <protocol-handlers></protocol-handlers>
         </settings-subpage>
@@ -292,8 +292,7 @@
           <usb-devices></usb-devices>
         </settings-subpage>
       </template>
-
-      <template is="dom-if" name="site-details" no-search>
+      <template is="dom-if" route-path="/siteSettings/siteDetails" no-search>
         <settings-subpage page-title="[[selectedSite.originForDisplay]]">
           <site-details site="[[selectedSite]]"></site-details>
         </settings-subpage>

chrome/browser/resources/settings/route.js

@@ -15,6 +15,9 @@ cr.define('settings', function() {
     /** @type {?settings.Route} */
     this.parent = null;
 
+    /** @type {number} */
+    this.depth = 0;
+
     // Below are all legacy properties to provide compatibility with the old
     // routing system. TODO(tommycli): Remove once routing refactor complete.
     this.section = '';
@@ -38,6 +41,7 @@ cr.define('settings', function() {
       var route = new Route(newUrl);
       route.parent = this;
       route.section = this.section;
+      route.depth = this.depth + 1;
 
       return route;
     },
@@ -131,7 +135,8 @@ cr.define('settings', function() {
 
   r.SITE_SETTINGS = r.PRIVACY.createChild('/siteSettings');
   r.SITE_SETTINGS_ALL = r.SITE_SETTINGS.createChild('all');
-  r.SITE_SETTINGS_ALL_DETAILS = r.SITE_SETTINGS_ALL.createChild('details');
+  r.SITE_SETTINGS_SITE_DETAILS =
+      r.SITE_SETTINGS_ALL.createChild('/siteSettings/siteDetails');
 
   r.SITE_SETTINGS_HANDLERS = r.SITE_SETTINGS.createChild('handlers');
 
@@ -154,33 +159,6 @@ cr.define('settings', function() {
       r.SITE_SETTINGS.createChild('unsandboxedPlugins');
   r.SITE_SETTINGS_USB_DEVICES = r.SITE_SETTINGS.createChild('usbDevices');
 
-  r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS_DETAILS =
-      r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS.createChild('details');
-  r.SITE_SETTINGS_BACKGROUND_SYNC_DETAILS =
-      r.SITE_SETTINGS_BACKGROUND_SYNC.createChild('details');
-  r.SITE_SETTINGS_CAMERA_DETAILS =
-      r.SITE_SETTINGS_CAMERA.createChild('details');
-  r.SITE_SETTINGS_COOKIES_DETAILS =
-      r.SITE_SETTINGS_COOKIES.createChild('details');
-  r.SITE_SETTINGS_IMAGES_DETAILS =
-      r.SITE_SETTINGS_IMAGES.createChild('details');
-  r.SITE_SETTINGS_JAVASCRIPT_DETAILS =
-      r.SITE_SETTINGS_JAVASCRIPT.createChild('details');
-  r.SITE_SETTINGS_KEYGEN_DETAILS =
-      r.SITE_SETTINGS_KEYGEN.createChild('details');
-  r.SITE_SETTINGS_LOCATION_DETAILS =
-      r.SITE_SETTINGS_LOCATION.createChild('details');
-  r.SITE_SETTINGS_MICROPHONE_DETAILS =
-      r.SITE_SETTINGS_MICROPHONE.createChild('details');
-  r.SITE_SETTINGS_NOTIFICATIONS_DETAILS =
-      r.SITE_SETTINGS_NOTIFICATIONS.createChild('details');
-  r.SITE_SETTINGS_PLUGINS_DETAILS =
-      r.SITE_SETTINGS_PLUGINS.createChild('details');
-  r.SITE_SETTINGS_POPUPS_DETAILS =
-      r.SITE_SETTINGS_POPUPS.createChild('details');
-  r.SITE_SETTINGS_UNSANDBOXED_PLUGINS_DETAILS =
-      r.SITE_SETTINGS_UNSANDBOXED_PLUGINS.createChild('details');
-
 <if expr="chromeos">
   r.DATETIME = r.ADVANCED.createSection('/dateTime', 'dateTime');
 

chrome/browser/resources/settings/settings_page/settings_animated_pages.js

@@ -92,12 +92,12 @@ Polymer({
     this.ensureSubpageInstance_();
 
     if (oldRoute) {
-      if (oldRoute.isSubpage() && oldRoute.contains(newRoute)) {
-        // Slide left for a descendant subpage.
+      if (oldRoute.isSubpage() && newRoute.depth > oldRoute.depth) {
+        // Slide left for a deeper subpage.
         this.$.animatedPages.exitAnimation = 'slide-left-animation';
         this.$.animatedPages.entryAnimation = 'slide-from-right-animation';
-      } else if (newRoute.contains(oldRoute)) {
-        // Slide right for an ancestor subpage.
+      } else if (oldRoute.depth > newRoute.depth) {
+        // Slide right for a shallower subpage.
         this.$.animatedPages.exitAnimation = 'slide-right-animation';
         this.$.animatedPages.entryAnimation = 'slide-from-left-animation';
       } else {

chrome/browser/resources/settings/settings_page/settings_subpage.js

@@ -48,7 +48,7 @@ Polymer({
         assert(settings.getRouteForPath(
             /** @type {string} */ (window.history.state)));
 
-    if (previousRoute && previousRoute.contains(settings.getCurrentRoute()))
+    if (previousRoute)
       window.history.back();
     else
       settings.navigateTo(assert(settings.getCurrentRoute().parent));

chrome/browser/resources/settings/site_settings/site_list.js

@@ -402,10 +402,7 @@ Polymer({
     if (this.isPolicyControlled_(this.selectedSite.source))
       return;
 
-    if (this.allSites)
-      settings.navigateTo(settings.Route.SITE_SETTINGS_ALL_DETAILS);
-    else
-      settings.navigateTo(this.computeCategoryDetailsRoute(this.category));
+    settings.navigateTo(settings.Route.SITE_SETTINGS_SITE_DETAILS);
   },
 
   /**

chrome/browser/resources/settings/site_settings/site_settings_behavior.js

@@ -149,44 +149,6 @@ var SiteSettingsBehaviorImpl = {
     assertNotReached();
   },
 
-  /**
-   * A utility function to lookup the 'details' route for a category name.
-   * @param {string} category The category ID to look up.
-   * @return {!settings.Route}
-   * @protected
-   */
-  computeCategoryDetailsRoute: function(category) {
-    switch (category) {
-      case settings.ContentSettingsTypes.AUTOMATIC_DOWNLOADS:
-        return settings.Route.SITE_SETTINGS_AUTOMATIC_DOWNLOADS_DETAILS;
-      case settings.ContentSettingsTypes.BACKGROUND_SYNC:
-        return settings.Route.SITE_SETTINGS_BACKGROUND_SYNC_DETAILS;
-      case settings.ContentSettingsTypes.CAMERA:
-        return settings.Route.SITE_SETTINGS_CAMERA_DETAILS;
-      case settings.ContentSettingsTypes.COOKIES:
-        return settings.Route.SITE_SETTINGS_COOKIES_DETAILS;
-      case settings.ContentSettingsTypes.GEOLOCATION:
-        return settings.Route.SITE_SETTINGS_LOCATION_DETAILS;
-      case settings.ContentSettingsTypes.IMAGES:
-        return settings.Route.SITE_SETTINGS_IMAGES_DETAILS;
-      case settings.ContentSettingsTypes.JAVASCRIPT:
-        return settings.Route.SITE_SETTINGS_JAVASCRIPT_DETAILS;
-      case settings.ContentSettingsTypes.KEYGEN:
-        return settings.Route.SITE_SETTINGS_KEYGEN_DETAILS;
-      case settings.ContentSettingsTypes.MIC:
-        return settings.Route.SITE_SETTINGS_MICROPHONE_DETAILS;
-      case settings.ContentSettingsTypes.NOTIFICATIONS:
-        return settings.Route.SITE_SETTINGS_NOTIFICATIONS_DETAILS;
-      case settings.ContentSettingsTypes.PLUGINS:
-        return settings.Route.SITE_SETTINGS_PLUGINS_DETAILS;
-      case settings.ContentSettingsTypes.POPUPS:
-        return settings.Route.SITE_SETTINGS_POPUPS_DETAILS;
-      case settings.ContentSettingsTypes.UNSANDBOXED_PLUGINS:
-        return settings.Route.SITE_SETTINGS_UNSANDBOXED_PLUGINS_DETAILS;
-    }
-    assertNotReached();
-  },
-
   /**
    * A utility function to compute the icon to use for the category, both for
    * the overall category as well as the individual permission in the details

chrome/test/data/webui/settings/route_tests.js

@@ -6,13 +6,16 @@ suite('route', function() {
   test('tree structure', function() {
     // Set up root page routes.
     var BASIC = new settings.Route('/');
+    assertEquals(0, BASIC.depth);
+
     var ADVANCED = new settings.Route('/advanced');
     assertFalse(ADVANCED.isSubpage());
+    assertEquals(0, ADVANCED.depth);
 
     // Test a section route.
-    var PRIVACY = ADVANCED.createChild('/privacy');
-    PRIVACY.section = 'privacy';
+    var PRIVACY = ADVANCED.createSection('/privacy', 'privacy');
     assertEquals(ADVANCED, PRIVACY.parent);
+    assertEquals(1, PRIVACY.depth);
     assertFalse(PRIVACY.isSubpage());
     assertFalse(BASIC.contains(PRIVACY));
     assertTrue(ADVANCED.contains(PRIVACY));
@@ -23,6 +26,7 @@ suite('route', function() {
     var SITE_SETTINGS = PRIVACY.createChild('/siteSettings');
     assertEquals('/siteSettings', SITE_SETTINGS.path);
     assertEquals(PRIVACY, SITE_SETTINGS.parent);
+    assertEquals(2, SITE_SETTINGS.depth);
     assertFalse(!!SITE_SETTINGS.dialog);
     assertTrue(SITE_SETTINGS.isSubpage());
     assertEquals('privacy', SITE_SETTINGS.section);
@@ -34,6 +38,7 @@ suite('route', function() {
     var SITE_SETTINGS_ALL = SITE_SETTINGS.createChild('all');
     assertEquals('/siteSettings/all', SITE_SETTINGS_ALL.path);
     assertEquals(SITE_SETTINGS, SITE_SETTINGS_ALL.parent);
+    assertEquals(3, SITE_SETTINGS_ALL.depth);
     assertTrue(SITE_SETTINGS_ALL.isSubpage());
   });
 

chrome/test/data/webui/settings/settings_subpage_test.js

@@ -5,11 +5,13 @@
 cr.define('settings_subpage', function() {
   function registerTests() {
     suite('SettingsSubpage', function() {
-      test('can navigate to parent', function() {
+      test('navigates to parent when there is no history', function() {
         PolymerTest.clearBody();
 
-        // Choose CERTIFICATES since it is not a descendant of BASIC.
-        settings.navigateTo(settings.Route.CERTIFICATES);
+        // Pretend that we initially started on the CERTIFICATES route.
+        window.history.replaceState(
+            undefined, '', settings.Route.CERTIFICATES.path);
+        settings.initializeRouteFromUrl();
         assertEquals(settings.Route.CERTIFICATES, settings.getCurrentRoute());
 
         var subpage = document.createElement('settings-subpage');
@@ -19,7 +21,7 @@ cr.define('settings_subpage', function() {
         assertEquals(settings.Route.PRIVACY, settings.getCurrentRoute());
       });
 
-      test('can navigate to grandparent using window.back()', function(done) {
+      test('navigates to any route via window.back()', function(done) {
         PolymerTest.clearBody();
 
         settings.navigateTo(settings.Route.BASIC);
@@ -31,8 +33,6 @@ cr.define('settings_subpage', function() {
 
         MockInteractions.tap(subpage.$$('paper-icon-button'));
 
-        // Since the previous history entry is an ancestor, we expect
-        // window.history.back() to be called and a popstate event to be fired.
         window.addEventListener('popstate', function(event) {
           assertEquals(settings.Route.BASIC, settings.getCurrentRoute());
           done();