MD History: Use one-way binding for history query state.

chrome/browser/resources/md_history/query_manager.js

Index: chrome/browser/resources/md_history/query_manager.js
diff --git a/chrome/browser/resources/md_history/query_manager.js b/chrome/browser/resources/md_history/query_manager.js
index 8ef65a278ace8efc30bb8aa2feabbe3f9bf3c532..bbf8acb5859800d16771396fa64e7134f55eb57d 100644
--- a/chrome/browser/resources/md_history/query_manager.js
+++ b/chrome/browser/resources/md_history/query_manager.js
@@ -10,12 +10,25 @@ Polymer({
     queryState: {
       type: Object,
       notify: true,
-    },
-
-    groupedRange_: {
-      type: Number,
-      // Use a computed property to be able to compare old and new values.
-      computed: 'computeGroupedRange_(queryState.range)',
+      value: function() {
+        return {
+          // Whether the most recent query was incremental.
+          incremental: false,
+          // A query is initiated by page load.
+          querying: true,
+          queryingDisabled: false,
+          _range: HistoryRange.ALL_TIME,
+          searchTerm: '',
+          groupedOffset: 0,
+
+          set range(val) {
+            this._range = Number(val);
+          },
+          get range() {
+            return this._range;
+          },
+        };
+      },
     },
 
     /** @type {QueryResult} */
@@ -24,21 +37,24 @@ Polymer({
 
   observers: [
     'searchTermChanged_(queryState.searchTerm)',
-    'groupedOffsetChanged_(queryState.groupedOffset)',
   ],
 
-  /** @private {?function(!Event)} */
-  boundOnQueryHistory_: null,
+  /** @private {!Object<string, !function(!Event)>} */
+  documentListeners_: {},
 
   /** @override */
   attached: function() {
-    this.boundOnQueryHistory_ = this.onQueryHistory_.bind(this);
-    document.addEventListener('query-history', this.boundOnQueryHistory_);
+    this.documentListeners_['change-query'] = this.onChangeQuery_.bind(this);
+    this.documentListeners_['query-history'] = this.onQueryHistory_.bind(this);
+
+    for (var e in this.documentListeners_)
+      document.addEventListener(e, this.documentListeners_[e]);
   },
 
   /** @override */
   detached: function() {
-    document.removeEventListener('query-history', this.boundOnQueryHistory_);
+    for (var e in this.documentListeners_)
+      document.removeEventListener(e, this.documentListeners_[e]);
   },
 
   /**
@@ -78,38 +94,61 @@ Polymer({
   },
 
   /**
-   * @param {!Event} e
+   * @param {{range: ?HistoryRange,
+   *          offset: ?number,
+   *          search: ?string}} changes
    * @private
    */
-  onQueryHistory_: function(e) {
-    this.queryHistory_(/** @type {boolean} */ e.detail);
-    return false;
-  },
+  changeQuery_: function(changes) {

[calamity, 2017/01/23 03:03:56]

Why not just put this all in onChangeQuery?

[tsergeant, 2017/01/23 04:18:26]

This is a relic from when all this code was in router, and it was desirable to
be able to call into this method directly.

Nowadays it could be pulled into the event listener, except that I cannot figure
out how to get closure to accept a custom type for `e.detail` while still seeing
`e` as an Event.

/shruggie

Happy to show you in person to see if we can puzzle it out. 

+    var needsUpdate = false;
 
-  /** @private */
-  groupedOffsetChanged_: function() {
-    this.queryHistory_(false);
+    if (changes.range != null && changes.range != this.queryState.range) {
+      this.set('queryState.range', changes.range);
+      needsUpdate = true;
+
+      // Reset back to page 0 of the results, unless changing to a specific
+      // page.
+      if (!changes.offset)
+        this.set('queryState.groupedOffset', 0);
+
+      this.fire('history-view-changed');
+    }
+
+    if (changes.offset != null &&
+        changes.offset != this.queryState.groupedOffset) {
+      this.set('queryState.groupedOffset', changes.offset);
+      needsUpdate = true;
+    }
+
+    if (changes.search != null &&
+        changes.search != this.queryState.searchTerm) {
+      this.set('queryState.searchTerm', changes.search);
+      needsUpdate = true;
+    }
+
+    if (needsUpdate)
+      this.queryHistory_(false);
   },
 
   /**
-   * @param {HistoryRange} range
-   * @return {HistoryRange}
+   * @param {Event} e
    * @private
    */
-  computeGroupedRange_: function(range) {
-    if (this.groupedRange_ != undefined) {
-      this.set('queryState.groupedOffset', 0);
-
-      this.queryHistory_(false);
-      this.fire('history-view-changed');
-    }
+  onChangeQuery_: function(e) {
+    this.changeQuery_(e.detail);
+  },
 
-    return range;
+  /**
+   * @param {!Event} e
+   * @private
+   */
+  onQueryHistory_: function(e) {
+    this.queryHistory_(/** @type {boolean} */ e.detail);
+    return false;
   },
 
   /** @private */
   searchTermChanged_: function() {
-    this.queryHistory_(false);
     // TODO(tsergeant): Ignore incremental searches in this metric.
     if (this.queryState.searchTerm)
       md_history.BrowserService.getInstance().recordAction('Search');