MD History: Replace app-route with a custom router

chrome/browser/resources/md_history/app.js

Index: chrome/browser/resources/md_history/app.js
diff --git a/chrome/browser/resources/md_history/app.js b/chrome/browser/resources/md_history/app.js
index 13f5e88e674e85131e8572c0e3b5bc3939e0f6f0..0b79a0350f650031c9064d47f824da46e7bf595d 100644
--- a/chrome/browser/resources/md_history/app.js
+++ b/chrome/browser/resources/md_history/app.js
@@ -14,7 +14,7 @@ Polymer({
     hasSyncedResults: Boolean,
 
     // The id of the currently selected page.
-    selectedPage_: {type: String, observer: 'unselectAll'},
+    selectedPage_: {type: String, observer: 'selectedPageChanged_'},
 
     // Whether domain-grouped history is enabled.
     grouped_: {type: Boolean, reflectToAttribute: true},
@@ -52,12 +52,6 @@ Polymer({
       }
     },
 
-    // Route data for the current page.
-    routeData_: Object,
-
-    // The query params for the page.
-    queryParams_: Object,
-
     // True if the window is narrow enough for the page to have a drawer.
     hasDrawer_: Boolean,
 
@@ -75,17 +69,6 @@ Polymer({
     }
   },
 
-  observers: [
-    // routeData_.page <=> selectedPage
-    'routeDataChanged_(routeData_.page)',
-    'selectedPageChanged_(selectedPage_)',
-
-    // queryParams_.q <=> queryState.searchTerm
-    'searchTermChanged_(queryState_.searchTerm)',
-    'searchQueryParamChanged_(queryParams_.q)',
-
-  ],
-
   // TODO(calamity): Replace these event listeners with data bound properties.
   listeners: {
     'cr-menu-tap': 'onMenuTap_',
@@ -104,12 +87,6 @@ Polymer({
     cr.ui.decorate('command', cr.ui.Command);
     document.addEventListener('canExecute', this.onCanExecute_.bind(this));
     document.addEventListener('command', this.onCommand_.bind(this));
-
-    // Redirect legacy search URLs to URLs compatible with material history.
-    if (window.location.hash) {
-      window.location.href = window.location.href.split('#')[0] + '?' +
-          window.location.hash.substr(1);
-    }
   },
 
   onFirstRender: function() {
@@ -130,7 +107,8 @@ Polymer({
 
   /** Overridden from IronScrollTargetBehavior */
   _scrollHandler: function() {
-    this.toolbarShadow_ = this.scrollTarget.scrollTop != 0;
+    if (this.scrollTarget)
+      this.toolbarShadow_ = this.scrollTarget.scrollTop != 0;
   },
 
   /** @private */
@@ -186,12 +164,6 @@ Polymer({
   focusToolbarSearchField: function() { this.$.toolbar.showSearchField(); },
 
   /**
-   * Fired when the user presses 'More from this site'.
-   * @param {{detail: {domain: string}}} e
-   */
-  searchDomain_: function(e) { this.$.toolbar.setSearchTerm(e.detail.domain); },
-
-  /**
    * @param {Event} e
    * @private
    */
@@ -211,26 +183,6 @@ Polymer({
   },
 
   /**
-   * @param {string} searchTerm
-   * @private
-   */
-  searchTermChanged_: function(searchTerm) {
-    this.set('queryParams_.q', searchTerm || null);
-    this.$['history'].queryHistory(false);
-    // TODO(tsergeant): Ignore incremental searches in this metric.
-    if (this.queryState_.searchTerm)
-      md_history.BrowserService.getInstance().recordAction('Search');
-  },
-
-  /**
-   * @param {string} searchQuery
-   * @private
-   */
-  searchQueryParamChanged_: function(searchQuery) {
-    this.$.toolbar.setSearchTerm(searchQuery || '');
-  },
-
-  /**
    * @param {Event} e
    * @private
    */
@@ -306,17 +258,10 @@ Polymer({
   },
 
   /**
-   * @param {string} page
    * @private
    */
-  routeDataChanged_: function(page) { this.selectedPage_ = page; },
-
-  /**
-   * @param {string} selectedPage
-   * @private
-   */
-  selectedPageChanged_: function(selectedPage) {
-    this.set('routeData_.page', selectedPage);
+  selectedPageChanged_: function() {
+    this.unselectAll();
     this.historyViewChanged_();
   },
 
@@ -325,8 +270,11 @@ Polymer({
     // This allows the synced-device-manager to render so that it can be set as
     // the scroll target.
     requestAnimationFrame(function() {
-      this.scrollTarget = this.$.content.selectedItem.getContentScrollTarget();
-      this._scrollHandler();
+      if (this.$.content.selectedItem) {

[tsergeant, 2016/09/21 07:57:47]

These guards are necessary because of tests.

I don't understand why, but I was getting a sequence of 4 calls to
historyViewChanged_ with `selectedItem` as `undefined` once and "history" every
other time. It was unclear why historyViewChanged_ was even being called -- I
couldn't reproduce anything like it in a regular browser.

I can try going back and debugging why this happened if you're concerned, but
I'm fairly certain that it wasn't anything which will affect the page outside of
tests.

[calamity, 2016/09/22 05:12:26]

Looks fine. Invert and early return with comment.

[tsergeant, 2016/09/22 23:57:42]

Done.

+        this.scrollTarget =
+            this.$.content.selectedItem.getContentScrollTarget();
+        this._scrollHandler();
+      }
     }.bind(this));
     this.recordHistoryPageView_();
   },