Bookmark manager: Only load the relevant subtree to update the commands.

BUG=41184
TEST=None

Review URL: http://codereview.chromium.org/1610022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44285 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 49 insertions, 112 deletions

diff --git a/chrome/browser/resources/bookmark_manager/js/bmm.js b/chrome/browser/resources/bookmark_manager/js/bmm.js
index 73cefa6..b02d850 100644
--- a/chrome/browser/resources/bookmark_manager/js/bmm.js
+++ b/chrome/browser/resources/bookmark_manager/js/bmm.js
@@ -30,63 +30,38 @@ cr.define('bmm', function() {
     return !('url' in node);
   }
 
-  var loadingPromise;
+  var loadingPromises = {};
 
   /**
-   * Loads the entire bookmark tree and returns a {@code cr.Promise} that will
-   * be fulfilled when done. This reuses multiple loads so that we never load
-   * more than one tree at the same time.
+   * Loads a subtree of the bookmark tree and returns a {@code cr.Promise} that
+   * will be fulfilled when done. This reuses multiple loads so that we do not
+   * load the same subtree more than once at the same time.
    * @return {!cr.Promise} The future promise for the load.
    */
-  function loadTree() {
+  function loadSubtree(id) {
     var p = new Promise;
-    if (!loadingPromise) {
-      loadingPromise = new Promise;
-      chrome.bookmarks.getTree(function(nodes) {
-        loadingPromise.value = nodes[0];
-        loadingPromise = null;
+    if (!(id in loadingPromises)) {
+      loadingPromises[id] = new Promise;
+      chrome.experimental.bookmarkManager.getSubtree(id, false,
+                                                     function(nodes) {
+        loadingPromises[id].value = nodes[0];
+        delete loadingPromises[id];
       });
     }
-    loadingPromise.addListener(function(n) {
+    loadingPromises[id].addListener(function(n) {
       p.value = n;
     });
     return p;
   }
 
   /**
-   * Helper function for {@code loadSubtree}. This does an in order search of
-   * the tree.
-   * @param {!BookmarkTreeNode} node The node to start searching at.
-   * @param {string} id The ID of the node to find.
-   * @return {BookmarkTreeNode} The found node or null if not found.
-   */
-  function findNode(node, id) {
-    var it = new TreeIterator(node);
-    var n;
-    while (it.moveNext()) {
-      n = it.current;
-      if (n.id == id)
-        return n;
-    }
-    return null;
-  }
-
-  /**
-   * Loads a subtree of the bookmark tree and returns a {@code cr.Promise} that
-   * will be fulfilled when done. This reuses multiple loads so that we never
-   * load more than one tree at the same time. (This actually loads the entire
-   * tree but it will only return the relevant subtree in the value of the
-   * future promise.)
+   * Loads the entire bookmark tree and returns a {@code cr.Promise} that will
+   * be fulfilled when done. This reuses multiple loads so that we do not load
+   * the same tree more than once at the same time.
    * @return {!cr.Promise} The future promise for the load.
    */
-  function loadSubtree(id) {
-    var p = new Promise;
-    var lp = loadTree();
-    lp.addListener(function(tree) {
-      var node = findNode(tree, id);
-      p.value = node || Error('Failed to load subtree ' + id);
-    });
-    return p;
+  function loadTree() {
+    return loadSubtree('');
   }
 
   return {
diff --git a/chrome/browser/resources/bookmark_manager/js/bmm_test.html b/chrome/browser/resources/bookmark_manager/js/bmm_test.html
index c336b4e..2515b6d 100644
--- a/chrome/browser/resources/bookmark_manager/js/bmm_test.html
+++ b/chrome/browser/resources/bookmark_manager/js/bmm_test.html
@@ -32,24 +32,32 @@ var tree = {
   ]
 };
 
-// Mock chrome.bookmarks.getTree
-crome = chrome || {};
-crome.bookmarks = chrome.bookmarks || {};
-chrome.bookmarks.getTree = function f(callback) {
-  f.callbacks_ = f.callbacks_ || [];
-  f.callbacks_.push(callback);
-  f.$calls++;
+// Mock chrome.experimental.bookmarkManager.getSubtree
+chrome = chrome || {};
+chrome.experimental = chrome.experimental || {};
+chrome.experimental.bookmarkManager = chrome.experimental.bookmarkManager || {};
+
+var callbacks = {};
+
+chrome.experimental.bookmarkManager.getSubtree = function(id, foldersOnly,
+                                                          callback) {
+  callbacks[id] = callbacks[id] || [];
+  callbacks[id].push(callback);
+  callbacks[id].$calls = callbacks[id].$calls ? callbacks[id].$calls++ : 1;
 };
-chrome.bookmarks.getTree.load = function(node) {
-  var callbacks = chrome.bookmarks.getTree.callbacks_;
-  for (var i = 0; i < callbacks.length; i++) {
-    callbacks[i].call(null, [node]);
+
+chrome.experimental.bookmarkManager.getSubtree.load = function(node) {
+  // getSubtree gets the root tree when id is ''.
+  var id = node.id;
+  if (id == tree.id)
+    id = '';
+  for (var i = 0; i < callbacks[id].length; i++) {
+    callbacks[id][i].call(null, [node]);
   }
-  chrome.bookmarks.getTree.callbacks_ = [];
 };
 
 function setUp() {
-  chrome.bookmarks.getTree.$calls = 0;
+  callbacks = {}
 }
 
 function testLoadSingle() {
@@ -61,9 +69,10 @@ function testLoadSingle() {
   var p = bmm.loadTree();
   p.addListener(f);
 
-  chrome.bookmarks.getTree.load(tree);
+  chrome.experimental.bookmarkManager.getSubtree.load(tree);
+
   assertEquals(1, calls);
-  assertEquals(1, chrome.bookmarks.getTree.$calls);
+  assertEquals(1, callbacks[''].$calls);
 }
 
 function testLoadMultiple() {
@@ -79,13 +88,15 @@ function testLoadMultiple() {
   }
 
   var p = bmm.loadTree();
+  var p2 = bmm.loadTree();
   p.addListener(f1);
-  p.addListener(f2);
+  p2.addListener(f2);
+
+  chrome.experimental.bookmarkManager.getSubtree.load(tree);
 
-  chrome.bookmarks.getTree.load(tree);
   assertEquals(1, calls1);
   assertEquals(1, calls2);
-  assertEquals(1, chrome.bookmarks.getTree.$calls);
+  assertEquals(1, callbacks[''].$calls);
 }
 
 function testLoadSubtree() {
@@ -97,59 +108,10 @@ function testLoadSubtree() {
   var p = bmm.loadSubtree(1);
   p.addListener(f);
 
-  chrome.bookmarks.getTree.load(tree);
-  assertEquals(1, calls);
-  assertEquals(1, chrome.bookmarks.getTree.$calls);
-}
-
-function testLoadMixed() {
-  var calls1 = 0;
-  var calls2 = 0;
-  function f1(node) {
-    calls1++;
-    assertEquals(tree.children[0], node);
-  }
-  function f2(node) {
-    calls2++;
-    assertEquals(tree, node);
-  }
-  var p1 = bmm.loadSubtree(1);
-  p1.addListener(f1);
-  var p2 = bmm.loadTree(1);
-  p2.addListener(f2);
-
-  chrome.bookmarks.getTree.load(tree);
-  assertEquals(1, calls1);
-  assertEquals(1, calls2);
-  assertEquals(1, chrome.bookmarks.getTree.$calls);
-}
-
-function testLoadTwice() {
-  var calls1 = 0;
-  var calls2 = 0;
-  function f1(node) {
-    calls1++;
-    assertEquals(tree.children[0], node);
-  }
-  function f2(node) {
-    calls2++;
-    assertEquals(tree, node);
-  }
-  var p1 = bmm.loadSubtree(1);
-  p1.addListener(f1);
-
-  chrome.bookmarks.getTree.load(tree);
-  assertEquals(1, calls1);
-  assertEquals(0, calls2);
-  assertEquals(1, chrome.bookmarks.getTree.$calls);
-
-  var p2 = bmm.loadTree(1);
-  p2.addListener(f2);
+  chrome.experimental.bookmarkManager.getSubtree.load(tree.children[0]);
 
-  chrome.bookmarks.getTree.load(tree);
-  assertEquals(1, calls1);
-  assertEquals(1, calls2);
-  assertEquals(2, chrome.bookmarks.getTree.$calls);
+  assertEquals(1, calls);
+  assertEquals(1, callbacks[1].$calls);
 }
 
 </script>