MD Bookmarks: Implement search and selection in new data flow system

chrome/browser/resources/md_bookmarks/app.js

Index: chrome/browser/resources/md_bookmarks/app.js
diff --git a/chrome/browser/resources/md_bookmarks/app.js b/chrome/browser/resources/md_bookmarks/app.js
index 23bb4984359987ce975042b8e94972673d61d652..026fc9c643241198206edde1f80f2552019252ce 100644
--- a/chrome/browser/resources/md_bookmarks/app.js
+++ b/chrome/browser/resources/md_bookmarks/app.js
@@ -9,8 +9,20 @@ Polymer({
     bookmarks.StoreClient,
   ],
 
+  properties: {
+    /** @private */
+    searchTerm_: {
+      type: String,
+      observer: 'searchTermChanged_',
+    },
+  },
+
   /** @override */
   attached: function() {
+    this.watch('searchTerm_', function(store) {
+      return store.search.term;
+    });
+
     chrome.bookmarks.getTree(function(results) {
       var nodeList = bookmarks.util.normalizeNodes(results[0]);
       var initialState = bookmarks.util.createEmptyState();
@@ -21,4 +33,16 @@ Polymer({
       bookmarks.ApiListener.init();
     }.bind(this));
   },
+
+  searchTermChanged_: function() {
+    if (!this.searchTerm_)
+      return;
+
+    chrome.bookmarks.search(this.searchTerm_, function(results) {
+      var ids = results.map(function(node) {
+        return node.id;
+      });
+      this.dispatch(bookmarks.actions.setSearchResults(ids));

[calamity, 2017/03/13 04:50:36]

If we're going to do search results by id, we need to worry about whether the
node will exist...

What happens when you delete a bookmark showing on the search results page, for
example? Is it possible for bad timing to cause a node id that's been deleted to
arrive in the frontend through search?

I think we should either handle the case where results don't exist in the node
map or make sure that it can never happen.

[tsergeant, 2017/03/14 02:40:36]

I was working yesterday on a follow-up CL which purges deleted nodes (including
all descendants) from all areas of the state tree, which addressed this concern.
The timing requirements for this are pretty specific, and I wasn't able to get
it to repro in JS alone (maybe there's a property of the JS api or the bookmark
mode that prevents this? who knows)
What kind of thing do you think should happen? Do we assert? Do we try and fail
gracefully in the UI and hide the <bookmark-item> if the node doesn't exist?

[calamity, 2017/03/14 03:32:18]

Taking a glance at the API implementation, timing issues don't seem to be a
problem. I think just asserting in item.js is fine, but I would feel better if
we had some way of reporting these assertions if they fail, just so we're aware
of any uptick in issues in the real world. Maybe just adding a histogram that
counts every time an assert gets hit? WDYT?

[tsergeant, 2017/03/14 05:51:10]

As discussed in person, I've added in an assertion to <bookmarks-item> which
checks if the node exists when itemId is assigned. There's also a TODO there to
do something with Histograms if it fails.

+    }.bind(this));
+  },
 });