[Enhanced Bookmark] Add GetAllFoldersWithDepth in bookmark bridge

chrome/browser/android/bookmarks/bookmarks_bridge.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/android/bookmarks/bookmarks_bridge.h"
 
 #include "base/android/jni_string.h"
 #include "base/containers/stack_container.h"
 #include "base/i18n/string_compare.h"
 #include "base/prefs/pref_service.h"
@@ skipping 227 matching lines @@
 
   for (std::vector<const BookmarkNode*>::const_iterator it =
       top_level_folders.begin(); it != top_level_folders.end(); ++it) {
     Java_BookmarksBridge_addToBookmarkIdList(env,
                                              j_result_obj,
                                              (*it)->id(),
                                              GetBookmarkType(*it));
   }
 }
 
+void BookmarksBridge::GetAllFoldersWithDepths(JNIEnv* env,
+                                              jobject obj,
+                                              jobject j_folder_obj,
+                                              jobject j_depth_obj) {
+  DCHECK(IsLoaded());
+
+  const BookmarkNode* desktop = bookmark_model_->bookmark_bar_node();
+  const BookmarkNode* mobile = bookmark_model_->mobile_node();
+  const BookmarkNode* other = bookmark_model_->other_node();
+
+  scoped_ptr<icu::Collator> collator = GetICUCollator();
+
+  // Vector to temporarily contain all child bookmarks at same level for sorting
+  std::vector<const BookmarkNode*> vct;

[Yaron, 2014/09/05 03:30:58]

nit: prefer meaningful names over abbreviated names.
allBookmarks/bookmarks/bookmarkList would all be better

[Ian Wen, 2014/09/05 21:17:32]

Done.

+  // Stack for Depth-First Search of bookmark model. It stores nodes and their
+  // heights.
+  std::stack<std::pair<const BookmarkNode*, int> > stk;

[Yaron, 2014/09/05 03:30:59]

stack/bookmarkDepth?

[Ian Wen, 2014/09/05 21:17:32]

Stack is the symbolic sign of depth first traversal of the tree. Here it is
solely used for dfs traversal as a "first in last out" container. I believe in
this case stack had better be named as stack or stk...

+
+  for (int i = 0; i < mobile->child_count(); ++i) {
+    vct.push_back(mobile->GetChild(i));
+  }
+  for (int i = 0; i < other->child_count(); ++i) {
+    vct.push_back(other->GetChild(i));

[Yaron, 2014/09/05 03:30:59]

as new bookmarks by default are added to "other" and you aren't filtering before
sorting on (L273) this will get unnecessarily expensive over time. I'd say to
add the is_folder check here and above.

[Ian Wen, 2014/09/05 21:17:32]

Done.

+  }
+  vct.push_back(desktop);
+  std::stable_sort(
+      vct.begin(), vct.end(), BookmarkTitleComparer(collator.get()));
+
+  // Push all sorted top folders in stack and give them depth of 0.
+  for (std::vector<const BookmarkNode*>::reverse_iterator it = vct.rbegin();

[Yaron, 2014/09/05 03:30:58]

why reverse_iterator? also, why not use const_iterator?

[Ian Wen, 2014/09/05 21:17:32]

Changed to const_reverse_iterator.

In vector, folders are ordered from A to Z. I also need to push them to a stack
for DFS traversal. To ensure same order in final output, they should be pushed
to stack from Z to A and be popped out from A to Z in traversal.

+       it != vct.rend();
+       ++it) {
+    stk.push(std::make_pair(*it, 0));
+  }
+
+  while (!stk.empty()) {
+    const BookmarkNode* node = stk.top().first;
+    int depth = stk.top().second;
+    stk.pop();
+    if (!node->is_folder() || !client_->CanBeEditedByUser(node))
+      continue;
+    Java_BookmarksBridge_addToBookmarkIdListWithDepth(env,
+                                                      j_folder_obj,
+                                                      node->id(),
+                                                      GetBookmarkType(node),
+                                                      j_depth_obj,
+                                                      depth);
+    vct.clear();
+    for (int i = 0; i < node->child_count(); ++i) {
+      vct.push_back(node->GetChild(i));

[Yaron, 2014/09/05 03:30:59]

same comment about effeciency

[Ian Wen, 2014/09/05 21:17:32]

Done.

+    }
+    std::stable_sort(
+        vct.begin(), vct.end(), BookmarkTitleComparer(collator.get()));
+    for (std::vector<const BookmarkNode*>::reverse_iterator it = vct.rbegin();
+         it != vct.rend();
+         ++it) {
+      stk.push(std::make_pair(*it, depth + 1));
+    }
+  }
+}
+
+ScopedJavaLocalRef<jobject> BookmarksBridge::GetMobileFolderId(JNIEnv* env,
+                                                               jobject obj) {
+  const BookmarkNode* mobileNode = bookmark_model_->mobile_node();
+  ScopedJavaLocalRef<jobject> folder_id_obj =
+      Java_BookmarksBridge_createBookmarkId(
+          env, mobileNode->id(), GetBookmarkType(mobileNode));
+  return folder_id_obj;
+}
+
 void BookmarksBridge::GetChildIDs(JNIEnv* env,
                                   jobject obj,
                                   jlong id,
                                   jint type,
                                   jboolean get_folders,
                                   jboolean get_bookmarks,
                                   jobject j_result_obj) {
   DCHECK(IsLoaded());
 
   const BookmarkNode* parent = GetNodeByID(id, type);
@@ skipping 495 matching lines @@
   BookmarkModelChanged();
 }
 
 void BookmarksBridge::PartnerShimLoaded(PartnerBookmarksShim* shim) {
   NotifyIfDoneLoading();
 }
 
 void BookmarksBridge::ShimBeingDeleted(PartnerBookmarksShim* shim) {
   partner_bookmarks_shim_ = NULL;
 }