MD Bookmarks: Update Delete command to only delete the minimal set of nodes

chrome/browser/resources/md_bookmarks/command_manager.js

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 Polymer({
   is: 'bookmarks-command-manager',
 
   behaviors: [
     bookmarks.StoreClient,
   ],
@@ skipping 85 matching lines @@
         /** @type {!BookmarksEditDialogElement} */ (this.$.editDialog.get())
             .showEditDialog(this.getState().nodes[id]);
         break;
       case Command.COPY:
         var idList = Array.from(itemIds);
         chrome.bookmarkManagerPrivate.copy(idList, function() {
           // TODO(jiaxi): Add toast later.
         });
         break;
       case Command.DELETE:
-        // TODO(tsergeant): Filter IDs so we don't try to delete children of
-        // something else already being deleted.
         chrome.bookmarkManagerPrivate.removeTrees(
-            Array.from(itemIds), function() {
+            Array.from(this.minimizeDeletionSet_(itemIds)), function() {
               // TODO(jiaxi): Add toast later.
             });
         break;
     }
-
   },
 
   ////////////////////////////////////////////////////////////////////////////
   // Private functions:
 
   /**
+   * Minimize the set of |itemIds| by removing any node which has an ancestor
+   * node already in the set. This ensures that instead of trying to delete both
+   * a node and its descendant, we will only try to delete the topmost node.

[calamity, 2017/05/05 06:20:41]

Mention why this is necessary.

[tsergeant, 2017/05/05 06:38:56]

Done.

+   * @param {!Set<string>} itemIds
+   * @return {!Set<string>}
+   */
+  minimizeDeletionSet_: function(itemIds) {
+    var minimizedSet = new Set();
+    var nodes = this.getState().nodes;
+    itemIds.forEach(function(itemId) {
+      var currentId = itemId;
+      while (currentId != ROOT_NODE_ID) {
+        var currentId = assert(nodes[currentId].parentId);

[calamity, 2017/05/05 06:20:41]

nit: extra var.

[tsergeant, 2017/05/05 06:38:56]

oops

+        if (itemIds.has(currentId))
+          return;
+      }
+      minimizedSet.add(itemId);
+    });
+    return minimizedSet;
+  },
+
+  /**
    * @param {!Set<string>} itemIds
    * @param {function(BookmarkNode):boolean} predicate
    * @return {boolean} True if any node in |itemIds| returns true for
    *     |predicate|.
    */
   containsMatchingNode_: function(itemIds, predicate) {
     var nodes = this.getState().nodes;
 
     return Array.from(itemIds).some(function(id) {
       return predicate(nodes[id]);
@@ skipping 60 matching lines @@
   getEditActionLabel_: function() {
     if (this.menuIds_.size > 1)
       return;
 
     var id = Array.from(this.menuIds_)[0];
     var itemUrl = this.getState().nodes[id].url;
     var label = itemUrl ? 'menuEdit' : 'menuRename';
     return loadTimeData.getString(label);
   },
 });