MD Settings: remove unsupported routes from guest-mode.

chrome/browser/resources/settings/route.js

Index: chrome/browser/resources/settings/route.js
diff --git a/chrome/browser/resources/settings/route.js b/chrome/browser/resources/settings/route.js
index 1d3496baa53825bda5d9738a01697bc316a2b2bb..9fecf239e311636678e4f126aa2973617d3b8e5c 100644
--- a/chrome/browser/resources/settings/route.js
+++ b/chrome/browser/resources/settings/route.js
@@ -2,36 +2,126 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+/**
+ * Specifies available routes in settings.

[dpapad, 2017/07/06 02:07:06]

Nit: Can we clarify this comment a bit?
s/available/all possible

Per my understanding this specifies all possible routes, without taking into
account the current state (like guest mode). Meaning that the routes in this
typedef are not necessarily existing on |settings.routes| at runtime.

[scottchen, 2017/07/07 20:58:45]

Done.

+ *
+ * @typedef {{
+ *   ABOUT: (undefined|!settings.Route),
+ *   ABOUT_ABOUT: (undefined|!settings.Route),
+ *   ACCESSIBILITY: (undefined|!settings.Route),
+ *   ACCOUNTS: (undefined|!settings.Route),
+ *   ADVANCED: (undefined|!settings.Route),
+ *   ANDROID_APPS: (undefined|!settings.Route),
+ *   ANDROID_APPS_DETAILS: (undefined|!settings.Route),
+ *   APPEARANCE: (undefined|!settings.Route),
+ *   AUTOFILL: (undefined|!settings.Route),
+ *   BASIC: (undefined|!settings.Route),
+ *   BLUETOOTH: (undefined|!settings.Route),
+ *   BLUETOOTH_DEVICES: (undefined|!settings.Route),
+ *   CERTIFICATES: (undefined|!settings.Route),
+ *   CHANGE_PICTURE: (undefined|!settings.Route),
+ *   CLEAR_BROWSER_DATA: (undefined|!settings.Route),
+ *   CLOUD_PRINTERS: (undefined|!settings.Route),
+ *   CUPS_PRINTER_DETAIL: (undefined|!settings.Route),
+ *   CUPS_PRINTERS: (undefined|!settings.Route),
+ *   DATETIME: (undefined|!settings.Route),
+ *   DEFAULT_BROWSER: (undefined|!settings.Route),
+ *   DETAILED_BUILD_INFO: (undefined|!settings.Route),
+ *   DEVICE: (undefined|!settings.Route),
+ *   DISPLAY: (undefined|!settings.Route),
+ *   DOWNLOADS: (undefined|!settings.Route),
+ *   EDIT_DICTIONARY: (undefined|!settings.Route),
+ *   FINGERPRINT: (undefined|!settings.Route),
+ *   FONTS: (undefined|!settings.Route),
+ *   IMPORT_DATA: (undefined|!settings.Route),
+ *   INPUT_METHODS: (undefined|!settings.Route),
+ *   INTERNET: (undefined|!settings.Route),
+ *   INTERNET_NETWORKS: (undefined|!settings.Route),
+ *   KEYBOARD: (undefined|!settings.Route),
+ *   KNOWN_NETWORKS: (undefined|!settings.Route),
+ *   LANGUAGES: (undefined|!settings.Route),
+ *   LOCK_SCREEN: (undefined|!settings.Route),
+ *   MANAGE_ACCESSIBILITY: (undefined|!settings.Route),
+ *   MANAGE_PASSWORDS: (undefined|!settings.Route),
+ *   MANAGE_PROFILE: (undefined|!settings.Route),
+ *   NETWORK_CONFIG: (undefined|!settings.Route),
+ *   NETWORK_DETAIL: (undefined|!settings.Route),
+ *   ON_STARTUP: (undefined|!settings.Route),
+ *   PASSWORDS: (undefined|!settings.Route),
+ *   PEOPLE: (undefined|!settings.Route),
+ *   POINTERS: (undefined|!settings.Route),
+ *   POWER: (undefined|!settings.Route),
+ *   PRINTING: (undefined|!settings.Route),
+ *   PRIVACY: (undefined|!settings.Route),
+ *   RESET: (undefined|!settings.Route),
+ *   RESET_DIALOG: (undefined|!settings.Route),
+ *   SEARCH: (undefined|!settings.Route),
+ *   SEARCH_ENGINES: (undefined|!settings.Route),
+ *   SIGN_OUT: (undefined|!settings.Route),
+ *   SITE_SETTINGS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_ADS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_ALL: (undefined|!settings.Route),
+ *   SITE_SETTINGS_AUTOMATIC_DOWNLOADS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_BACKGROUND_SYNC: (undefined|!settings.Route),
+ *   SITE_SETTINGS_CAMERA: (undefined|!settings.Route),
+ *   SITE_SETTINGS_COOKIES: (undefined|!settings.Route),
+ *   SITE_SETTINGS_DATA_DETAILS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_FLASH: (undefined|!settings.Route),
+ *   SITE_SETTINGS_HANDLERS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_IMAGES: (undefined|!settings.Route),
+ *   SITE_SETTINGS_JAVASCRIPT: (undefined|!settings.Route),
+ *   SITE_SETTINGS_LOCATION: (undefined|!settings.Route),
+ *   SITE_SETTINGS_MICROPHONE: (undefined|!settings.Route),
+ *   SITE_SETTINGS_MIDI_DEVICES: (undefined|!settings.Route),
+ *   SITE_SETTINGS_NOTIFICATIONS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_PDF_DOCUMENTS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_POPUPS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_PROTECTED_CONTENT: (undefined|!settings.Route),
+ *   SITE_SETTINGS_SITE_DETAILS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_UNSANDBOXED_PLUGINS: (undefined|!settings.Route),
+ *   SITE_SETTINGS_USB_DEVICES: (undefined|!settings.Route),
+ *   SITE_SETTINGS_ZOOM_LEVELS: (undefined|!settings.Route),
+ *   STORAGE: (undefined|!settings.Route),
+ *   STYLUS: (undefined|!settings.Route),
+ *   SYNC: (undefined|!settings.Route),
+ *   SYSTEM: (undefined|!settings.Route),
+ *   TRIGGERED_RESET_DIALOG: (undefined|!settings.Route),
+ * }}
+ */
+var SettingsRoutes;
+
 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;
