Settings Router Refactor: Migrate to settings.Route.navigateTo calls.

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

Index: chrome/browser/resources/settings/settings_page/settings_router.js
diff --git a/chrome/browser/resources/settings/settings_page/settings_router.js b/chrome/browser/resources/settings/settings_page/settings_router.js
index 9f249ae7ab8507ecc3e5338a23382c2534deb3a0..45385a546f9350caf953088d0e53a8cbf6133a3c 100644
--- a/chrome/browser/resources/settings/settings_page/settings_router.js
+++ b/chrome/browser/resources/settings/settings_page/settings_router.js
@@ -3,22 +3,6 @@
 // found in the LICENSE file.
 
 /**
- * @typedef {{
- *   dialog: (string|undefined),
- *   page: string,
- *   section: string,
- *   subpage: !Array<string>,
- * }}
- */
-var SettingsRoute;
-
-/** @typedef {SettingsRoute|{url: string}} */
-var CanonicalRoute;
-
-/** @typedef {SettingsRoute|{inHistory: boolean}} */
-var HistoricRoute;

[Dan Beam, 2016/07/23 00:17:12]

my only regret is that I never got to get my kicks from naming one "66"

[tommycli, 2016/07/25 16:47:21]

Acknowledged.

-
-/**
  * @fileoverview
  * 'settings-router' is a simple router for settings. Its responsibilities:
  *  - Update the URL when the routing state changes.
@@ -35,8 +19,8 @@ Polymer({
 
   properties: {
     /**
-     * The current active route. This is reflected to the URL. Updates to this
-     * property should replace the whole object.
+     * The current active route. This may only be updated via the global
+     * function settings.navigateTo.
      *
      * currentRoute.page refers to top-level pages such as Basic and Advanced.
      *
@@ -47,121 +31,55 @@ Polymer({
      * the user is on. The previous elements are the ancestor subpages. This
      * enables support for multiple paths to the same subpage. This is used by
      * both the Back button and the Breadcrumb to determine ancestor subpages.
-     * @type {SettingsRoute}
+     * @type {!settings.Route}
      */
     currentRoute: {
       notify: true,
-      observer: 'currentRouteChanged_',
-      type: Object,
-      value: function() {
-        var initialRoute = this.canonicalRoutes_[0];
-
-        // Take the current URL, find a matching pre-defined route, and
-        // initialize the currentRoute to that pre-defined route.
-        for (var i = 0; i < this.canonicalRoutes_.length; ++i) {
-          var canonicalRoute = this.canonicalRoutes_[i];
-          if (canonicalRoute.url == window.location.pathname) {
-            initialRoute = canonicalRoute;
-            break;
-          }
-        }
-
-        return {
-          page: initialRoute.page,
-          section: initialRoute.section,
-          subpage: initialRoute.subpage,
-          dialog: initialRoute.dialog,
-        };
-      },
-    },
-
-    /**
-     * Page titles for the currently active route. Updated by the currentRoute
-     * property observer.
-     * @type {{pageTitle: string}}
-     */
-    currentRouteTitles: {
-      notify: true,
       type: Object,
       value: function() {
-        return {
-          pageTitle: '',
-        };
+        return this.getRouteFor_(window.location.pathname);
       },
     },
   },
 
-
-  /**
-   * @private {!Array<!CanonicalRoute>}
-   * The 'url' property is not accessible to other elements.
-   */
-  canonicalRoutes_: Object.keys(settings.Route).map(function(key) {
-    return settings.Route[key];
-  }),
-
   /**
    * Sets up a history popstate observer.
+   * @override
    */
   created: function() {
     window.addEventListener('popstate', function(event) {
-      if (event.state && event.state.page)
-        this.currentRoute = event.state;
+      // On pop state, do not push the state onto the window.history again.
+      this.currentRoute = this.getRouteFor_(window.location.pathname);

[Dan Beam, 2016/07/23 00:17:12]

so we basically assume this will always work, right?  does this work for invalid
URLs?  i.e. chrome://settings/gobbleDeeGoop

[tommycli, 2016/07/25 16:47:21]

It will give you settings.Route.BASIC for gobbleDeeGoop. Which I think is the
intended behavior...

     }.bind(this));
+
+    settings.navigateTo = this.navigateTo_.bind(this);
   },
 
   /**
-   * Is called when another element modifies the route. This observer validates
-   * the route change against the pre-defined list of routes, and updates the
-   * URL appropriately.
-   * @param {!SettingsRoute} newRoute Where we're headed.
-   * @param {!SettingsRoute|undefined} oldRoute Where we've been.
+   * Returns the matching canonical route, or the default route if none matches.
+   * @param {string} path
+   * @return {!settings.Route}
    * @private
    */
-  currentRouteChanged_: function(newRoute, oldRoute) {
-    for (var i = 0; i < this.canonicalRoutes_.length; ++i) {
-      var canonicalRoute = this.canonicalRoutes_[i];
-      if (canonicalRoute.page == newRoute.page &&
-          canonicalRoute.section == newRoute.section &&
-          canonicalRoute.dialog == newRoute.dialog &&
-          canonicalRoute.subpage.length == newRoute.subpage.length &&
-          canonicalRoute.subpage.every(function(value, index) {
-            return value == newRoute.subpage[index];
-          })) {
-        // Update the property containing the titles for the current route.
-        this.currentRouteTitles = {
-          pageTitle: loadTimeData.getString(canonicalRoute.page + 'PageTitle'),
-        };
+  getRouteFor_: function(path) {
+    var matchingKey = Object.keys(settings.Route).find(function(key) {

[Dan Beam, 2016/07/23 00:17:12]

Object.values()

[tommycli, 2016/07/25 16:47:21]

Closure compiler does not like values. Even after pulling this morning.

+      return settings.Route[key].url == path;
+    });
 
-        // If we are restoring a state from history, don't push it again.
-        if (/** @type {HistoricRoute} */(newRoute).inHistory)
-          return;
+    if (!matchingKey)
+      return settings.Route.BASIC;
 
-        // Mark routes persisted in history as already stored in history.
-        var historicRoute = /** @type {HistoricRoute} */({
-          inHistory: true,
-          page: newRoute.page,
-          section: newRoute.section,
-          subpage: newRoute.subpage,
-          dialog: newRoute.dialog,
-        });
-
-        // Push the current route to the history state, so when the user
-        // navigates with the browser back button, we can recall the route.
-        if (oldRoute) {
-          window.history.pushState(historicRoute, document.title,
-                                   canonicalRoute.url);
-        } else {
-          // For the very first route (oldRoute will be undefined), we replace
-          // the existing state instead of pushing a new one. This is to allow
-          // the user to use the browser back button to exit Settings entirely.
-          window.history.replaceState(historicRoute, document.title);
-        }
-
-        return;
-      }
-    }
+    return settings.Route[matchingKey];
+  },
 
-    assertNotReached('Route not found: ' + JSON.stringify(newRoute));
+  /**
+   * Navigates to a canonical route.
+   * @param {!settings.Route} route
+   * @private
+   */
+  navigateTo_: function(route) {
+    assert(!!route);
+    window.history.pushState(undefined, document.title, route.url);
+    this.currentRoute = route;
   },
 });