Settings Router Refactor: Replace route.subpage usage with route.path

chrome/browser/resources/settings/route.js

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('settings', function() {
   /**
    * Class for navigable routes. May only be instantiated within this file.
    * @constructor
    * @param {string} path
    * @private
    */
   var Route = function(path) {
     this.path = path;
 
     /** @type {?settings.Route} */
     this.parent = null;
 
     // Below are all legacy properties to provide compatibility with the old
     // routing system. TODO(tommycli): Remove once routing refactor complete.
     this.section = '';
-    /** @type {!Array<string>} */ this.subpage = [];
   };
 
   Route.prototype = {
     /**
      * Returns a new Route instance that's a child of this route.
      * @param {string} path Extends this route's path if it doesn't contain a
      *     leading slash.
-     * @param {string=} opt_subpageName
      * @return {!settings.Route}
      * @private
      */
-    createChild: function(path, opt_subpageName) {
+    createChild: function(path) {
       assert(path);
 
       // |path| extends this route's path if it doesn't have a leading slash.
       // If it does have a leading slash, it's just set as the new route's URL.
       var newUrl = path[0] == '/' ? path : this.path + '/' + path;
 
       var route = new Route(newUrl);
       route.parent = this;
       route.section = this.section;
-      route.subpage = this.subpage.slice();  // Shallow copy.
-
-      if (opt_subpageName)
-        route.subpage.push(opt_subpageName);
 
       return route;
     },
 
     /**
      * Returns a new Route instance that's a child dialog of this route.
      * @param {string} path
      * @param {string} dialogName
      * @return {!settings.Route}
      * @private
@@ skipping 23 matching lines @@
      * @param {!settings.Route} route
      * @return {boolean}
      */
     contains: function(route) {
       for (var r = route; r != null; r = r.parent) {
         if (this == r)
           return true;
       }
       return false;
     },
+
+    /**
+     * Returns true if this route is a subpage of a section (but not a dialog).
+     * @return {boolean}
+     */
+    isSubpage: function() {
+      return !this.dialog && !!this.parent &&

[michaelpg, 2016/08/11 03:41:13]

I suggest removing the !this.dialog check (and comment): if dialog routes are
never in sections, then it's redundant. Removing the check means we're still
safe if we *do* add a subpage dialog -- the transitions won't get confused and
broken.

IOW: if we don't have navigable subpage dialogs, `!this.dialog &&` has no
effect; if we do, things break. That was my original point, anyway -- we're
replacing subpage.length > 0 with isSubpage(), so I'd prefer if they stayed
equal in this CL.

[tommycli, 2016/08/11 19:15:02]

Removing !this.dialog doesn't work, since we changed the isSubpage test from
looking at the length of the subpage array to checking if the current route's
section is equal to the parent route's section.

In the latest patchset, I just removed the concept of a dialog route and special
cased the CLEAR_BROWSING_DATA instance. Is that clearer?

[michaelpg, 2016/08/12 22:43:05]

Right, so this would have returned *true* for clear-browser-data, which is what
I wanted. I'll open a bug on the behavior that I see now, so we can start with
concrete behavior :-)

+          this.parent.section == this.section;
+    },
   };
 
   // Abbreviated variable for easier definitions.
   var r = Route;
 
   // Root pages.
   r.BASIC = new Route('/');
   r.ADVANCED = new Route('/advanced');
   r.ABOUT = new Route('/help');
 
 <if expr="chromeos">
   r.INTERNET = r.BASIC.createSection('/internet', 'internet');
-  r.NETWORK_DETAIL = r.INTERNET.createChild('/networkDetail', 'network-detail');
-  r.KNOWN_NETWORKS = r.INTERNET.createChild('/knownNetworks', 'known-networks');
+  r.NETWORK_DETAIL = r.INTERNET.createChild('/networkDetail');
+  r.KNOWN_NETWORKS = r.INTERNET.createChild('/knownNetworks');
 </if>
 
   r.APPEARANCE = r.BASIC.createSection('/appearance', 'appearance');
-  r.FONTS = r.APPEARANCE.createChild('/fonts', 'appearance-fonts');
+  r.FONTS = r.APPEARANCE.createChild('/fonts');
 
   r.DEFAULT_BROWSER =
       r.BASIC.createSection('/defaultBrowser', 'defaultBrowser');
 
   r.SEARCH = r.BASIC.createSection('/search', 'search');
-  r.SEARCH_ENGINES = r.SEARCH.createChild('/searchEngines', 'search-engines');
+  r.SEARCH_ENGINES = r.SEARCH.createChild('/searchEngines');
 
   r.ON_STARTUP = r.BASIC.createSection('/onStartup', 'onStartup');
 
   r.PEOPLE = r.BASIC.createSection('/people', 'people');
-  r.SYNC = r.PEOPLE.createChild('/syncSetup', 'sync');
+  r.SYNC = r.PEOPLE.createChild('/syncSetup');
 <if expr="not chromeos">
-  r.MANAGE_PROFILE = r.PEOPLE.createChild('/manageProfile', 'manageProfile');
+  r.MANAGE_PROFILE = r.PEOPLE.createChild('/manageProfile');
 </if>
 <if expr="chromeos">
-  r.CHANGE_PICTURE = r.PEOPLE.createChild('/changePicture', 'changePicture');
-  r.ACCOUNTS = r.PEOPLE.createChild('/accounts', 'users');
-  r.LOCK_SCREEN = r.PEOPLE.createChild('/lockScreen', 'lockScreen');
-  r.SETUP_PIN = r.LOCK_SCREEN.createDialog('/setupPin', 'setupPin');
+  r.CHANGE_PICTURE = r.PEOPLE.createChild('/changePicture');
+  r.LOCK_SCREEN = r.PEOPLE.createChild('/lockScreen');
+  r.SETUP_PIN = r.LOCK_SCREEN.createDialog('/setupPin');
 
   r.DEVICE = r.BASIC.createSection('/device', 'device');
-  r.POINTERS = r.DEVICE.createChild('/pointer-overlay', 'pointers');
-  r.KEYBOARD = r.DEVICE.createChild('/keyboard-overlay', 'keyboard');
-  r.DISPLAY = r.DEVICE.createChild('/display', 'display');
-  r.NOTES = r.DEVICE.createChild('/note', 'note');
+  r.POINTERS = r.DEVICE.createChild('/pointer-overlay');
+  r.KEYBOARD = r.DEVICE.createChild('/keyboard-overlay');
+  r.DISPLAY = r.DEVICE.createChild('/display');
+  r.NOTES = r.DEVICE.createChild('/note');
 </if>
 
   r.PRIVACY = r.ADVANCED.createSection('/privacy', 'privacy');
-  r.CERTIFICATES =
-      r.PRIVACY.createChild('/certificates', 'manage-certificates');
+  r.CERTIFICATES = r.PRIVACY.createChild('/certificates');
   r.CLEAR_BROWSER_DATA =
       r.PRIVACY.createDialog('/clearBrowserData', 'clear-browsing-data');
-  r.SITE_SETTINGS = r.PRIVACY.createChild('/siteSettings', 'site-settings');
-  r.SITE_SETTINGS_ALL = r.SITE_SETTINGS.createChild('all', 'all-sites');
-  r.SITE_SETTINGS_ALL_DETAILS =
-      r.SITE_SETTINGS_ALL.createChild('details', 'site-details');
+  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_HANDLERS = r.SITE_SETTINGS.createChild(
-      'handlers', 'protocol-handlers');
+  r.SITE_SETTINGS_HANDLERS = r.SITE_SETTINGS.createChild('handlers');
 
   // TODO(tommicli): Find a way to refactor these repetitive category routes.
-  r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS = r.SITE_SETTINGS.createChild(
-      'automaticDownloads', 'site-settings-category-automatic-downloads');
-  r.SITE_SETTINGS_BACKGROUND_SYNC = r.SITE_SETTINGS.createChild(
-      'backgroundSync', 'site-settings-category-background-sync');
-  r.SITE_SETTINGS_CAMERA = r.SITE_SETTINGS.createChild(
-      'camera', 'site-settings-category-camera');
-  r.SITE_SETTINGS_COOKIES = r.SITE_SETTINGS.createChild(
-      'cookies', 'site-settings-category-cookies');
-  r.SITE_SETTINGS_IMAGES = r.SITE_SETTINGS.createChild(
-      'images', 'site-settings-category-images');
-  r.SITE_SETTINGS_JAVASCRIPT = r.SITE_SETTINGS.createChild(
-      'javascript', 'site-settings-category-javascript');
-  r.SITE_SETTINGS_KEYGEN = r.SITE_SETTINGS.createChild(
-      'keygen', 'site-settings-category-keygen');
-  r.SITE_SETTINGS_LOCATION = r.SITE_SETTINGS.createChild(
-      'location', 'site-settings-category-location');
-  r.SITE_SETTINGS_MICROPHONE = r.SITE_SETTINGS.createChild(
-      'microphone', 'site-settings-category-microphone');
-  r.SITE_SETTINGS_NOTIFICATIONS = r.SITE_SETTINGS.createChild(
-      'notifications', 'site-settings-category-notifications');
-  r.SITE_SETTINGS_PLUGINS = r.SITE_SETTINGS.createChild(
-      'plugins', 'site-settings-category-plugins');
-  r.SITE_SETTINGS_POPUPS = r.SITE_SETTINGS.createChild(
-      'popups', 'site-settings-category-popups');
-  r.SITE_SETTINGS_UNSANDBOXED_PLUGINS = r.SITE_SETTINGS.createChild(
-      'unsandboxedPlugins', 'site-settings-category-unsandboxed-plugins');
+  r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS =
+      r.SITE_SETTINGS.createChild('automaticDownloads');
+  r.SITE_SETTINGS_BACKGROUND_SYNC =
+      r.SITE_SETTINGS.createChild('backgroundSync');
+  r.SITE_SETTINGS_CAMERA = r.SITE_SETTINGS.createChild('camera');
+  r.SITE_SETTINGS_COOKIES = r.SITE_SETTINGS.createChild('cookies');
+  r.SITE_SETTINGS_IMAGES = r.SITE_SETTINGS.createChild('images');
+  r.SITE_SETTINGS_JAVASCRIPT = r.SITE_SETTINGS.createChild('javascript');
+  r.SITE_SETTINGS_KEYGEN = r.SITE_SETTINGS.createChild('keygen');
+  r.SITE_SETTINGS_LOCATION = r.SITE_SETTINGS.createChild('location');
+  r.SITE_SETTINGS_MICROPHONE = r.SITE_SETTINGS.createChild('microphone');
+  r.SITE_SETTINGS_NOTIFICATIONS = r.SITE_SETTINGS.createChild('notifications');
+  r.SITE_SETTINGS_PLUGINS = r.SITE_SETTINGS.createChild('plugins');
+  r.SITE_SETTINGS_POPUPS = r.SITE_SETTINGS.createChild('popups');
+  r.SITE_SETTINGS_UNSANDBOXED_PLUGINS =
+      r.SITE_SETTINGS.createChild('unsandboxedPlugins');
 
   r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS_DETAILS =
-      r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS.createChild('details',
-                                                      'site-details');
+      r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS.createChild('details');
   r.SITE_SETTINGS_BACKGROUND_SYNC_DETAILS =
-      r.SITE_SETTINGS_BACKGROUND_SYNC.createChild('details', 'site-details');
+      r.SITE_SETTINGS_BACKGROUND_SYNC.createChild('details');
   r.SITE_SETTINGS_CAMERA_DETAILS =
-      r.SITE_SETTINGS_CAMERA.createChild('details', 'site-details');
+      r.SITE_SETTINGS_CAMERA.createChild('details');
   r.SITE_SETTINGS_COOKIES_DETAILS =
-      r.SITE_SETTINGS_COOKIES.createChild('details', 'site-details');
+      r.SITE_SETTINGS_COOKIES.createChild('details');
   r.SITE_SETTINGS_IMAGES_DETAILS =
-      r.SITE_SETTINGS_IMAGES.createChild('details', 'site-details');
+      r.SITE_SETTINGS_IMAGES.createChild('details');
   r.SITE_SETTINGS_JAVASCRIPT_DETAILS =
-      r.SITE_SETTINGS_JAVASCRIPT.createChild('details', 'site-details');
+      r.SITE_SETTINGS_JAVASCRIPT.createChild('details');
   r.SITE_SETTINGS_KEYGEN_DETAILS =
-      r.SITE_SETTINGS_KEYGEN.createChild('details', 'site-details');
+      r.SITE_SETTINGS_KEYGEN.createChild('details');
   r.SITE_SETTINGS_LOCATION_DETAILS =
-      r.SITE_SETTINGS_LOCATION.createChild('details', 'site-details');
+      r.SITE_SETTINGS_LOCATION.createChild('details');
   r.SITE_SETTINGS_MICROPHONE_DETAILS =
-      r.SITE_SETTINGS_MICROPHONE.createChild('details', 'site-details');
+      r.SITE_SETTINGS_MICROPHONE.createChild('details');
   r.SITE_SETTINGS_NOTIFICATIONS_DETAILS =
-      r.SITE_SETTINGS_NOTIFICATIONS.createChild('details', 'site-details');
+      r.SITE_SETTINGS_NOTIFICATIONS.createChild('details');
   r.SITE_SETTINGS_PLUGINS_DETAILS =
-      r.SITE_SETTINGS_PLUGINS.createChild('details', 'site-details');
+      r.SITE_SETTINGS_PLUGINS.createChild('details');
   r.SITE_SETTINGS_POPUPS_DETAILS =
-      r.SITE_SETTINGS_POPUPS.createChild('details', 'site-details');
+      r.SITE_SETTINGS_POPUPS.createChild('details');
   r.SITE_SETTINGS_UNSANDBOXED_PLUGINS_DETAILS =
-      r.SITE_SETTINGS_UNSANDBOXED_PLUGINS.createChild('details',
-                                                      'site-details');
+      r.SITE_SETTINGS_UNSANDBOXED_PLUGINS.createChild('details');
 
 <if expr="chromeos">
   r.DATETIME = r.ADVANCED.createSection('/dateTime', 'dateTime');
 
   r.BLUETOOTH = r.ADVANCED.createSection('/bluetooth', 'bluetooth');
-  r.BLUETOOTH_ADD_DEVICE =
-      r.BLUETOOTH.createChild('/bluetoothAddDevice', 'bluetooth-add-device');
-  r.BLUETOOTH_PAIR_DEVICE = r.BLUETOOTH_ADD_DEVICE.createChild(
-      'bluetoothPairDevice', 'bluetooth-pair-device');
+  r.BLUETOOTH_ADD_DEVICE = r.BLUETOOTH.createChild('/bluetoothAddDevice');
+  r.BLUETOOTH_PAIR_DEVICE =
+      r.BLUETOOTH_ADD_DEVICE.createChild('bluetoothPairDevice');
 </if>
 
   r.PASSWORDS = r.ADVANCED.createSection('/passwords', 'passwordsAndForms');
-  r.AUTOFILL = r.PASSWORDS.createChild('/autofill', 'manage-autofill');
-  r.MANAGE_PASSWORDS =
-      r.PASSWORDS.createChild('/managePasswords', 'manage-passwords');
+  r.AUTOFILL = r.PASSWORDS.createChild('/autofill');
+  r.MANAGE_PASSWORDS = r.PASSWORDS.createChild('/managePasswords');
 
   r.LANGUAGES = r.ADVANCED.createSection('/languages', 'languages');
-  r.LANGUAGES_DETAIL = r.LANGUAGES.createChild('edit', 'language-detail');
-  r.MANAGE_LANGUAGES =
-      r.LANGUAGES.createChild('/manageLanguages', 'manage-languages');
+  r.LANGUAGES_DETAIL = r.LANGUAGES.createChild('edit');
+  r.MANAGE_LANGUAGES = r.LANGUAGES.createChild('/manageLanguages');
 <if expr="chromeos">
-  r.INPUT_METHODS =
-      r.LANGUAGES.createChild('/inputMethods', 'manage-input-methods');
+  r.INPUT_METHODS = r.LANGUAGES.createChild('/inputMethods');
 </if>
 <if expr="not is_macosx">
-  r.EDIT_DICTIONARY =
-      r.LANGUAGES.createChild('/editDictionary', 'edit-dictionary');
+  r.EDIT_DICTIONARY = r.LANGUAGES.createChild('/editDictionary');
 </if>
 
   r.DOWNLOADS = r.ADVANCED.createSection('/downloadsDirectory', 'downloads');
 
   r.PRINTING = r.ADVANCED.createSection('/printing', 'printing');
-  r.CLOUD_PRINTERS = r.PRINTING.createChild('/cloudPrinters', 'cloud-printers');
+  r.CLOUD_PRINTERS = r.PRINTING.createChild('/cloudPrinters');
 <if expr="chromeos">
-  r.CUPS_PRINTERS = r.PRINTING.createChild('/cupsPrinters', 'cups-printers');
-  r.CUPS_PRINTER_DETAIL = r.CUPS_PRINTERS.createChild(
-      '/cupsPrinterDetails', 'cups-printer-details-page');
+  r.CUPS_PRINTERS = r.PRINTING.createChild('/cupsPrinters');
+  r.CUPS_PRINTER_DETAIL = r.CUPS_PRINTERS.createChild('/cupsPrinterDetails');
 </if>
 
   r.ACCESSIBILITY = r.ADVANCED.createSection('/accessibility', 'a11y');
-  r.MANAGE_ACCESSIBILITY = r.ACCESSIBILITY.createChild(
-      '/manageAccessibility', 'manage-a11y');
+  r.MANAGE_ACCESSIBILITY = r.ACCESSIBILITY.createChild('/manageAccessibility');
 
   r.SYSTEM = r.ADVANCED.createSection('/system', 'system');
   r.RESET = r.ADVANCED.createSection('/reset', 'reset');
 
 <if expr="chromeos">
-  r.INPUT_METHODS =
-      r.LANGUAGES.createChild('/inputMethods', 'manage-input-methods');
-  r.DETAILED_BUILD_INFO =
-      r.ABOUT.createChild('/help/details', 'detailed-build-info');
+  r.INPUT_METHODS = r.LANGUAGES.createChild('/inputMethods');
+  r.DETAILED_BUILD_INFO = r.ABOUT.createChild('/help/details');
   r.DETAILED_BUILD_INFO.section = 'about';
 </if>
 
   var routeObservers_ = new Set();
 
   /** @polymerBehavior */
   var RouteObserverBehavior = {
     /** @override */
     attached: function() {
       assert(!routeObservers_.has(this));
@@ skipping 105 matching lines @@
 
   return {
     Route: Route,
     RouteObserverBehavior: RouteObserverBehavior,
     getRouteForPath: getRouteForPath,
     getCurrentRoute: getCurrentRoute,
     getQueryParameters: getQueryParameters,
     navigateTo: navigateTo,
   };
 });