Commit 41145444 authored by Theresa Wellington's avatar Theresa Wellington Committed by Commit Bot

Reenable BookmarkTest#testSearchBookmarks_Delete

This reverts commit 48a0f056 and
re-enables the #testSearchBookmarks_Delete. In order for this test to
pass on tablets, special handling of the search state while updating the
native page's URL was added.

BUG=791485,789910

Change-Id: I43133858518754fa321e563a81982432eec98b93
Reviewed-on: https://chromium-review.googlesource.com/806932Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521527}
parent 8af1ed83
...@@ -285,7 +285,15 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate, ...@@ -285,7 +285,15 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate,
if (mBookmarkModel == null) return; if (mBookmarkModel == null) return;
if (mBookmarkModel.isBookmarkModelLoaded()) { if (mBookmarkModel.isBookmarkModelLoaded()) {
BookmarkUIState searchState = null;
if (!mStateStack.isEmpty()
&& mStateStack.peek().mState == BookmarkUIState.STATE_SEARCHING) {
searchState = mStateStack.pop();
}
setState(BookmarkUIState.createStateFromUrl(url, mBookmarkModel)); setState(BookmarkUIState.createStateFromUrl(url, mBookmarkModel));
if (searchState != null) setState(searchState);
} else { } else {
mInitialUrl = url; mInitialUrl = url;
} }
...@@ -330,8 +338,9 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate, ...@@ -330,8 +338,9 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate,
} }
mStateStack.push(state); mStateStack.push(state);
if (state.mState != BookmarkUIState.STATE_LOADING) { if (state.mState == BookmarkUIState.STATE_FOLDER) {
// Loading state may be pushed to the stack but should never be stored in preferences. // Loading and searching states may be pushed to the stack but should never be stored in
// preferences.
BookmarkUtils.setLastUsedUrl(mActivity, state.mUrl); BookmarkUtils.setLastUsedUrl(mActivity, state.mUrl);
// If a loading state is replaced by another loading state, do not notify this change. // If a loading state is replaced by another loading state, do not notify this change.
if (mNativePage != null) { if (mNativePage != null) {
......
...@@ -27,7 +27,6 @@ import org.chromium.base.ApplicationStatus; ...@@ -27,7 +27,6 @@ import org.chromium.base.ApplicationStatus;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CallbackHelper; import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction; import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure; import org.chromium.base.test.util.RetryOnFailure;
...@@ -320,7 +319,6 @@ public class BookmarkTest { ...@@ -320,7 +319,6 @@ public class BookmarkTest {
@Test @Test
@MediumTest @MediumTest
@DisabledTest(message = "crbug.com/791485")
public void testSearchBookmarks_Delete() throws Exception { public void testSearchBookmarks_Delete() throws Exception {
BookmarkPromoHeader.forcePromoStateForTests(BookmarkPromoHeader.PromoState.PROMO_NONE); BookmarkPromoHeader.forcePromoStateForTests(BookmarkPromoHeader.PromoState.PROMO_NONE);
BookmarkId testFolder = addFolder(TEST_FOLDER_TITLE); BookmarkId testFolder = addFolder(TEST_FOLDER_TITLE);
...@@ -331,12 +329,14 @@ public class BookmarkTest { ...@@ -331,12 +329,14 @@ public class BookmarkTest {
BookmarkItemsAdapter adapter = ((BookmarkItemsAdapter) mItemsContainer.getAdapter()); BookmarkItemsAdapter adapter = ((BookmarkItemsAdapter) mItemsContainer.getAdapter());
BookmarkManager manager = (BookmarkManager) adapter.getDelegateForTesting(); BookmarkManager manager = (BookmarkManager) adapter.getDelegateForTesting();
Assert.assertEquals(BookmarkUIState.STATE_FOLDER, manager.getCurrentState()); Assert.assertEquals("Wrong state, should be in folder", BookmarkUIState.STATE_FOLDER,
manager.getCurrentState());
assertBookmarkItems("Wrong number of items before starting search.", 3, adapter, manager); assertBookmarkItems("Wrong number of items before starting search.", 3, adapter, manager);
// Start searching without entering a query. // Start searching without entering a query.
ThreadUtils.runOnUiThreadBlocking(manager::openSearchUI); ThreadUtils.runOnUiThreadBlocking(manager::openSearchUI);
Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState()); Assert.assertEquals("Wrong state, should be searching", BookmarkUIState.STATE_SEARCHING,
manager.getCurrentState());
// Select the folder and delete it. // Select the folder and delete it.
ThreadUtils.runOnUiThreadBlocking( ThreadUtils.runOnUiThreadBlocking(
...@@ -348,12 +348,14 @@ public class BookmarkTest { ...@@ -348,12 +348,14 @@ public class BookmarkTest {
R.id.selection_mode_delete_menu_id))); R.id.selection_mode_delete_menu_id)));
// Search should be exited and the folder should be gone. // Search should be exited and the folder should be gone.
Assert.assertEquals(BookmarkUIState.STATE_FOLDER, manager.getCurrentState()); Assert.assertEquals("Wrong state, should be in folder", BookmarkUIState.STATE_FOLDER,
manager.getCurrentState());
assertBookmarkItems("Wrong number of items before starting search.", 2, adapter, manager); assertBookmarkItems("Wrong number of items before starting search.", 2, adapter, manager);
// Start searching, enter a query. // Start searching, enter a query.
ThreadUtils.runOnUiThreadBlocking(manager::openSearchUI); ThreadUtils.runOnUiThreadBlocking(manager::openSearchUI);
Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState()); Assert.assertEquals("Wrong state, should be searching", BookmarkUIState.STATE_SEARCHING,
manager.getCurrentState());
searchBookmarks("Google"); searchBookmarks("Google");
assertBookmarkItems( assertBookmarkItems(
"Wrong number of items after searching.", 1, mItemsContainer.getAdapter(), manager); "Wrong number of items after searching.", 1, mItemsContainer.getAdapter(), manager);
...@@ -362,7 +364,8 @@ public class BookmarkTest { ...@@ -362,7 +364,8 @@ public class BookmarkTest {
removeBookmark(testBookmark); removeBookmark(testBookmark);
// The user should still be searching, and the bookmark should be gone. // The user should still be searching, and the bookmark should be gone.
Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState()); Assert.assertEquals("Wrong state, should be searching", BookmarkUIState.STATE_SEARCHING,
manager.getCurrentState());
assertBookmarkItems( assertBookmarkItems(
"Wrong number of items after searching.", 0, mItemsContainer.getAdapter(), manager); "Wrong number of items after searching.", 0, mItemsContainer.getAdapter(), manager);
...@@ -370,7 +373,8 @@ public class BookmarkTest { ...@@ -370,7 +373,8 @@ public class BookmarkTest {
ThreadUtils.runOnUiThreadBlocking(() -> manager.getUndoControllerForTests().onAction(null)); ThreadUtils.runOnUiThreadBlocking(() -> manager.getUndoControllerForTests().onAction(null));
// The user should still be searching, and the bookmark should reappear. // The user should still be searching, and the bookmark should reappear.
Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState()); Assert.assertEquals("Wrong state, should be searching", BookmarkUIState.STATE_SEARCHING,
manager.getCurrentState());
assertBookmarkItems( assertBookmarkItems(
"Wrong number of items after searching.", 1, mItemsContainer.getAdapter(), manager); "Wrong number of items after searching.", 1, mItemsContainer.getAdapter(), manager);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment