[Home] Exit search if bookmark deleted while search query empty

If the user initiates a search, then deletes a folder without entering
a query, we were previously crashing. Folders are not typically shown
in search view except when the query is empty. Prevent the crash by
exiting search if a bookmark is removed while the search query is empty.

BUG=789910

Change-Id: If4c3586deb72ce5cd61c87a903ba594d1a706351
Reviewed-on: https://chromium-review.googlesource.com/804419
Commit-Queue: Theresa <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521157}

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemsAdapter.java

@@ -73,6 +73,12 @@ class BookmarkItemsAdapter
         public void bookmarkNodeRemoved(BookmarkItem parent, int oldIndex, BookmarkItem node,
                 boolean isDoingExtensiveChanges) {
             assert mDelegate != null;
+
+            if (mDelegate.getCurrentState() == BookmarkUIState.STATE_SEARCHING
+                    && TextUtils.equals(mSearchText, EMPTY_QUERY)) {
+                mDelegate.closeSearchUI();
+            }
+
             if (node.isFolder()) {
                 mDelegate.notifyStateChange(BookmarkItemsAdapter.this);
             } else {

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManager.java

@@ -410,15 +410,7 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate,
 
     @Override
     public void closeSearchUI() {
-        mSelectableListLayout.onEndSearch();
-
-        // Pop the search state off the stack.
-        mStateStack.pop();
-
-        // Set the state back to the folder that was previously being viewed. Listeners, including
-        // the BookmarkItemsAdapter, will be notified of the change and the list of bookmarks will
-        // be updated.
-        setState(mStateStack.pop());
+        mToolbar.hideSearchView();
     }
 
     @Override
@@ -456,7 +448,15 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate,
 
     @Override
     public void onEndSearch() {
-        closeSearchUI();
+        mSelectableListLayout.onEndSearch();
+
+        // Pop the search state off the stack.
+        mStateStack.pop();
+
+        // Set the state back to the folder that was previously being viewed. Listeners, including
+        // the BookmarkItemsAdapter, will be notified of the change and the list of bookmarks will
+        // be updated.
+        setState(mStateStack.pop());
     }
 
     // Testing methods
@@ -466,6 +466,11 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate,
         return mToolbar;
     }
 
+    @VisibleForTesting
+    public BookmarkUndoController getUndoControllerForTests() {
+        return mUndoController;
+    }
+
     /**
      * @param preventLoading Whether to prevent the bookmark model from fully loading for testing.
      */

chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java

@@ -317,6 +317,62 @@ public class BookmarkTest {
         Assert.assertEquals(BookmarkUIState.STATE_FOLDER, delegate.getCurrentState());
     }
 
+    @Test
+    @MediumTest
+    public void testSearchBookmarks_Delete() throws Exception {
+        BookmarkPromoHeader.forcePromoStateForTests(BookmarkPromoHeader.PromoState.PROMO_NONE);
+        BookmarkId testFolder = addFolder(TEST_FOLDER_TITLE);
+        BookmarkId testBookmark = addBookmark(TEST_PAGE_TITLE_GOOGLE, mTestPage);
+        addBookmark(TEST_PAGE_TITLE_FOO, mTestPageFoo);
+        openBookmarkManager();
+
+        BookmarkItemsAdapter adapter = ((BookmarkItemsAdapter) mItemsContainer.getAdapter());
+        BookmarkManager manager = (BookmarkManager) adapter.getDelegateForTesting();
+
+        Assert.assertEquals(BookmarkUIState.STATE_FOLDER, manager.getCurrentState());
+        assertBookmarkItems("Wrong number of items before starting search.", 3, adapter, manager);
+
+        // Start searching without entering a query.
+        ThreadUtils.runOnUiThreadBlocking(manager::openSearchUI);
+        Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState());
+
+        // Select the folder and delete it.
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> manager.getSelectionDelegate().toggleSelectionForItem(adapter.getItem(0)));
+        ThreadUtils.runOnUiThreadBlocking(
+                ()
+                        -> manager.getToolbarForTests().onMenuItemClick(
+                                manager.getToolbarForTests().getMenu().findItem(
+                                        R.id.selection_mode_delete_menu_id)));
+
+        // Search should be exited and the folder should be gone.
+        Assert.assertEquals(BookmarkUIState.STATE_FOLDER, manager.getCurrentState());
+        assertBookmarkItems("Wrong number of items before starting search.", 2, adapter, manager);
+
+        // Start searching, enter a query.
+        ThreadUtils.runOnUiThreadBlocking(manager::openSearchUI);
+        Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState());
+        searchBookmarks("Google");
+        assertBookmarkItems(
+                "Wrong number of items after searching.", 1, mItemsContainer.getAdapter(), manager);
+
+        // Remove the bookmark.
+        removeBookmark(testBookmark);
+
+        // The user should still be searching, and the bookmark should be gone.
+        Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState());
+        assertBookmarkItems(
+                "Wrong number of items after searching.", 0, mItemsContainer.getAdapter(), manager);
+
+        // Undo the deletion.
+        ThreadUtils.runOnUiThreadBlocking(() -> manager.getUndoControllerForTests().onAction(null));
+
+        // The user should still be searching, and the bookmark should reappear.
+        Assert.assertEquals(BookmarkUIState.STATE_SEARCHING, manager.getCurrentState());
+        assertBookmarkItems(
+                "Wrong number of items after searching.", 1, mItemsContainer.getAdapter(), manager);
+    }
+
     @Test
     @MediumTest
     @Feature({"RenderTest"})