Settings WebUI: when text changes in settings, redo search

Bug: 711156
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ic2847a86c7ec3671db79e524a25fa15c08193455
Reviewed-on: https://chromium-review.googlesource.com/996768Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548689}

chrome/browser/resources/settings/basic_page/basic_page.js

@@ -170,34 +170,15 @@ Polymer({
    * sections.
    * @param {string} query The text to search for.
    * @return {!Promise<!settings.SearchResult>} A signal indicating that
-   *     searching finished.
+   * searching finished.
    */
   searchContents: function(query) {
-    const whenSearchDone = [
-      settings.getSearchManager().search(query, assert(this.$$('#basicPage'))),
-    ];
-
-    if (this.pageVisibility.advancedSettings !== false) {
-      whenSearchDone.push(
-          this.$$('#advancedPageTemplate').get().then(function(advancedPage) {
-            return settings.getSearchManager().search(query, advancedPage);
-          }));
-    }
+    const nodes = [assert(this.$$('#basicPage'))];
+    if (this.pageVisibility.advancedSettings !== false)
+      nodes.push(this.$$('#advancedPageTemplate').get());
 
-    return Promise.all(whenSearchDone).then(function(requests) {
-      // Combine the SearchRequests results to a single SearchResult object.
-      return {
-        canceled: requests.some(function(r) {
-          return r.canceled;
-        }),
-        didFindMatches: requests.some(function(r) {
-          return r.didFindMatches();
-        }),
-        // All requests correspond to the same user query, so only need to check
-        // one of them.
-        wasClearSearch: requests[0].isSame(''),
-      };
-    });
+    return Promise.all(nodes).then(
+        nodes => settings.getSearchManager().search(query, nodes));
   },
 
   // <if expr="chromeos">

chrome/browser/resources/settings/downloads_page/downloads_page.html

@@ -24,7 +24,7 @@
     <div class="settings-box first two-line">
       <div class="start">
         <div>$i18n{downloadLocation}</div>
-        <div class="secondary" no-search>
+        <div class="secondary">
 <if expr="not chromeos">
           [[prefs.download.default_directory.value]]
 </if>

chrome/browser/resources/settings/settings_main/settings_main.js

@@ -87,6 +87,7 @@ Polymer({
   attached: function() {
     this.listen(this, 'freeze-scroll', 'onFreezeScroll_');
     this.listen(this, 'lazy-loaded', 'onLazyLoaded_');
+    this.registerSearchManagerObserver_();
   },
 
   /** @private */
@@ -250,42 +251,56 @@ Polymer({
     assertNotReached();
   },
 
+  /** @private */
+  registerSearchManagerObserver_: function() {
+    const observer = /** @type {!settings.SearchManagerObserver} */ ({
+      onSearchStart: () => {
+        // Trigger rendering of the basic and advanced pages and search once
+        // ready.
+        this.inSearchMode_ = true;
+        this.toolbarSpinnerActive = true;
+      },
+      onSearchComplete: searchResult => {
+        if (searchResult.canceled) {
+          // Nothing to do here. A previous search request was canceled
+          // because a new search request was issued with a different query
+          // before the previous completed.
+          return;
+        }
+
+        this.toolbarSpinnerActive = false;
+        this.inSearchMode_ = !searchResult.wasClearSearch;
+        this.showNoResultsFound_ =
+            this.inSearchMode_ && !searchResult.didFindMatches;
+        if (this.showNoResultsFound_ &&
+            settings.getCurrentRoute().isSubpage()) {
+          settings.navigateTo(settings.routes.BASIC);
+          this.searchContents(searchResult.rawQuery);
+          return;
+        }
+        if (this.inSearchMode_) {
+          Polymer.IronA11yAnnouncer.requestAvailability();
+          this.fire('iron-announce', {
+            text: this.showNoResultsFound_ ?
+                loadTimeData.getString('searchNoResults') :
+                loadTimeData.getStringF('searchResults', searchResult.rawQuery)
+          });
+        }
+      },
+    });
+    settings.getSearchManager().registerObserver(observer);
+  },
+
   /**
    * @param {string} query
-   * @return {!Promise} A promise indicating that searching finished.
+   * @return {!Promise<!settings.SearchResult>} A promise indicating that
+   * searching finished.
    */
   searchContents: function(query) {
-    // Trigger rendering of the basic and advanced pages and search once ready.
-    this.inSearchMode_ = true;
-    this.toolbarSpinnerActive = true;
-
     return new Promise((resolve, reject) => {
       setTimeout(() => {
-        const whenSearchDone =
-            assert(this.getPage_(settings.routes.BASIC)).searchContents(query);
-        whenSearchDone.then(result => {
-          resolve();
-          if (result.canceled) {
-            // Nothing to do here. A previous search request was canceled
-            // because a new search request was issued with a different query
-            // before the previous completed.
-            return;
-          }
-
-          this.toolbarSpinnerActive = false;
-          this.inSearchMode_ = !result.wasClearSearch;
-          this.showNoResultsFound_ =
-              this.inSearchMode_ && !result.didFindMatches;
-
-          if (this.inSearchMode_) {
-            Polymer.IronA11yAnnouncer.requestAvailability();
-            this.fire('iron-announce', {
-              text: this.showNoResultsFound_ ?
-                  loadTimeData.getString('searchNoResults') :
-                  loadTimeData.getStringF('searchResults', query)
-            });
-          }
-        });
+        const basicPage = assert(this.getPage_(settings.routes.BASIC));
+        basicPage.searchContents(query).then(resolve);
       }, 0);
     });
   },

chrome/test/data/webui/settings/basic_page_browsertest.js

@@ -45,8 +45,7 @@ TEST_F('SettingsBasicPageBrowserTest', 'DISABLED_Load', function() {
     /** @override */
     search(text, page) {
       if (this.searchRequest_ == null || !this.searchRequest_.isSame(text)) {
-        this.searchRequest_ = new settings.SearchRequest(
-            text, document.createElement('div'));
+        this.searchRequest_ = new settings.SearchRequest(text);
         this.searchRequest_.finished = true;
         this.searchRequest_.updateMatches(false);
 
@@ -66,6 +65,9 @@ TEST_F('SettingsBasicPageBrowserTest', 'DISABLED_Load', function() {
       }
       return this.searchRequest_.resolver.promise;
     }
+
+    /** @override */
+    registerObserver(observer) {}
   }
 
   // Register mocha tests.

chrome/test/data/webui/settings/search_settings_test.js

@@ -37,29 +37,32 @@ cr.define('settings_test', function() {
       const div = document.querySelector('#mydiv');
 
       assertTrue(section.hiddenBySearch);
-      return searchManager.search('settings', section).then(function() {
-        assertFalse(section.hiddenBySearch);
-
-        const highlightWrapper = div.querySelector('.search-highlight-wrapper');
-        assertTrue(!!highlightWrapper);
-
-        const originalContent = highlightWrapper.querySelector(
-            '.search-highlight-original-content');
-        assertTrue(!!originalContent);
-        assertEquals(optionText, originalContent.textContent);
-
-        const searchHits = highlightWrapper.querySelectorAll(
-            '.search-highlight-hit');
-        assertEquals(1, searchHits.length);
-        assertEquals('Settings', searchHits[0].textContent);
-
-        // Check that original DOM structure is restored when search
-        // highlights are cleared.
-        return searchManager.search('', section);
-      }).then(function() {
-        assertEquals(0, div.children.length);
-        assertEquals(optionText, div.textContent);
-      });
+      return searchManager.search('settings', [section])
+          .then(function() {
+            assertFalse(section.hiddenBySearch);
+
+            const highlightWrapper =
+                div.querySelector('.search-highlight-wrapper');
+            assertTrue(!!highlightWrapper);
+
+            const originalContent = highlightWrapper.querySelector(
+                '.search-highlight-original-content');
+            assertTrue(!!originalContent);
+            assertEquals(optionText, originalContent.textContent);
+
+            const searchHits =
+                highlightWrapper.querySelectorAll('.search-highlight-hit');
+            assertEquals(1, searchHits.length);
+            assertEquals('Settings', searchHits[0].textContent);
+
+            // Check that original DOM structure is restored when search
+            // highlights are cleared.
+            return searchManager.search('', [section]);
+          })
+          .then(function() {
+            assertEquals(0, div.children.length);
+            assertEquals(optionText, div.textContent);
+          });
     });
 
     /**
@@ -81,23 +84,25 @@ cr.define('settings_test', function() {
       const select = section.querySelector('select');
 
       assertTrue(section.hiddenBySearch);
-      return searchManager.search('settings', section).then(function() {
-        assertFalse(section.hiddenBySearch);
-
-        const highlightWrapper = select.querySelector(
-            '.search-highlight-wrapper');
-        assertFalse(!!highlightWrapper);
-
-        // Check that original DOM structure is present even after search
-        // highlights are cleared.
-        return searchManager.search('', section);
-      }).then(function() {
-        const options = select.querySelectorAll('option');
-        assertEquals(3, options.length);
-        assertEquals('Foo', options[0].textContent);
-        assertEquals('Settings', options[1].textContent);
-        assertEquals('Baz', options[2].textContent);
-      });
+      return searchManager.search('settings', [section])
+          .then(function() {
+            assertFalse(section.hiddenBySearch);
+
+            const highlightWrapper =
+                select.querySelector('.search-highlight-wrapper');
+            assertFalse(!!highlightWrapper);
+
+            // Check that original DOM structure is present even after search
+            // highlights are cleared.
+            return searchManager.search('', [section]);
+          })
+          .then(function() {
+            const options = select.querySelectorAll('option');
+            assertEquals(3, options.length);
+            assertEquals('Foo', options[0].textContent);
+            assertEquals('Settings', options[1].textContent);
+            assertEquals('Baz', options[2].textContent);
+          });
     });
 
     test('ignored elements are ignored', function() {
@@ -119,7 +124,7 @@ cr.define('settings_test', function() {
       const section = document.querySelector('settings-section');
       assertTrue(section.hiddenBySearch);
 
-      return searchManager.search(text, section).then(function() {
+      return searchManager.search(text, [section]).then(function() {
         assertTrue(section.hiddenBySearch);
       });
     });
@@ -141,15 +146,95 @@ cr.define('settings_test', function() {
       const sections = Array.prototype.slice.call(
           document.querySelectorAll('settings-section'));
 
-      return Promise.all(sections.map(s => searchManager.search('there', s)))
-          .then(function(requests) {
-            assertTrue(requests[0].didFindMatches());
+      return Promise.all(sections.map(s => searchManager.search('there', [s])))
+          .then(function(results) {
+            assertTrue(results[0].didFindMatches);
             assertFalse(sections[0].hiddenBySearch);
-            assertTrue(requests[1].didFindMatches());
+            assertTrue(results[1].didFindMatches);
             assertFalse(sections[1].hiddenBySearch);
-            assertFalse(requests[2].didFindMatches());
+            assertFalse(results[2].didFindMatches);
             assertTrue(sections[2].hiddenBySearch);
           });
     });
+
+    test('search is redone when text is changed', function() {
+      const observer = /** @type {!settings.SearchManagerObserver} */ ((() => {
+        let resolver;
+        return {
+          onSearchStart() {},
+          onSearchComplete(searchResult) {
+            resolver.resolve(searchResult);
+          },
+          waitForSearchComplete(doSearch) {
+            resolver = new PromiseResolver();
+            doSearch();
+            return resolver.promise;
+          }
+        };
+      })());
+      searchManager.registerObserver(observer);
+
+      const originalText = 'FooSettingsFoo';
+
+      document.body.innerHTML = `<settings-section hidden-by-search>
+            <div id="mydiv">${originalText}</div>
+          </settings-section>`;
+
+      const section = document.querySelector('settings-section');
+      const div = document.querySelector('#mydiv');
+      assertTrue(section.hiddenBySearch);
+      return observer
+          .waitForSearchComplete(() => {
+            searchManager.search('settings', [document.body]);
+          })
+          .then(searchResult => {
+            assertFalse(section.hiddenBySearch);
+            const expected = {
+              canceled: false,
+              didFindMatches: true,
+              rawQuery: 'settings',
+              wasClearSearch: false,
+            };
+            assertDeepEquals(expected, searchResult);
+            const highlightWrapper =
+                div.querySelector('.search-highlight-wrapper');
+            assertTrue(!!highlightWrapper);
+            const originalContent = highlightWrapper.querySelector(
+                '.search-highlight-original-content');
+            assertTrue(!!originalContent);
+            return observer.waitForSearchComplete(() => {
+              originalContent.childNodes[0].nodeValue = 'Foo';
+            });
+          })
+          .then(searchResult => {
+            assertTrue(section.hiddenBySearch);
+            const expected = {
+              canceled: false,
+              didFindMatches: false,
+              rawQuery: 'settings',
+              wasClearSearch: false,
+            };
+            assertDeepEquals(expected, searchResult);
+            const highlightWrapper =
+                div.querySelector('.search-highlight-wrapper');
+            assertTrue(!highlightWrapper);
+            return observer.waitForSearchComplete(() => {
+              div.childNodes[0].nodeValue = originalText;
+            });
+          })
+          .then(searchResult => {
+            assertFalse(section.hiddenBySearch);
+            const expected = {
+              canceled: false,
+              didFindMatches: true,
+              rawQuery: 'settings',
+              wasClearSearch: false,
+            };
+            assertDeepEquals(expected, searchResult);
+            const highlightWrapper =
+                div.querySelector('.search-highlight-wrapper');
+            assertTrue(!!highlightWrapper);
+          });
+    });
   });
 });

chrome/test/data/webui/settings/settings_main_test.js

@@ -22,6 +22,9 @@ cr.define('settings_main_page', function() {
 
       /** @private {?settings.SearchRequest} */
       this.searchRequest_ = null;
+
+      /** @private {?settings.SearchManagerObserver} */
+      this.observer_ = null;
     }
 
     /**
@@ -33,6 +36,7 @@ cr.define('settings_main_page', function() {
 
     /** @override */
     search(text, page) {
+      this.observer_.onSearchStart();
       this.methodCalled('search', text);
 
       if (this.searchRequest_ == null || !this.searchRequest_.isSame(text)) {
@@ -40,9 +44,15 @@ cr.define('settings_main_page', function() {
         this.searchRequest_.finished = true;
         this.searchRequest_.updateMatches(this.matchesFound_);
         this.searchRequest_.resolver.resolve(this.searchRequest_);
+        this.observer_.onSearchComplete(this.searchRequest_.result());
       }
       return this.searchRequest_.resolver.promise;
     }
+
+    /** @override */
+    registerObserver(observer) {
+      this.observer_ = observer;
+    }
   }
 
   let settingsPrefs = null;