WebUI: cr-search-field-behavior, update value with potentially no eve…

…nt first

When there is a search term present, navigating to another section or about
page cleared the search term. The |setValue()| is being called with an
argument that should avoid firing a 'search-changed' event. Due to the order
of how the values are updated, |setValue()| could be called more than once
due to a single |setValue()| call. The subsequent calls to |setValue()| can
potentially fire a 'search-changed' event if the search value has changed.

It is therefore important to update the underlying search term and last value
first while respecting the no event argument. Once completed, the remaining
logic can be executed.

Bug: 954457
Change-Id: I3d4ce6933ba04419b529f45eeeb8ba2ed4977687
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1585174
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655665}

chrome/test/data/webui/cr_elements/cr_search_field_tests.js

@@ -79,14 +79,32 @@ suite('cr-search-field', function() {
     field.setValue('foo');
 
     field.setValue('');
-    assertEquals(['query1', '', 'query2', 'foo', ''].join(), searches.join());
+    assertDeepEquals(['query1', '', 'query2', 'foo', ''], searches);
   });
 
   test('does not notify on setValue with noEvent=true', function() {
     field.click();
     field.setValue('foo', true);
     field.setValue('bar');
     field.setValue('baz', true);
-    assertEquals(['bar'].join(), searches.join());
+    assertDeepEquals(['bar'], searches);
+  });
+
+  test('setValue will return early if the query has not changed', () => {
+    // Need a space at the end, since the effective query will strip the spaces
+    // at the beginning, but not at the end of the query.
+    const value = 'test ';
+    assertNotEquals(value, field.getValue());
+    let calledSetValue = false;
+    field.onSearchTermInput = () => {
+      if (!calledSetValue) {
+        calledSetValue = true;
+        field.setValue(value);
+      }
+    };
+    field.setValue(value, true);
+    field.setValue(`  ${value}  `);
+    assertTrue(calledSetValue);
+    assertEquals(0, searches.length);
   });
 });

ui/webui/resources/cr_elements/cr_search_field/cr_search_field_behavior.js

@@ -24,14 +24,11 @@ const CrSearchFieldBehavior = {
       reflectToAttribute: true,
       value: false,
     },
-
-    /** @private */
-    lastValue_: {
-      type: String,
-      value: '',
-    },
   },
 
+  /** @private {string} */
+  effectiveValue_: '',
+
   /** @private {number} */
   searchDelayTimer_: -1,
 
@@ -55,11 +52,16 @@ const CrSearchFieldBehavior = {
    *     firing for this change.
    */
   setValue: function(value, opt_noEvent) {
-    const searchInput = this.getSearchInput();
-    searchInput.value = value;
+    const updated = this.updateEffectiveValue_(value);
+    this.getSearchInput().value = this.effectiveValue_;
+    if (!updated) {
+      return;
+    }
 
     this.onSearchTermInput();
-    this.onValueChanged_(value, !!opt_noEvent);
+    if (!opt_noEvent) {
+      this.fire('search-changed', this.effectiveValue_);
+    }
   },
 
   /** @private */
@@ -106,18 +108,27 @@ const CrSearchFieldBehavior = {
    * @private
    */
   onValueChanged_: function(newValue, noEvent) {
-    // Trim leading whitespace and replace consecutive whitespace with single
-    // space. This will prevent empty string searches and searches for
-    // effectively the same query.
-    const effectiveValue = newValue.replace(/\s+/g, ' ').replace(/^\s/, '');
-    if (effectiveValue == this.lastValue_) {
-      return;
+    const updated = this.updateEffectiveValue_(newValue);
+    if (updated && !noEvent) {
+      this.fire('search-changed', this.effectiveValue_);
     }
+  },
 
-    this.lastValue_ = effectiveValue;
-
-    if (!noEvent) {
-      this.fire('search-changed', effectiveValue);
+  /**
+   * Trim leading whitespace and replace consecutive whitespace with single
+   * space. This will prevent empty string searches and searches for
+   * effectively the same query.
+   * @param {string} value
+   * @return {boolean}
+   * @private
+   */
+  updateEffectiveValue_: function(value) {
+    const effectiveValue = value.replace(/\s+/g, ' ').replace(/^\s/, '');
+    if (effectiveValue == this.effectiveValue_) {
+      return false;
     }
+
+    this.effectiveValue_ = effectiveValue;
+    return true;
   },
 };