MD Bookmarks: Remove deleted nodes from state tree

chrome/browser/resources/md_bookmarks/store_client.js

Index: chrome/browser/resources/md_bookmarks/store_client.js
diff --git a/chrome/browser/resources/md_bookmarks/store_client.js b/chrome/browser/resources/md_bookmarks/store_client.js
index 344df43db1ea8142842866d281d8346b0273021f..7d1512734967740a7b2c1f51f90ab171bc17bd8a 100644
--- a/chrome/browser/resources/md_bookmarks/store_client.js
+++ b/chrome/browser/resources/md_bookmarks/store_client.js
@@ -38,7 +38,8 @@ cr.define('bookmarks', function() {
      *   watch('item', (state) => state.nodes[this.itemId]);
      *
      * Note that object identity is used to determine if the value has changed
-     * before updating the UI, rather than Polymer-style deep equality.
+     * before updating the UI, rather than Polymer-style deep equality. If the
+     * getter function returns |undefined|, no changes will propagate to the UI.
      *
      * Typechecking is supressed because this conflicts with
      * Object.prototype.watch, which is a Gecko-only method that is recognized
@@ -73,7 +74,7 @@ cr.define('bookmarks', function() {
         // Avoid poking Polymer unless something has actually changed. Reducers
         // must return new objects rather than mutating existing objects, so
         // any real changes will pass through correctly.
-        if (oldValue == newValue)
+        if (oldValue == newValue || newValue == undefined)

[calamity, 2017/04/03 06:54:24]

What case does this prevent? Anything deleted in the nodeList shouldn't be
showing anywhere already, right?

[tsergeant, 2017/04/04 04:42:28]

It comes down to an ordering thing. The sequence of events is

1. <bookmarks-list> creates <bookmarks-item id="X">
2. state.nodes["X"] is removed from the map
3. <bookmarks-item> is notified that the state changed, and sees that the new
value of `state.nodes[this.itemId]` is `undefined`
4. <bookmarks-list> is notified that the state changed, updates the
bookmarks-item to id="Y"
5. <bookmarks-item> updates with its new, defined item.

It's step 3 that's the problem: it is only undefined while the update propagates
fully, but it still causes problems in listeners that expect a defined value.

           return;
 
         this[watch.localProperty] = newValue;