+  class Route {
+    /**
+     * @param {string} path

[dpapad, 2017/07/06 02:07:06]

Nit: Maybe shorten as follows:

/** @param {string} path */

[scottchen, 2017/07/07 20:58:44]

Done.

+     */
+    constructor(path) {
+      /** @type {string} */
+      this.path = path;
 
-    /** @type {?settings.Route} */
-    this.parent = null;
+      /** @type {?settings.Route} */
+      this.parent = null;
 
-    /** @type {number} */
-    this.depth = 0;
+      /** @type {number} */
+      this.depth = 0;
 
-    /**
-     * @type {boolean} Whether this route corresponds to a navigable
-     *     dialog. Those routes don't belong to a "section".
-     */
-    this.isNavigableDialog = false;
+      /**
+       * @type {boolean} Whether this route corresponds to a navigable
+       *     dialog. Those routes don't belong to a "section".
+       */
+      this.isNavigableDialog = false;
 
-    // Below are all legacy properties to provide compatibility with the old
-    // routing system.
+      // Below are all legacy properties to provide compatibility with the old
+      // routing system.
 
-    /** @type {string} */
-    this.section = '';
-  };
+      /** @type {string} */
+      this.section = '';
+    }
 
-  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
@@ -39,7 +129,7 @@ cr.define('settings', function() {
      * @return {!settings.Route}
      * @private
      */
-    createChild: function(path) {
+    createChild(path) {
       assert(path);
 
       // |path| extends this route's path if it doesn't have a leading slash.
@@ -52,7 +142,7 @@ cr.define('settings', function() {
       route.depth = this.depth + 1;
 
       return route;
-    },
+    }
 
     /**
      * Returns a new Route instance that's a child section of this route.
@@ -62,188 +152,394 @@ cr.define('settings', function() {
      * @return {!settings.Route}
      * @private
      */
-    createSection: function(path, section) {
+    createSection(path, section) {
       var route = this.createChild(path);
       route.section = section;
       return route;
-    },
+    }
 
     /**
      * Returns true if this route matches or is an ancestor of the parameter.
      * @param {!settings.Route} route
      * @return {boolean}
      */
-    contains: function(route) {
+    contains(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.
      * @return {boolean}
      */
-    isSubpage: function() {
+    isSubpage() {
       return !!this.parent && !!this.section &&
           this.parent.section == this.section;
-    },
-  };
+    }
+  }
+
+  class Router {
+    constructor() {
+      /** @private {!SettingsRoutes} */

[dpapad, 2017/07/06 02:07:06]

Let's add a comment to differentiate between routes_ and SettingsRoutes.
Something like

"List of available routes. This is populated taking into account current state
(like guest mode)."

[scottchen, 2017/07/07 20:58:44]

Done.

+      this.routes_ = {};

[dpapad, 2017/07/06 02:07:06]

Not sure if this is worth doing, but mentioning it for discussion.

1) We could simply return a new object from initializeRoutes().
this.routes_ = this.initializeRoutes_();
2) Given on 1, initializeRoutes, does not access |this| anymore, so it does not
even need to be an instance method, so we could simply move it outside of this
class (and possibly rename), as follows:

this.routes_ = computeAvailableRoutes();

[scottchen, 2017/07/07 20:58:44]

I was going to use Router_.initializeRoutes() in the test to re-initiate all the
routes after fiddling with settings.pageVisibility, in which case having it as
an instance method made sense. 

But then I realized it didn't reset this.routes_ to start from scratch anyway,
so I changed to just make a whole new Router() in the test instead. Given this,
I can change this part to be as you suggested.

[dpapad, 2017/07/07 21:19:30]

SG. Consider this optional.

+      this.initializeRoutes();
+
+      /**
+       * The current active route. This updated is only by settings.navigateTo
+       * or settings.initializeRouteFromUrl.
+       * @type {!settings.Route}
+       */
+      this.currentRoute = this.routes_.BASIC;
+
+      /**
+       * The current query parameters. This is updated only by
+       * settings.navigateTo or settings.initializeRouteFromUrl.
+       * @private {!URLSearchParams}
+       */
+      this.currentQueryParameters_ = new URLSearchParams();
+
+      /** @private {boolean} */
+      this.wasLastRouteChangePopstate_ = false;
+
+      /** @private {boolean}*/
+      this.initializeRouteFromUrlCalled_ = false;
+    }
+
+    initializeRoutes() {

[dpapad, 2017/07/06 02:07:06]

Can this be private?

[scottchen, 2017/07/07 20:58:44]

Moving this method out of the class definition.

+      var pageVisibility = settings.pageVisibility || {};
+
+      // Abbreviated variable for easier definitions.
+      var r = this.routes_;
+
+      // Root pages.
+      r.BASIC = new Route('/');
+      r.ABOUT = new Route('/help');
+
+      // Navigable dialogs. These are the only non-section children of root
+      // pages. These are disfavored. If we add anymore, we should add explicit
+      // support.
+      r.IMPORT_DATA = r.BASIC.createChild('/importData');
+      r.IMPORT_DATA.isNavigableDialog = true;
+      r.SIGN_OUT = r.BASIC.createChild('/signOut');
+      r.SIGN_OUT.isNavigableDialog = true;
+
+      // <if expr="chromeos">
+      r.INTERNET = r.BASIC.createSection('/internet', 'internet');
+      r.INTERNET_NETWORKS = r.INTERNET.createChild('/networks');
+      r.NETWORK_CONFIG = r.INTERNET.createChild('/networkConfig');
+      r.NETWORK_DETAIL = r.INTERNET.createChild('/networkDetail');
+      r.KNOWN_NETWORKS = r.INTERNET.createChild('/knownNetworks');
+      r.BLUETOOTH = r.BASIC.createSection('/bluetooth', 'bluetooth');
+      r.BLUETOOTH_DEVICES = r.BLUETOOTH.createChild('/bluetoothDevices');
+      // </if>
+
+      if (pageVisibility.appearance !== false) {
+        r.APPEARANCE = r.BASIC.createSection('/appearance', 'appearance');
+        r.FONTS = r.APPEARANCE.createChild('/fonts');
+      }
+
+      if (pageVisibility.defaultBrowser !== false) {
+        r.DEFAULT_BROWSER =
+            r.BASIC.createSection('/defaultBrowser', 'defaultBrowser');
+      }
+
+      r.SEARCH = r.BASIC.createSection('/search', 'search');
+      r.SEARCH_ENGINES = r.SEARCH.createChild('/searchEngines');
+
+      // <if expr="chromeos">
+      r.ANDROID_APPS = r.BASIC.createSection('/androidApps', 'androidApps');
+      r.ANDROID_APPS_DETAILS =
+          r.ANDROID_APPS.createChild('/androidApps/details');
+      // </if>
+
+      if (pageVisibility.onStartup !== false) {
+        r.ON_STARTUP = r.BASIC.createSection('/onStartup', 'onStartup');
+      }
+
+      if (pageVisibility.people !== false) {
+        r.PEOPLE = r.BASIC.createSection('/people', 'people');
+        r.SYNC = r.PEOPLE.createChild('/syncSetup');
+        // <if expr="not chromeos">
+        r.MANAGE_PROFILE = r.PEOPLE.createChild('/manageProfile');
+        // </if>
+        // <if expr="chromeos">
+        r.CHANGE_PICTURE = r.PEOPLE.createChild('/changePicture');
+        r.ACCOUNTS = r.PEOPLE.createChild('/accounts');
+        r.LOCK_SCREEN = r.PEOPLE.createChild('/lockScreen');
+        r.FINGERPRINT = r.LOCK_SCREEN.createChild('/lockScreen/fingerprint');
+        // </if>
+      }
+
+      // <if expr="chromeos">
+      r.DEVICE = r.BASIC.createSection('/device', 'device');
+      r.POINTERS = r.DEVICE.createChild('/pointer-overlay');
+      r.KEYBOARD = r.DEVICE.createChild('/keyboard-overlay');
+      r.STYLUS = r.DEVICE.createChild('/stylus');
+      r.DISPLAY = r.DEVICE.createChild('/display');
+      r.STORAGE = r.DEVICE.createChild('/storage');
+      r.POWER = r.DEVICE.createChild('/power');
+      // </if>
+
+      // Advacned Routes
+      if (pageVisibility.advancedSettings !== false) {
+        r.ADVANCED = new Route('/advanced');
+
+        r.CLEAR_BROWSER_DATA = r.ADVANCED.createChild('/clearBrowserData');
+        r.CLEAR_BROWSER_DATA.isNavigableDialog = true;
+
+        if (pageVisibility.privacy !== false) {
+          r.PRIVACY = r.ADVANCED.createSection('/privacy', 'privacy');
+          r.CERTIFICATES = r.PRIVACY.createChild('/certificates');
+          r.SITE_SETTINGS = r.PRIVACY.createChild('/content');
+        }
+
+        if (loadTimeData.getBoolean('enableSiteSettings')) {
+          r.SITE_SETTINGS_ALL = r.SITE_SETTINGS.createChild('all');
+          r.SITE_SETTINGS_SITE_DETAILS =
+              r.SITE_SETTINGS_ALL.createChild('/content/siteDetails');
+        } else if (loadTimeData.getBoolean('enableSiteDetails')) {
+          // When there is no "All Sites", pressing 'back' from "Site Details"
+          // should return to "Content Settings". This should only occur when
+          // |kSiteSettings| is off and |kSiteDetails| is on.
+          r.SITE_SETTINGS_SITE_DETAILS =
+              r.SITE_SETTINGS.createChild('/content/siteDetails');
+        }
+
+        r.SITE_SETTINGS_HANDLERS = r.SITE_SETTINGS.createChild('/handlers');
+
+        // TODO(tommycli): Find a way to refactor these repetitive category
+        // routes.
+        r.SITE_SETTINGS_ADS = r.SITE_SETTINGS.createChild('ads');
+        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_DATA_DETAILS =
+            r.SITE_SETTINGS_COOKIES.createChild('/cookies/detail');
+        r.SITE_SETTINGS_IMAGES = r.SITE_SETTINGS.createChild('images');
+        r.SITE_SETTINGS_JAVASCRIPT = r.SITE_SETTINGS.createChild('javascript');
+        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_FLASH = r.SITE_SETTINGS.createChild('flash');
+        r.SITE_SETTINGS_POPUPS = r.SITE_SETTINGS.createChild('popups');
+        r.SITE_SETTINGS_UNSANDBOXED_PLUGINS =
+            r.SITE_SETTINGS.createChild('unsandboxedPlugins');
+        r.SITE_SETTINGS_MIDI_DEVICES =
+            r.SITE_SETTINGS.createChild('midiDevices');
+        r.SITE_SETTINGS_USB_DEVICES = r.SITE_SETTINGS.createChild('usbDevices');
+        r.SITE_SETTINGS_ZOOM_LEVELS = r.SITE_SETTINGS.createChild('zoomLevels');
+        r.SITE_SETTINGS_PDF_DOCUMENTS =
+            r.SITE_SETTINGS.createChild('pdfDocuments');
+        r.SITE_SETTINGS_PROTECTED_CONTENT =
+            r.SITE_SETTINGS.createChild('protectedContent');
+
+        // <if expr="chromeos">
+        if (pageVisibility.dateTime !== false) {
+          r.DATETIME = r.ADVANCED.createSection('/dateTime', 'dateTime');
+        }
+        // </if>
+
+        if (pageVisibility.passwordsAndForms !== false) {
+          r.PASSWORDS = r.ADVANCED.createSection(
+              '/passwordsAndForms', 'passwordsAndForms');
+          r.AUTOFILL = r.PASSWORDS.createChild('/autofill');
+          r.MANAGE_PASSWORDS = r.PASSWORDS.createChild('/passwords');
+        }
+
+        r.LANGUAGES = r.ADVANCED.createSection('/languages', 'languages');
+        // <if expr="chromeos">
+        r.INPUT_METHODS = r.LANGUAGES.createChild('/inputMethods');
+        // </if>
+        // <if expr="not is_macosx">
+        r.EDIT_DICTIONARY = r.LANGUAGES.createChild('/editDictionary');
+        // </if>
+
+        if (pageVisibility.downloads !== false) {
+          r.DOWNLOADS = r.ADVANCED.createSection('/downloads', 'downloads');
+        }
+
+        r.PRINTING = r.ADVANCED.createSection('/printing', 'printing');
+        r.CLOUD_PRINTERS = r.PRINTING.createChild('/cloudPrinters');
+        // <if expr="chromeos">
+        r.CUPS_PRINTERS = r.PRINTING.createChild('/cupsPrinters');
+        r.CUPS_PRINTER_DETAIL =
+            r.CUPS_PRINTERS.createChild('/cupsPrinterDetails');
+        // </if>
+
+        r.ACCESSIBILITY = r.ADVANCED.createSection('/accessibility', 'a11y');
+        // <if expr="chromeos">
+        r.MANAGE_ACCESSIBILITY =
+            r.ACCESSIBILITY.createChild('/manageAccessibility');
+        // </if>
+
+        r.SYSTEM = r.ADVANCED.createSection('/system', 'system');
+
+        if (pageVisibility.reset !== false) {
+          r.RESET = r.ADVANCED.createSection('/reset', 'reset');
+          r.RESET_DIALOG = r.ADVANCED.createChild('/resetProfileSettings');
+          r.RESET_DIALOG.isNavigableDialog = true;
+          r.TRIGGERED_RESET_DIALOG =
+              r.ADVANCED.createChild('/triggeredResetProfileSettings');
+          r.TRIGGERED_RESET_DIALOG.isNavigableDialog = true;
+        }
+      }
+
+      // <if expr="chromeos">
+      // "About" is the only section in About, but we still need to create the
+      // route in order to show the subpage on Chrome OS.
+      r.ABOUT_ABOUT = r.ABOUT.createSection('/help/about', 'about');
+      r.DETAILED_BUILD_INFO = r.ABOUT_ABOUT.createChild('/help/details');
+      // </if>
+    }
+
+    /** @return {settings.Route} */
+    getRoute(routeName) {
+      return this.routes_[routeName];
+    }
+
+    /** @return {!SettingsRoutes} */
+    getRoutes() {
+      return this.routes_;
+    }
+
+    /**
+     * Helper function to set the current route and notify all observers.
+     * @param {!settings.Route} route
+     * @param {!URLSearchParams} queryParameters
+     * @param {boolean} isPopstate
+     */
+    setCurrentRoute(route, queryParameters, isPopstate) {
+      var oldRoute = this.currentRoute;
+      this.currentRoute = route;
+      this.currentQueryParameters_ = queryParameters;
+      this.wasLastRouteChangePopstate_ = isPopstate;
+      routeObservers.forEach(function(observer) {
+        observer.currentRouteChanged(this.currentRoute, oldRoute);
+      }.bind(this));
+    }
+
+    /** @return {!settings.Route} */
+    getCurrentRoute() {
+      return this.currentRoute;
+    }
+
+    /** @return {!URLSearchParams} */
+    getQueryParameters() {
+      return new URLSearchParams(
+          this.currentQueryParameters_);  // Defensive copy.
+    }
+
+    /** @return {boolean} */
+    lastRouteChangeWasPopstate() {
+      return this.wasLastRouteChangePopstate_;
+    }
+
+    /**
+     * @param {string} path
+     * @return {?settings.Route} The matching canonical route, or null if none
+     *     matches.
+     */
+    getRouteForPath(path) {
+      // Allow trailing slash in paths.
+      var canonicalPath = path.replace(CANONICAL_PATH_REGEX, '$1$2');
+
+      // TODO(tommycli): Use Object.values once Closure compilation supports it.
+      var matchingKey = Object.keys(this.routes_).find(function(key) {
+        return this.routes_[key].path == canonicalPath;
+      }.bind(this));
+
+      return !!matchingKey ? this.routes_[matchingKey] : null;
+    }
+
+    /**
+     * Navigates to a canonical route and pushes a new history entry.
+     * @param {!settings.Route} route
+     * @param {URLSearchParams=} opt_dynamicParameters Navigations to the same
+     *     URL parameters in a different order will still push to history.
+     * @param {boolean=} opt_removeSearch Whether to strip the 'search' URL
+     *     parameter during navigation. Defaults to false.
+     */
+    navigateTo(route, opt_dynamicParameters, opt_removeSearch) {
+      // The ADVANCED route only serves as a parent of subpages, and should not
+      // be possible to navigate to it directly.
+      if (route == this.routes_.ADVANCED)
+        route = /** @type {!settings.Route} */ (this.routes_.BASIC);
+
+      var params = opt_dynamicParameters || new URLSearchParams();
+      var removeSearch = !!opt_removeSearch;
+
+      var oldSearchParam = this.getQueryParameters().get('search') || '';
+      var newSearchParam = params.get('search') || '';
 
-  // Abbreviated variable for easier definitions.
-  var r = Route;
-
-  // Root pages.
-  r.BASIC = new Route('/');
-  r.ADVANCED = new Route('/advanced');
-  r.ABOUT = new Route('/help');
-
-  // Navigable dialogs. These are the only non-section children of root pages.
-  // These are disfavored. If we add anymore, we should add explicit support.
-  r.IMPORT_DATA = r.BASIC.createChild('/importData');
-  r.IMPORT_DATA.isNavigableDialog = true;
-  r.SIGN_OUT = r.BASIC.createChild('/signOut');
-  r.SIGN_OUT.isNavigableDialog = true;
-  r.CLEAR_BROWSER_DATA = r.ADVANCED.createChild('/clearBrowserData');
-  r.CLEAR_BROWSER_DATA.isNavigableDialog = true;
-  r.RESET_DIALOG = r.ADVANCED.createChild('/resetProfileSettings');
-  r.RESET_DIALOG.isNavigableDialog = true;
-  r.TRIGGERED_RESET_DIALOG =
-      r.ADVANCED.createChild('/triggeredResetProfileSettings');
-  r.TRIGGERED_RESET_DIALOG.isNavigableDialog = true;
-
-  // <if expr="chromeos">
-  r.INTERNET = r.BASIC.createSection('/internet', 'internet');
-  r.INTERNET_NETWORKS = r.INTERNET.createChild('/networks');
-  r.NETWORK_CONFIG = r.INTERNET.createChild('/networkConfig');
-  r.NETWORK_DETAIL = r.INTERNET.createChild('/networkDetail');
-  r.KNOWN_NETWORKS = r.INTERNET.createChild('/knownNetworks');
-  r.BLUETOOTH = r.BASIC.createSection('/bluetooth', 'bluetooth');
-  r.BLUETOOTH_DEVICES = r.BLUETOOTH.createChild('/bluetoothDevices');
-  // </if>
-
-  r.APPEARANCE = r.BASIC.createSection('/appearance', 'appearance');
-  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');
-
-  // <if expr="chromeos">
-  r.ANDROID_APPS = r.BASIC.createSection('/androidApps', 'androidApps');
-  r.ANDROID_APPS_DETAILS = r.ANDROID_APPS.createChild('/androidApps/details');
-  // </if>
-
-  r.ON_STARTUP = r.BASIC.createSection('/onStartup', 'onStartup');
-
-  r.PEOPLE = r.BASIC.createSection('/people', 'people');
-  r.SYNC = r.PEOPLE.createChild('/syncSetup');
-  // <if expr="not chromeos">
-  r.MANAGE_PROFILE = r.PEOPLE.createChild('/manageProfile');
-  // </if>
-  // <if expr="chromeos">
-  r.CHANGE_PICTURE = r.PEOPLE.createChild('/changePicture');
-  r.ACCOUNTS = r.PEOPLE.createChild('/accounts');
-  r.LOCK_SCREEN = r.PEOPLE.createChild('/lockScreen');
-  r.FINGERPRINT = r.LOCK_SCREEN.createChild('/lockScreen/fingerprint');
-
-  r.DEVICE = r.BASIC.createSection('/device', 'device');
-  r.POINTERS = r.DEVICE.createChild('/pointer-overlay');
-  r.KEYBOARD = r.DEVICE.createChild('/keyboard-overlay');
-  r.STYLUS = r.DEVICE.createChild('/stylus');
-  r.DISPLAY = r.DEVICE.createChild('/display');
-  r.STORAGE = r.DEVICE.createChild('/storage');
-  r.POWER = r.DEVICE.createChild('/power');
-  // </if>
-
-  r.PRIVACY = r.ADVANCED.createSection('/privacy', 'privacy');
-  r.CERTIFICATES = r.PRIVACY.createChild('/certificates');
-
-  r.SITE_SETTINGS = r.PRIVACY.createChild('/content');
-
-  if (loadTimeData.getBoolean('enableSiteSettings')) {
-    r.SITE_SETTINGS_ALL = r.SITE_SETTINGS.createChild('all');
-    r.SITE_SETTINGS_SITE_DETAILS =
-        r.SITE_SETTINGS_ALL.createChild('/content/siteDetails');
-  } else if (loadTimeData.getBoolean('enableSiteDetails')) {
-    // When there is no "All Sites", pressing 'back' from "Site Details" should
-    // return to "Content Settings". This should only occur when |kSiteSettings|
-    // is off and |kSiteDetails| is on.
-    r.SITE_SETTINGS_SITE_DETAILS =
-        r.SITE_SETTINGS.createChild('/content/siteDetails');
+      if (!removeSearch && oldSearchParam && !newSearchParam)
+        params.append('search', oldSearchParam);
+
+      var url = route.path;
+      var queryString = params.toString();
+      if (queryString)
+        url += '?' + queryString;
+
+      // History serializes the state, so we don't push the actual route object.
+      window.history.pushState(this.currentRoute.path, '', url);
+      this.setCurrentRoute(route, params, false);
+    }
+
+    /**
+     * Navigates to the previous route if it has an equal or lesser depth.
+     * If there is no previous route in history meeting those requirements,
+     * this navigates to the immediate parent. This will never exit Settings.
+     */
+    navigateToPreviousRoute() {
+      var previousRoute = window.history.state &&
+          assert(this.getRouteForPath(
+              /** @type {string} */ (window.history.state)));
+
+      if (previousRoute && previousRoute.depth <= this.currentRoute.depth)
+        window.history.back();
+      else
+        this.navigateTo(
+            this.currentRoute.parent ||
+            /** @type {!settings.Route} */ (this.routes_.BASIC));
+    }
+
+    /**
+     * Initialize the route and query params from the URL.
+     */
+    initializeRouteFromUrl() {
+      assert(!this.initializeRouteFromUrlCalled_);
+      this.initializeRouteFromUrlCalled_ = true;
+
+      var route = this.getRouteForPath(window.location.pathname);
+      // Never allow direct navigation to ADVANCED.
+      if (route && route != this.routes_.ADVANCED) {
+        this.currentRoute = route;
+        this.currentQueryParameters_ =
+            new URLSearchParams(window.location.search);
+      } else {
+        window.history.replaceState(undefined, '', this.routes_.BASIC.path);
+      }
+    }
+
+    resetRouteForTesting() {
+      this.initializeRouteFromUrlCalled_ = false;
+      this.wasLastRouteChangePopstate_ = false;
+      this.currentRoute = /** @type {!settings.Route} */ (this.routes_.BASIC);
+      this.currentQueryParameters_ = new URLSearchParams();
+    }
   }
 
-  r.SITE_SETTINGS_HANDLERS = r.SITE_SETTINGS.createChild('/handlers');
-
-  // TODO(tommycli): Find a way to refactor these repetitive category routes.
-  r.SITE_SETTINGS_ADS = r.SITE_SETTINGS.createChild('ads');
-  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_DATA_DETAILS =
-      r.SITE_SETTINGS_COOKIES.createChild('/cookies/detail');
-  r.SITE_SETTINGS_IMAGES = r.SITE_SETTINGS.createChild('images');
-  r.SITE_SETTINGS_JAVASCRIPT = r.SITE_SETTINGS.createChild('javascript');
-  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_FLASH = r.SITE_SETTINGS.createChild('flash');
-  r.SITE_SETTINGS_POPUPS = r.SITE_SETTINGS.createChild('popups');
-  r.SITE_SETTINGS_UNSANDBOXED_PLUGINS =
-      r.SITE_SETTINGS.createChild('unsandboxedPlugins');
-  r.SITE_SETTINGS_MIDI_DEVICES = r.SITE_SETTINGS.createChild('midiDevices');
-  r.SITE_SETTINGS_USB_DEVICES = r.SITE_SETTINGS.createChild('usbDevices');
-  r.SITE_SETTINGS_ZOOM_LEVELS = r.SITE_SETTINGS.createChild('zoomLevels');
-  r.SITE_SETTINGS_PDF_DOCUMENTS = r.SITE_SETTINGS.createChild('pdfDocuments');
-  r.SITE_SETTINGS_PROTECTED_CONTENT =
-      r.SITE_SETTINGS.createChild('protectedContent');
-
-  // <if expr="chromeos">
-  r.DATETIME = r.ADVANCED.createSection('/dateTime', 'dateTime');
-  // </if>
-
-  r.PASSWORDS =
-      r.ADVANCED.createSection('/passwordsAndForms', 'passwordsAndForms');
-  r.AUTOFILL = r.PASSWORDS.createChild('/autofill');
-  r.MANAGE_PASSWORDS = r.PASSWORDS.createChild('/passwords');
-
-  r.LANGUAGES = r.ADVANCED.createSection('/languages', 'languages');
-  // <if expr="chromeos">
-  r.INPUT_METHODS = r.LANGUAGES.createChild('/inputMethods');
-  // </if>
-  // <if expr="not is_macosx">
-  r.EDIT_DICTIONARY = r.LANGUAGES.createChild('/editDictionary');
-  // </if>
-
-  r.DOWNLOADS = r.ADVANCED.createSection('/downloads', 'downloads');
-
-  r.PRINTING = r.ADVANCED.createSection('/printing', 'printing');
-  r.CLOUD_PRINTERS = r.PRINTING.createChild('/cloudPrinters');
-  // <if expr="chromeos">
-  r.CUPS_PRINTERS = r.PRINTING.createChild('/cupsPrinters');
-  r.CUPS_PRINTER_DETAIL = r.CUPS_PRINTERS.createChild('/cupsPrinterDetails');
-  // </if>
-
-  r.ACCESSIBILITY = r.ADVANCED.createSection('/accessibility', 'a11y');
-  // <if expr="chromeos">
-  r.MANAGE_ACCESSIBILITY = r.ACCESSIBILITY.createChild('/manageAccessibility');
-  // </if>
-
-  r.SYSTEM = r.ADVANCED.createSection('/system', 'system');
-  r.RESET = r.ADVANCED.createSection('/reset', 'reset');
-
-  // <if expr="chromeos">
-  // "About" is the only section in About, but we still need to create the route
-  // in order to show the subpage on Chrome OS.
-  r.ABOUT_ABOUT = r.ABOUT.createSection('/help/about', 'about');
-  r.DETAILED_BUILD_INFO = r.ABOUT_ABOUT.createChild('/help/details');
-  // </if>
+  var Router_ = new Router();

[dpapad, 2017/07/06 02:07:06]

Typically only class names start with a capital letter. Let's rename this to
router_ or instance_ (denoting the singleton instance).

[scottchen, 2017/07/07 20:58:45]

Done. Changed to routerInstance because apparently presubmit doesn't like
lowercaseWithUnderscore_.

+
+  var getRouter = function() {

[dpapad, 2017/07/06 02:07:06]

Is this used anywhere?

[scottchen, 2017/07/07 20:58:45]

not anymore, removing.

+    return Router_;
+  };
 
   var routeObservers = new Set();
 
@@ -256,7 +552,7 @@ cr.define('settings', function() {
 
       // Emulating Polymer data bindings, the observer is called when the
       // element starts observing the route.
-      this.currentRouteChanged(currentRoute, undefined);
+      this.currentRouteChanged(Router_.currentRoute, undefined);
     },
 
     /** @override */
@@ -281,155 +577,34 @@ cr.define('settings', function() {
    */
   var CANONICAL_PATH_REGEX = /(^\/)([\/-\w]+)(\/$)/;
 
-  /**
-   * @param {string} path
-   * @return {?settings.Route} The matching canonical route, or null if none
-   *     matches.
-   */
-  var getRouteForPath = function(path) {
-    // Allow trailing slash in paths.
-    var canonicalPath = path.replace(CANONICAL_PATH_REGEX, '$1$2');
-
-    // TODO(tommycli): Use Object.values once Closure compilation supports it.
-    var matchingKey = Object.keys(Route).find(function(key) {
-      return Route[key].path == canonicalPath;
-    });
-
-    return !!matchingKey ? Route[matchingKey] : null;
-  };
-
-  /**
-   * The current active route. This updated is only by settings.navigateTo or
-   * settings.initializeRouteFromUrl.
-   * @private {!settings.Route}
-   */
-  var currentRoute = Route.BASIC;
-
-  /**
-   * The current query parameters. This is updated only by settings.navigateTo
-   * or settings.initializeRouteFromUrl.
-   * @private {!URLSearchParams}
-   */
-  var currentQueryParameters = new URLSearchParams();
-
-  /** @private {boolean} */
-  var wasLastRouteChangePopstate = false;
-
-  /** @private */
-  var initializeRouteFromUrlCalled = false;
-
-  /**
-   * Initialize the route and query params from the URL.
-   */
-  var initializeRouteFromUrl = function() {
-    assert(!initializeRouteFromUrlCalled);
-    initializeRouteFromUrlCalled = true;
-
-    var route = getRouteForPath(window.location.pathname);
-    // Never allow direct navigation to ADVANCED.
-    if (route && route != Route.ADVANCED) {
-      currentRoute = route;
-      currentQueryParameters = new URLSearchParams(window.location.search);
-    } else {
-      window.history.replaceState(undefined, '', Route.BASIC.path);
-    }
-  };
-
-  function resetRouteForTesting() {
-    initializeRouteFromUrlCalled = false;
-    wasLastRouteChangePopstate = false;
-    currentRoute = Route.BASIC;
-    currentQueryParameters = new URLSearchParams();
-  }
-
-  /**
-   * Helper function to set the current route and notify all observers.
-   * @param {!settings.Route} route
-   * @param {!URLSearchParams} queryParameters
-   * @param {boolean} isPopstate
-   */
-  var setCurrentRoute = function(route, queryParameters, isPopstate) {
-    var oldRoute = currentRoute;
-    currentRoute = route;
-    currentQueryParameters = queryParameters;
-    wasLastRouteChangePopstate = isPopstate;
-    routeObservers.forEach(function(observer) {
-      observer.currentRouteChanged(currentRoute, oldRoute);
-    });
-  };
-
-  /** @return {!settings.Route} */
-  var getCurrentRoute = function() {
-    return currentRoute;
-  };
-
-  /** @return {!URLSearchParams} */
-  var getQueryParameters = function() {
-    return new URLSearchParams(currentQueryParameters);  // Defensive copy.
-  };
-
-  /** @return {boolean} */
-  var lastRouteChangeWasPopstate = function() {
-    return wasLastRouteChangePopstate;
-  };
-
-  /**
-   * Navigates to a canonical route and pushes a new history entry.
-   * @param {!settings.Route} route
-   * @param {URLSearchParams=} opt_dynamicParameters Navigations to the same
-   *     URL parameters in a different order will still push to history.
-   * @param {boolean=} opt_removeSearch Whether to strip the 'search' URL
-   *     parameter during navigation. Defaults to false.
-   */
-  var navigateTo = function(route, opt_dynamicParameters, opt_removeSearch) {
-    // The ADVANCED route only serves as a parent of subpages, and should not
-    // be possible to navigate to it directly.
-    if (route == settings.Route.ADVANCED)
-      route = settings.Route.BASIC;
-
-    var params = opt_dynamicParameters || new URLSearchParams();
-    var removeSearch = !!opt_removeSearch;
-
-    var oldSearchParam = getQueryParameters().get('search') || '';
-    var newSearchParam = params.get('search') || '';
-
-    if (!removeSearch && oldSearchParam && !newSearchParam)
-      params.append('search', oldSearchParam);
-
-    var url = route.path;
-    var queryString = params.toString();
-    if (queryString)
-      url += '?' + queryString;
-
-    // History serializes the state, so we don't push the actual route object.
-    window.history.pushState(currentRoute.path, '', url);
-    setCurrentRoute(route, params, false);
-  };
-
-  /**
-   * Navigates to the previous route if it has an equal or lesser depth.
-   * If there is no previous route in history meeting those requirements,
-   * this navigates to the immediate parent. This will never exit Settings.
-   */
-  var navigateToPreviousRoute = function() {
-    var previousRoute = window.history.state &&
-        assert(getRouteForPath(/** @type {string} */ (window.history.state)));
-
-    if (previousRoute && previousRoute.depth <= currentRoute.depth)
-      window.history.back();
-    else
-      navigateTo(currentRoute.parent || Route.BASIC);
-  };
-
   window.addEventListener('popstate', function(event) {
     // On pop state, do not push the state onto the window.history again.
-    setCurrentRoute(
-        getRouteForPath(window.location.pathname) || Route.BASIC,
+    Router_.setCurrentRoute(
+        /** @type {!settings.Route} */ (
+            Router_.getRouteForPath(window.location.pathname) ||
+            Router_.getRoutes().BASIC),
         new URLSearchParams(window.location.search), true);
   });
 
+  // TODO(scottchen): Change to 'get routes() {}' in export when we figure
+  // out why doing so causes a closure compiler error.

[dpapad, 2017/07/06 02:07:06]

Maybe update the comment to mention our newest finding. Something like
"ChromePass limits the syntax of what can be returned from cr.define()."

[scottchen, 2017/07/07 20:58:45]

Done.

+  var routes = Router_.getRoutes();
+

[dpapad, 2017/07/06 02:07:06]

Maybe add a TODO as follows?

"Stop exposing all those methods directly on settings.*, and instead update all
clients to use the router_ instance directly".

[scottchen, 2017/07/07 20:58:45]

Done.

+  var getCurrentRoute = Router_.getCurrentRoute.bind(Router_);
+  var getRouteForPath = Router_.getRouteForPath.bind(Router_);
+  var initializeRouteFromUrl = Router_.initializeRouteFromUrl.bind(Router_);
+  var resetRouteForTesting = Router_.resetRouteForTesting.bind(Router_);
+  var getQueryParameters = Router_.getQueryParameters.bind(Router_);
+  var lastRouteChangeWasPopstate =
+      Router_.lastRouteChangeWasPopstate.bind(Router_);
+  var navigateTo = Router_.navigateTo.bind(Router_);
+  var navigateToPreviousRoute = Router_.navigateToPreviousRoute.bind(Router_);
+
   return {
-    Route: Route,
+    Route: Route,     // The class definition.
+    Router: Router,   // The class definition.
+    router: Router_,  // the singleton.
+    routes: routes,
     RouteObserverBehavior: RouteObserverBehavior,
     getRouteForPath: getRouteForPath,
     initializeRouteFromUrl: initializeRouteFromUrl,