From dc6f13b4f91b60f22faed3083386be1caec57a37 Mon Sep 17 00:00:00 2001 From: Kibeom Kim Date: Mon, 2 Feb 2015 13:45:42 -0800 Subject: [PATCH] [Android] Change enhanced bookmark folder structure. - Top level folders are now all the sub folders of Bookmark Bar, Mobile, and Others nodes. - We no longer use Uncategorized. BUG=453024 Review URL: https://codereview.chromium.org/869193008 Cr-Commit-Position: refs/heads/master@{#314013} (cherry picked from commit a01c664cd6802d64d212f68d1c9392a4340ca835) TBR=newt@chromium.org Review URL: https://codereview.chromium.org/894853002 Cr-Commit-Position: refs/branch-heads/2272@{#188} Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958} --- .../chrome/browser/BookmarksBridge.java | 19 +----- .../chrome/browser/BookmarksBridgeTest.java | 48 +++++++++----- .../android/bookmarks/bookmarks_bridge.cc | 65 ++++--------------- .../android/bookmarks/bookmarks_bridge.h | 3 - 4 files changed, 45 insertions(+), 90 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java index 479ec7ccfbff3..b31f59732f488 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java @@ -225,7 +225,7 @@ public List getPermanentNodeIDs() { } /** - * @return The top level folder's parents, which are root node, mobile node, and other node. + * @return The top level folder's parents. */ public List getTopLevelFolderParentIDs() { assert mIsNativeBookmarkModelLoaded; @@ -238,9 +238,7 @@ public List getTopLevelFolderParentIDs() { * @param getSpecial Whether special top folders should be returned. * @param getNormal Whether normal top folders should be returned. * @return The top level folders. Note that special folders come first and normal top folders - * will be in the alphabetical order. Special top folders are managed bookmark and - * partner bookmark. Normal top folders are desktop permanent folder, and the - * sub-folders of mobile permanent folder and others permanent folder. + * will be in the alphabetical order. */ public List getTopLevelFolderIDs(boolean getSpecial, boolean getNormal) { assert mIsNativeBookmarkModelLoaded; @@ -249,17 +247,6 @@ public List getTopLevelFolderIDs(boolean getSpecial, boolean getNorm return result; } - /** - * @return The uncategorized bookmark IDs. They are direct descendant bookmarks of mobile and - * other folders. - */ - public List getUncategorizedBookmarkIDs() { - assert mIsNativeBookmarkModelLoaded; - List result = new ArrayList(); - nativeGetUncategorizedBookmarkIDs(mNativeBookmarksBridge, result); - return result; - } - /** * Populates folderList with BookmarkIds of folders users can move bookmarks * to and all folders have corresponding depth value in depthList. Folders @@ -702,8 +689,6 @@ private native void nativeGetTopLevelFolderParentIDs(long nativeBookmarksBridge, List bookmarksList); private native void nativeGetTopLevelFolderIDs(long nativeBookmarksBridge, boolean getSpecial, boolean getNormal, List bookmarksList); - private native void nativeGetUncategorizedBookmarkIDs(long nativeBookmarksBridge, - List bookmarksList); private native void nativeGetAllFoldersWithDepths(long nativeBookmarksBridge, List folderList, List depthList); private native BookmarkId nativeGetMobileFolderId(long nativeBookmarksBridge); diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java index bc050c0a9ad57..90e3a9f871f69 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java @@ -103,14 +103,16 @@ public void testGetAllFoldersWithDepths() { // Map folders to depths as expected results HashMap idToDepth = new HashMap(); - idToDepth.put(folderA, 0); - idToDepth.put(folderAA, 1); - idToDepth.put(folderAAA, 2); - idToDepth.put(folderAAAA, 3); + idToDepth.put(mMobileNode, 0); + idToDepth.put(folderA, 1); + idToDepth.put(folderAA, 2); + idToDepth.put(folderAAA, 3); + idToDepth.put(folderAAAA, 4); idToDepth.put(mDesktopNode, 0); idToDepth.put(folderB, 1); idToDepth.put(folderBA, 2); - idToDepth.put(folderC, 0); + idToDepth.put(mOtherNode, 0); + idToDepth.put(folderC, 1); List folderList = new ArrayList(); List depthList = new ArrayList(); @@ -141,44 +143,54 @@ public void testGetMoveDestinations() { List depthList = new ArrayList(); mBookmarksBridge.getMoveDestinations(folderList, depthList, Arrays.asList(folderA)); + idToDepth.put(mMobileNode, 0); idToDepth.put(mDesktopNode, 0); idToDepth.put(folderB, 1); idToDepth.put(folderBA, 2); - idToDepth.put(folderC, 0); + idToDepth.put(mOtherNode, 0); + idToDepth.put(folderC, 1); verifyFolderDepths(folderList, depthList, idToDepth); mBookmarksBridge.getMoveDestinations(folderList, depthList, Arrays.asList(folderB)); - idToDepth.put(folderA, 0); - idToDepth.put(folderAA, 1); - idToDepth.put(folderAAA, 2); + idToDepth.put(mMobileNode, 0); + idToDepth.put(folderA, 1); + idToDepth.put(folderAA, 2); + idToDepth.put(folderAAA, 3); idToDepth.put(mDesktopNode, 0); - idToDepth.put(folderC, 0); + idToDepth.put(mOtherNode, 0); + idToDepth.put(folderC, 1); verifyFolderDepths(folderList, depthList, idToDepth); mBookmarksBridge.getMoveDestinations(folderList, depthList, Arrays.asList(folderC)); - idToDepth.put(folderA, 0); - idToDepth.put(folderAA, 1); - idToDepth.put(folderAAA, 2); + idToDepth.put(mMobileNode, 0); + idToDepth.put(folderA, 1); + idToDepth.put(folderAA, 2); + idToDepth.put(folderAAA, 3); idToDepth.put(mDesktopNode, 0); idToDepth.put(folderB, 1); idToDepth.put(folderBA, 2); + idToDepth.put(mOtherNode, 0); verifyFolderDepths(folderList, depthList, idToDepth); mBookmarksBridge.getMoveDestinations(folderList, depthList, Arrays.asList(folderBA)); - idToDepth.put(folderA, 0); - idToDepth.put(folderAA, 1); - idToDepth.put(folderAAA, 2); + idToDepth.put(mMobileNode, 0); + idToDepth.put(folderA, 1); + idToDepth.put(folderAA, 2); + idToDepth.put(folderAAA, 3); idToDepth.put(mDesktopNode, 0); idToDepth.put(folderB, 1); - idToDepth.put(folderC, 0); + idToDepth.put(mOtherNode, 0); + idToDepth.put(folderC, 1); verifyFolderDepths(folderList, depthList, idToDepth); mBookmarksBridge.getMoveDestinations(folderList, depthList, Arrays.asList(folderAA, folderC)); - idToDepth.put(folderA, 0); + idToDepth.put(mMobileNode, 0); + idToDepth.put(folderA, 1); idToDepth.put(mDesktopNode, 0); idToDepth.put(folderB, 1); idToDepth.put(folderBA, 2); + idToDepth.put(mOtherNode, 0); verifyFolderDepths(folderList, depthList, idToDepth); } diff --git a/chrome/browser/android/bookmarks/bookmarks_bridge.cc b/chrome/browser/android/bookmarks/bookmarks_bridge.cc index 1089e789ad832..ea8f8ac679d87 100644 --- a/chrome/browser/android/bookmarks/bookmarks_bridge.cc +++ b/chrome/browser/android/bookmarks/bookmarks_bridge.cc @@ -209,12 +209,6 @@ void BookmarksBridge::GetTopLevelFolderParentIDs(JNIEnv* env, Java_BookmarksBridge_addToBookmarkIdList( env, j_result_obj, bookmark_model_->root_node()->id(), GetBookmarkType(bookmark_model_->root_node())); - Java_BookmarksBridge_addToBookmarkIdList( - env, j_result_obj, bookmark_model_->mobile_node()->id(), - GetBookmarkType(bookmark_model_->mobile_node())); - Java_BookmarksBridge_addToBookmarkIdList( - env, j_result_obj, bookmark_model_->other_node()->id(), - GetBookmarkType(bookmark_model_->other_node())); } void BookmarksBridge::GetTopLevelFolderIDs(JNIEnv* env, @@ -240,8 +234,6 @@ void BookmarksBridge::GetTopLevelFolderIDs(JNIEnv* env, if (get_normal) { DCHECK_EQ(bookmark_model_->root_node()->child_count(), 4); - top_level_folders.push_back(bookmark_model_->bookmark_bar_node()); - const BookmarkNode* mobile_node = bookmark_model_->mobile_node(); for (int i = 0; i < mobile_node->child_count(); ++i) { const BookmarkNode* node = mobile_node->GetChild(i); @@ -250,6 +242,15 @@ void BookmarksBridge::GetTopLevelFolderIDs(JNIEnv* env, } } + const BookmarkNode* bookmark_bar_node = + bookmark_model_->bookmark_bar_node(); + for (int i = 0; i < bookmark_bar_node->child_count(); ++i) { + const BookmarkNode* node = bookmark_bar_node->GetChild(i); + if (node->is_folder()) { + top_level_folders.push_back(node); + } + } + const BookmarkNode* other_node = bookmark_model_->other_node(); for (int i = 0; i < other_node->child_count(); ++i) { const BookmarkNode* node = other_node->GetChild(i); @@ -273,64 +274,24 @@ void BookmarksBridge::GetTopLevelFolderIDs(JNIEnv* env, } } -void BookmarksBridge::GetUncategorizedBookmarkIDs(JNIEnv* env, - jobject obj, - jobject j_result_obj) { - const BookmarkNode* mobile_node = bookmark_model_->mobile_node(); - for (int i = 0; i < mobile_node->child_count(); ++i) { - const BookmarkNode* node = mobile_node->GetChild(i); - if (!node->is_folder()) { - Java_BookmarksBridge_addToBookmarkIdList(env, - j_result_obj, - node->id(), - GetBookmarkType(node)); - } - } - - const BookmarkNode* other_node = bookmark_model_->other_node(); - for (int i = 0; i < other_node->child_count(); ++i) { - const BookmarkNode* node = other_node->GetChild(i); - if (!node->is_folder()) { - Java_BookmarksBridge_addToBookmarkIdList(env, - j_result_obj, - node->id(), - GetBookmarkType(node)); - } - } -} - void BookmarksBridge::GetAllFoldersWithDepths(JNIEnv* env, jobject obj, jobject j_folders_obj, jobject j_depths_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 collator = GetICUCollator(); // Vector to temporarily contain all child bookmarks at same level for sorting std::vector bookmarkList; + // Stack for Depth-First Search of bookmark model. It stores nodes and their // heights. std::stack > stk; - for (int i = 0; i < mobile->child_count(); ++i) { - const BookmarkNode* child = mobile->GetChild(i); - if (child->is_folder() && client_->CanBeEditedByUser(child)) - bookmarkList.push_back(child); - } - for (int i = 0; i < other->child_count(); ++i) { - const BookmarkNode* child = other->GetChild(i); - if (child->is_folder() && client_->CanBeEditedByUser(child)) - bookmarkList.push_back(child); - } - bookmarkList.push_back(desktop); - std::stable_sort(bookmarkList.begin(), - bookmarkList.end(), - BookmarkTitleComparer(this, collator.get())); + bookmarkList.push_back(bookmark_model_->mobile_node()); + bookmarkList.push_back(bookmark_model_->bookmark_bar_node()); + bookmarkList.push_back(bookmark_model_->other_node()); // Push all sorted top folders in stack and give them depth of 0. // Note the order to push folders to stack should be opposite to the order in diff --git a/chrome/browser/android/bookmarks/bookmarks_bridge.h b/chrome/browser/android/bookmarks/bookmarks_bridge.h index d7e1fed958dd6..9ed5d09f01906 100644 --- a/chrome/browser/android/bookmarks/bookmarks_bridge.h +++ b/chrome/browser/android/bookmarks/bookmarks_bridge.h @@ -57,9 +57,6 @@ class BookmarksBridge : public bookmarks::BaseBookmarkModelObserver, jboolean get_normal, jobject j_result_obj); - void GetUncategorizedBookmarkIDs(JNIEnv* env, - jobject obj, - jobject j_result_obj); void GetAllFoldersWithDepths(JNIEnv* env, jobject obj, jobject j_folders_obj,