MD Bookmarks: Prevent flash of folder contents when changing search term

In MD Bookmarks, changing search term would clear the results of the
current search, then fire off a new search request, which would bring in
new results <1 second later. This caused a brief flash of folder contents
for whatever folder was currently selected.

This CL fixes that issue by holding on to the old search results until
the new search results are available.

BUG=737076
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2972963003
Cr-Commit-Position: refs/heads/master@{#484494}

chrome/browser/resources/md_bookmarks/reducers.js

@@ -118,7 +118,7 @@ cr.define('bookmarks', function() {
     return {
       term: action.term,
       inProgress: true,
-      results: [],
+      results: search.results,
     };
   };
 
@@ -139,7 +139,7 @@ cr.define('bookmarks', function() {
     return {
       term: '',
       inProgress: false,
-      results: [],
+      results: null,
     };
   };
 
@@ -149,6 +149,9 @@ cr.define('bookmarks', function() {
    * @return {SearchState}
    */
   SearchState.removeDeletedResults = function(search, deletedIds) {
+    if (!search.results)
+      return search;
+
     var newResults = [];
     search.results.forEach(function(id) {
       if (!deletedIds.has(id))

chrome/browser/resources/md_bookmarks/types.js

@@ -37,10 +37,20 @@ var NodeMap;
 var SelectionState;
 
 /**
+ * Note:
+ * - If |results| is null, it means no search results have been returned. This
+ *   is different to |results| being [], which means the last search returned 0
+ *   results.
+ * - |term| is the last search that was performed by the user, and |results| are
+ *   the last results that were returned from the backend. We don't clear
+ *   |results| on incremental searches, meaning that |results| can be 'stale'
+ *   data from a previous search term (while |inProgress| is true). If you need
+ *   to know the exact search term used to generate |results|, you'll need to
+ *   add a new field to the state to track it (eg, SearchState.resultsTerm).
  * @typedef {{
  *   term: string,
  *   inProgress: boolean,
- *   results: !Array<string>,
+ *   results: ?Array<string>,
  * }}
  */
 var SearchState;

chrome/browser/resources/md_bookmarks/util.js

@@ -14,10 +14,10 @@ cr.define('bookmarks.util', function() {
    * @return {!Array<string>}
    */
   function getDisplayedList(state) {
-    if (!isShowingSearch(state))
-      return assert(state.nodes[state.selectedFolder].children);
+    if (isShowingSearch(state))
+      return assert(state.search.results);
 
-    return state.search.results;
+    return assert(state.nodes[state.selectedFolder].children);
   }
 
   /**
@@ -77,7 +77,7 @@ cr.define('bookmarks.util', function() {
       search: {
         term: '',
         inProgress: false,
-        results: [],
+        results: null,
       },
       selection: {
         items: new Set(),
@@ -91,7 +91,7 @@ cr.define('bookmarks.util', function() {
    * @return {boolean}
    */
   function isShowingSearch(state) {
-    return !!state.search.term && !state.search.inProgress;
+    return state.search.results != null;
   }
 
   /**

chrome/test/data/webui/md_bookmarks/reducers_test.js

@@ -436,7 +436,7 @@ suite('search state', function() {
     assertFalse(bookmarks.util.isShowingSearch(clearedState));
     assertDeepEquals(['3'], bookmarks.util.getDisplayedList(clearedState));
     assertEquals('', clearedState.search.term);
-    assertDeepEquals([], clearedState.search.results);
+    assertDeepEquals(null, clearedState.search.results);
 
     // Case 2: Clear search by selecting a new folder.
     action = bookmarks.actions.selectFolder('1');
@@ -446,7 +446,36 @@ suite('search state', function() {
     assertFalse(bookmarks.util.isShowingSearch(selectedState));
     assertDeepEquals(['2'], bookmarks.util.getDisplayedList(selectedState));
     assertEquals('', selectedState.search.term);
-    assertDeepEquals([], selectedState.search.results);
+    assertDeepEquals(null, selectedState.search.results);
+  });
+
+  test('results do not clear while performing a second search', function() {
+    action = bookmarks.actions.setSearchTerm('te');
+    state = bookmarks.reduceAction(state, action);
+
+    assertFalse(bookmarks.util.isShowingSearch(state));
+
+    action = bookmarks.actions.setSearchResults(['2', '3']);
+    state = bookmarks.reduceAction(state, action);
+
+    assertFalse(state.search.inProgress);
+    assertTrue(bookmarks.util.isShowingSearch(state));
+
+    // Continuing the search should not clear the previous results, which should
+    // continue to show until the new results arrive.
+    action = bookmarks.actions.setSearchTerm('test');
+    state = bookmarks.reduceAction(state, action);
+
+    assertTrue(state.search.inProgress);
+    assertTrue(bookmarks.util.isShowingSearch(state));
+    assertDeepEquals(['2', '3'], bookmarks.util.getDisplayedList(state));
+
+    action = bookmarks.actions.setSearchResults(['3']);
+    state = bookmarks.reduceAction(state, action);
+
+    assertFalse(state.search.inProgress);
+    assertTrue(bookmarks.util.isShowingSearch(state));
+    assertDeepEquals(['3'], bookmarks.util.getDisplayedList(state));
   });
 
   test('removes deleted nodes', function() {