From efe56eafb8e74f07c009bc384d15675ac2d50c6b Mon Sep 17 00:00:00 2001 From: ianwen Date: Thu, 18 Sep 2014 18:00:26 -0700 Subject: [PATCH] Instrumental test for BookmarksBridge. It currently tests these functionalities: addBookmark, addFolder and getAllFoldersWithDepths. BUG=411545 Review URL: https://codereview.chromium.org/550543003 Cr-Commit-Position: refs/heads/master@{#295627} --- .../chromium/chrome/browser/BookmarksBridge.java | 76 ++++++++++++ chrome/android/javatests/DEPS | 1 + .../chrome/browser/BookmarksBridgeTest.java | 137 +++++++++++++++++++++ .../browser/android/bookmarks/bookmarks_bridge.cc | 80 +++++++++++- .../browser/android/bookmarks/bookmarks_bridge.h | 26 +++- 5 files changed, 315 insertions(+), 5 deletions(-) create mode 100644 chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java 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 110c3e521b35..6a79fa53e799 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java @@ -6,8 +6,10 @@ package org.chromium.chrome.browser; import org.chromium.base.CalledByNative; import org.chromium.base.ObserverList; +import org.chromium.base.VisibleForTesting; import org.chromium.chrome.browser.profiles.Profile; import org.chromium.components.bookmarks.BookmarkId; +import org.chromium.components.bookmarks.BookmarkType; import java.util.ArrayList; import java.util.List; @@ -162,6 +164,15 @@ public class BookmarksBridge { } /** + * Load an empty partner bookmark shim for testing. The root node for bookmark will be an + * empty node. + */ + @VisibleForTesting + public void loadEmptyPartnerBookmarkShimForTesting() { + nativeLoadEmptyPartnerBookmarkShimForTesting(mNativeBookmarksBridge); + } + + /** * Add an observer to bookmark model changes. * @param observer The observer to be added. */ @@ -263,6 +274,24 @@ public class BookmarksBridge { } /** + * @return Id representing the special "other" folder from bookmark model. + */ + @VisibleForTesting + public BookmarkId getOtherFolderId() { + assert mIsNativeBookmarkModelLoaded; + return nativeGetOtherFolderId(mNativeBookmarksBridge); + } + + /** + * @return BokmarkId representing special "desktop" folder, namely "bookmark bar". + */ + @VisibleForTesting + public BookmarkId getDesktopFolderId() { + assert mIsNativeBookmarkModelLoaded; + return nativeGetDesktopFolderId(mNativeBookmarksBridge); + } + + /** * Reads sub-folder IDs, sub-bookmark IDs, or both of the given folder. * * @param getFolders Whether sub-folders should be returned. @@ -382,6 +411,46 @@ public class BookmarksBridge { } /** + * Add a new folder to the given parent folder + * + * @param parent Folder where to add. Must be a normal editable folder, instead of a partner + * bookmark folder or a managed bookomark folder or root node of the entire + * bookmark model. + * @param index The position to locate the new folder + * @param title The title text of the new folder + * @return Id of the added node. If adding failed (index is invalid, string is null, parent is + * not editable), returns null. + */ + public BookmarkId addFolder(BookmarkId parent, int index, String title) { + assert parent.getType() == BookmarkType.BOOKMARK_TYPE_NORMAL; + assert index >= 0; + assert title != null; + + return nativeAddFolder(mNativeBookmarksBridge, parent, index, title); + } + + /** + * Add a new bookmark to a specific position below parent + * + * @param parent Folder where to add. Must be a normal editable folder, instead of a partner + * bookmark folder or a managed bookomark folder or root node of the entire + * bookmark model. + * @param index The position where the bookmark will be placed in parent folder + * @param title Title of the new bookmark + * @param url Url of the new bookmark + * @return Id of the added node. If adding failed (index is invalid, string is null, parent is + * not editable), returns null. + */ + public BookmarkId addBookmark(BookmarkId parent, int index, String title, String url) { + assert parent.getType() == BookmarkType.BOOKMARK_TYPE_NORMAL; + assert index >= 0; + assert title != null; + assert url != null; + + return nativeAddBookmark(mNativeBookmarksBridge, parent, index, title, url); + } + + /** * A bridge function to BookmarkModelFactory::GetForProfile. */ public static long getNativeBookmarkModel(Profile profile) { @@ -525,6 +594,8 @@ public class BookmarksBridge { private native void nativeGetAllFoldersWithDepths(long nativeBookmarksBridge, List folderList, List depthList); private native BookmarkId nativeGetMobileFolderId(long nativeBookmarksBridge); + private native BookmarkId nativeGetOtherFolderId(long nativeBookmarksBridge); + private native BookmarkId nativeGetDesktopFolderId(long nativeBookmarksBridge); private native void nativeGetChildIDs(long nativeBookmarksBridge, long id, int type, boolean getFolders, boolean getBookmarks, List bookmarksList); private native void nativeGetAllBookmarkIDsOrderedByCreationDate(long nativeBookmarksBridge, @@ -540,11 +611,16 @@ public class BookmarksBridge { private native void nativeGetCurrentFolderHierarchy(long nativeBookmarksBridge, BookmarkId folderId, BookmarksCallback callback, List bookmarksList); + private native BookmarkId nativeAddFolder(long nativeBookmarksBridge, BookmarkId parent, + int index, String title); private native void nativeDeleteBookmark(long nativeBookmarksBridge, BookmarkId bookmarkId); private native void nativeMoveBookmark(long nativeBookmarksBridge, BookmarkId bookmarkId, BookmarkId newParentId, int index); + private native BookmarkId nativeAddBookmark(long nativeBookmarksBridge, BookmarkId parent, + int index, String title, String url); private static native long nativeGetNativeBookmarkModel(Profile profile); private static native boolean nativeIsEnhancedBookmarksFeatureEnabled(Profile profile); + private native void nativeLoadEmptyPartnerBookmarkShimForTesting(long nativeBookmarksBridge); private native long nativeInit(Profile profile); private native boolean nativeIsDoingExtensiveChanges(long nativeBookmarksBridge); private native void nativeDestroy(long nativeBookmarksBridge); diff --git a/chrome/android/javatests/DEPS b/chrome/android/javatests/DEPS index c3c100bdf3f4..c3995586984c 100644 --- a/chrome/android/javatests/DEPS +++ b/chrome/android/javatests/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core", + "+components/bookmarks/common/android/java/src/org/chromium/components/bookmarks", "+content/public/android/java", "+sync/android/java/src/org/chromium/sync/internal_api/pub", "+sync/android/java/src/org/chromium/sync/notifier", diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java new file mode 100644 index 000000000000..43392e5f1931 --- /dev/null +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java @@ -0,0 +1,137 @@ +// Copyright 2014 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. + +package org.chromium.chrome.browser; + +import android.test.UiThreadTest; +import android.test.suitebuilder.annotation.SmallTest; + +import org.chromium.base.ThreadUtils; +import org.chromium.base.test.util.Feature; +import org.chromium.chrome.browser.BookmarksBridge.BookmarkItem; +import org.chromium.chrome.browser.profiles.Profile; +import org.chromium.chrome.shell.ChromeShellActivity; +import org.chromium.chrome.shell.ChromeShellTab; +import org.chromium.chrome.shell.ChromeShellTestBase; +import org.chromium.components.bookmarks.BookmarkId; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +/** + * Tests for bookmark bridge + */ +public class BookmarksBridgeTest extends ChromeShellTestBase { + + private ChromeShellActivity mActivity; + private Profile mProfile; + private BookmarksBridge mBookmarksBridge; + private BookmarkId mMobileNode; + private BookmarkId mOtherNode; + private BookmarkId mDesktopNode; + + @Override + protected void setUp() throws Exception { + super.setUp(); + mActivity = launchChromeShellWithBlankPage(); + assertTrue(waitForActiveShellToBeDoneLoading()); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + ChromeShellTab tab = mActivity.getActiveTab(); + mProfile = tab.getProfile(); + mBookmarksBridge = new BookmarksBridge(mProfile); + mBookmarksBridge.loadEmptyPartnerBookmarkShimForTesting(); + mMobileNode = mBookmarksBridge.getMobileFolderId(); + mDesktopNode = mBookmarksBridge.getDesktopFolderId(); + mOtherNode = mBookmarksBridge.getOtherFolderId(); + } + }); + } + + @UiThreadTest + @SmallTest + @Feature({"Bookmark"}) + public void testAddBookmarksAndFolders() { + BookmarkId bookmarkA = mBookmarksBridge.addBookmark(mDesktopNode, 0, "a", "http://a.com"); + verifyBookmark(bookmarkA, "a", "http://a.com/", false, mDesktopNode); + BookmarkId bookmarkB = mBookmarksBridge.addBookmark(mOtherNode, 0, "b", "http://b.com"); + verifyBookmark(bookmarkB, "b", "http://b.com/", false, mOtherNode); + BookmarkId bookmarkC = mBookmarksBridge.addBookmark(mMobileNode, 0, "c", "http://c.com"); + verifyBookmark(bookmarkC, "c", "http://c.com/", false, mMobileNode); + BookmarkId folderA = mBookmarksBridge.addFolder(mOtherNode, 0, "fa"); + verifyBookmark(folderA, "fa", null, true, mOtherNode); + BookmarkId folderB = mBookmarksBridge.addFolder(mDesktopNode, 0, "fb"); + verifyBookmark(folderB, "fb", null, true, mDesktopNode); + BookmarkId folderC = mBookmarksBridge.addFolder(mMobileNode, 0, "fc"); + verifyBookmark(folderC, "fc", null, true, mMobileNode); + BookmarkId bookmarkAA = mBookmarksBridge.addBookmark(folderA, 0, "aa", "http://aa.com"); + verifyBookmark(bookmarkAA, "aa", "http://aa.com/", false, folderA); + BookmarkId folderAA = mBookmarksBridge.addFolder(folderA, 0, "faa"); + verifyBookmark(folderAA, "faa", null, true, folderA); + } + + private void verifyBookmark(BookmarkId idToVerify, String expectedTitle, + String expectedUrl, boolean isFolder, BookmarkId expectedParent) { + assertNotNull(idToVerify); + BookmarkItem item = mBookmarksBridge.getBookmarkById(idToVerify); + assertEquals(expectedTitle, item.getTitle()); + assertEquals(item.isFolder(), isFolder); + if (!isFolder) assertEquals(expectedUrl, item.getUrl()); + assertEquals(item.getParentId(), expectedParent); + } + + @UiThreadTest + @SmallTest + @Feature({"Bookmark"}) + public void testGetAllFoldersWithDepths() { + BookmarkId folderA = mBookmarksBridge.addFolder(mMobileNode, 0, "a"); + BookmarkId folderB = mBookmarksBridge.addFolder(mDesktopNode, 0, "b"); + BookmarkId folderC = mBookmarksBridge.addFolder(mOtherNode, 0, "c"); + BookmarkId folderAA = mBookmarksBridge.addFolder(folderA, 0, "aa"); + BookmarkId folderBA = mBookmarksBridge.addFolder(folderB, 0, "ba"); + BookmarkId folderAAA = mBookmarksBridge.addFolder(folderAA, 0, "aaa"); + BookmarkId folderAAAA = mBookmarksBridge.addFolder(folderAAA, 0, "aaaa"); + + mBookmarksBridge.addBookmark(mMobileNode, 0, "ua", "http://www.google.com"); + mBookmarksBridge.addBookmark(mDesktopNode, 0, "ua", "http://www.google.com"); + mBookmarksBridge.addBookmark(mOtherNode, 0, "ua", "http://www.google.com"); + mBookmarksBridge.addBookmark(folderA, 0, "ua", "http://www.medium.com"); + + // 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(mDesktopNode, 0); + idToDepth.put(folderB, 1); + idToDepth.put(folderBA, 2); + idToDepth.put(folderC, 0); + + List folderList = new ArrayList(); + List depthList = new ArrayList(); + mBookmarksBridge.getAllFoldersWithDepths(folderList, depthList); + + assertEquals("Returned folder list has different size (" + folderList.size() + + ") from depth list (" + depthList.size() + ").", + folderList.size(), depthList.size()); + assertEquals("Returned lists have different sizes (" + folderList.size() + + ") from expected size (" + idToDepth.size() + ").", + folderList.size(), idToDepth.size()); + + for (int i = 0; i < folderList.size(); i++) { + assertNotNull(folderList.get(i)); + assertNotNull(depthList.get(i)); + assertTrue("Returned list contained unexpected key: " + folderList.get(i), + idToDepth.containsKey(folderList.get(i))); + assertEquals(depthList.get(i), idToDepth.get(folderList.get(i))); + idToDepth.remove(folderList.get(i)); + } + + assertEquals("Expected list contains elements that are not in returned list", + idToDepth.size(), 0); + } +} diff --git a/chrome/browser/android/bookmarks/bookmarks_bridge.cc b/chrome/browser/android/bookmarks/bookmarks_bridge.cc index 55fa1ac0bbc9..fe7c99b8705f 100644 --- a/chrome/browser/android/bookmarks/bookmarks_bridge.cc +++ b/chrome/browser/android/bookmarks/bookmarks_bridge.cc @@ -146,6 +146,15 @@ static jboolean IsEditBookmarksEnabled(JNIEnv* env, jclass clazz) { return IsEditBookmarksEnabled(); } +void BookmarksBridge::LoadEmptyPartnerBookmarkShimForTesting(JNIEnv* env, + jobject obj) { + if (partner_bookmarks_shim_->IsLoaded()) + return; + partner_bookmarks_shim_->SetPartnerBookmarksRoot( + new BookmarkPermanentNode(0)); + DCHECK(partner_bookmarks_shim_->IsLoaded()); +} + ScopedJavaLocalRef BookmarksBridge::GetBookmarkByID(JNIEnv* env, jobject obj, jlong id, @@ -356,10 +365,28 @@ void BookmarksBridge::GetAllFoldersWithDepths(JNIEnv* env, ScopedJavaLocalRef BookmarksBridge::GetMobileFolderId(JNIEnv* env, jobject obj) { - const BookmarkNode* mobileNode = bookmark_model_->mobile_node(); + const BookmarkNode* mobile_node = bookmark_model_->mobile_node(); ScopedJavaLocalRef folder_id_obj = Java_BookmarksBridge_createBookmarkId( - env, mobileNode->id(), GetBookmarkType(mobileNode)); + env, mobile_node->id(), GetBookmarkType(mobile_node)); + return folder_id_obj; +} + +ScopedJavaLocalRef BookmarksBridge::GetOtherFolderId(JNIEnv* env, + jobject obj) { + const BookmarkNode* other_node = bookmark_model_->other_node(); + ScopedJavaLocalRef folder_id_obj = + Java_BookmarksBridge_createBookmarkId( + env, other_node->id(), GetBookmarkType(other_node)); + return folder_id_obj; +} + +ScopedJavaLocalRef BookmarksBridge::GetDesktopFolderId(JNIEnv* env, + jobject obj) { + const BookmarkNode* desktop_node = bookmark_model_->bookmark_bar_node(); + ScopedJavaLocalRef folder_id_obj = + Java_BookmarksBridge_createBookmarkId( + env, desktop_node->id(), GetBookmarkType(desktop_node)); return folder_id_obj; } @@ -550,6 +577,28 @@ void BookmarksBridge::GetCurrentFolderHierarchy(JNIEnv* env, env, j_callback_obj, j_folder_id_obj, j_result_obj); } +ScopedJavaLocalRef BookmarksBridge::AddFolder(JNIEnv* env, + jobject obj, + jobject j_parent_id_obj, + jint index, + jstring j_title) { + DCHECK(IsLoaded()); + long bookmark_id = JavaBookmarkIdGetId(env, j_parent_id_obj); + int type = JavaBookmarkIdGetType(env, j_parent_id_obj); + const BookmarkNode* parent = GetNodeByID(bookmark_id, type); + + const BookmarkNode* new_node = bookmark_model_->AddFolder( + parent, index, base::android::ConvertJavaStringToUTF16(env, j_title)); + if (!new_node) { + NOTREACHED(); + return ScopedJavaLocalRef(); + } + ScopedJavaLocalRef new_java_obj = + Java_BookmarksBridge_createBookmarkId( + env, new_node->id(), GetBookmarkType(new_node)); + return new_java_obj; +} + void BookmarksBridge::DeleteBookmark(JNIEnv* env, jobject obj, jobject j_bookmark_id_obj) { @@ -593,6 +642,33 @@ void BookmarksBridge::MoveBookmark(JNIEnv* env, bookmark_model_->Move(node, new_parent_node, index); } +ScopedJavaLocalRef BookmarksBridge::AddBookmark( + JNIEnv* env, + jobject obj, + jobject j_parent_id_obj, + jint index, + jstring j_title, + jstring j_url) { + DCHECK(IsLoaded()); + long bookmark_id = JavaBookmarkIdGetId(env, j_parent_id_obj); + int type = JavaBookmarkIdGetType(env, j_parent_id_obj); + const BookmarkNode* parent = GetNodeByID(bookmark_id, type); + + const BookmarkNode* new_node = bookmark_model_->AddURL( + parent, + index, + base::android::ConvertJavaStringToUTF16(env, j_title), + GURL(base::android::ConvertJavaStringToUTF16(env, j_url))); + if (!new_node) { + NOTREACHED(); + return ScopedJavaLocalRef(); + } + ScopedJavaLocalRef new_java_obj = + Java_BookmarksBridge_createBookmarkId( + env, new_node->id(), GetBookmarkType(new_node)); + return new_java_obj; +} + ScopedJavaLocalRef BookmarksBridge::CreateJavaBookmark( const BookmarkNode* node) { JNIEnv* env = AttachCurrentThread(); diff --git a/chrome/browser/android/bookmarks/bookmarks_bridge.h b/chrome/browser/android/bookmarks/bookmarks_bridge.h index 84a3e9bf1e98..91fc2f0f1110 100644 --- a/chrome/browser/android/bookmarks/bookmarks_bridge.h +++ b/chrome/browser/android/bookmarks/bookmarks_bridge.h @@ -31,6 +31,8 @@ class BookmarksBridge : public BaseBookmarkModelObserver, bool IsDoingExtensiveChanges(JNIEnv* env, jobject obj); + void LoadEmptyPartnerBookmarkShimForTesting(JNIEnv* env, jobject obj); + base::android::ScopedJavaLocalRef GetBookmarkByID( JNIEnv* env, jobject obj, @@ -62,6 +64,12 @@ class BookmarksBridge : public BaseBookmarkModelObserver, base::android::ScopedJavaLocalRef GetMobileFolderId(JNIEnv* env, jobject obj); + base::android::ScopedJavaLocalRef GetOtherFolderId(JNIEnv* env, + jobject obj); + + base::android::ScopedJavaLocalRef GetDesktopFolderId(JNIEnv* env, + jobject obj); + void GetChildIDs(JNIEnv* env, jobject obj, jlong id, @@ -100,9 +108,13 @@ class BookmarksBridge : public BaseBookmarkModelObserver, jobject j_callback_obj, jobject j_result_obj); - void DeleteBookmark(JNIEnv* env, - jobject obj, - jobject j_bookmark_id_obj); + base::android::ScopedJavaLocalRef AddFolder(JNIEnv* env, + jobject obj, + jobject j_parent_id_obj, + jint index, + jstring j_title); + + void DeleteBookmark(JNIEnv* env, jobject obj, jobject j_bookmark_id_obj); void MoveBookmark(JNIEnv* env, jobject obj, @@ -110,6 +122,14 @@ class BookmarksBridge : public BaseBookmarkModelObserver, jobject j_parent_id_obj, jint index); + base::android::ScopedJavaLocalRef AddBookmark( + JNIEnv* env, + jobject obj, + jobject j_parent_id_obj, + jint index, + jstring j_title, + jstring j_url); + private: virtual ~BookmarksBridge(); -- 2.11.4.GIT