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

chrome/browser/resources/md_history/query_manager.js

 // Copyright 2017 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.
 
 Polymer({
   is: 'history-query-manager',
 
   properties: {
     /** @type {QueryState} */
     queryState: {
       type: Object,
       notify: true,
-    },
+      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,
 
-    groupedRange_: {
-      type: Number,
-      // Use a computed property to be able to compare old and new values.
-      computed: 'computeGroupedRange_(queryState.range)',
+          set range(val) {
+            this._range = Number(val);
+          },
+          get range() {
+            return this._range;
+          },
+        };
+      },
     },
 
     /** @type {QueryResult} */
     queryResult: Object,
   },
 
   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]);
   },
 
   /**
    * @param {boolean} incremental
    * @private
    */
   queryHistory_: function(incremental) {
     var queryState = this.queryState;
     // Disable querying until the first set of results have been returned. If
     // there is a search, query immediately to support search query params from
@@ skipping 19 matching lines @@
     chrome.send('queryHistory', [
       queryState.searchTerm,
       queryState.groupedOffset,
       queryState.range,
       lastVisitTime,
       maxResults,
     ]);
   },
 
   /**
+   * @param {{range: ?HistoryRange,
+   *          offset: ?number,
+   *          search: ?string}} changes
+   * @private
+   */
+  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;
+
+    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 {Event} e
+   * @private
+   */
+  onChangeQuery_: function(e) {
+    this.changeQuery_(e.detail);
+  },
+
+  /**
    * @param {!Event} e
    * @private
    */
   onQueryHistory_: function(e) {
     this.queryHistory_(/** @type {boolean} */ e.detail);
     return false;
   },
 
   /** @private */
-  groupedOffsetChanged_: function() {
-    this.queryHistory_(false);
-  },
-
-  /**
-   * @param {HistoryRange} range
-   * @return {HistoryRange}
-   * @private
-   */
-  computeGroupedRange_: function(range) {
-    if (this.groupedRange_ != undefined) {
-      this.set('queryState.groupedOffset', 0);
-
-      this.queryHistory_(false);
-      this.fire('history-view-changed');
-    }
-
-    return range;
-  },
-
-  /** @private */
   searchTermChanged_: function() {
-    this.queryHistory_(false);
     // TODO(tsergeant): Ignore incremental searches in this metric.
     if (this.queryState.searchTerm)
       md_history.BrowserService.getInstance().recordAction('Search');
   },
 });