Ensure bookmark re-order items are not visible on partner bookmarks

Previously we relied solely on a check for the item position to
determine if "Move up" and "Move down" items should be shown in any
given bookmark item menu. This CL updates the check to also makes sure
the bookmark is moveable.

It looks like we had this check at one point and it got lost in a bad
rebase.

BUG=1019178

Change-Id: I595ba7646de23f79bd13e85d0bb2bb1e5d17f413
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1887732
Commit-Queue: Theresa  <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710549}

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

@@ -197,6 +197,7 @@ public class BookmarkBridge {
         private final BookmarkId mParentId;
         private final boolean mIsEditable;
         private final boolean mIsManaged;
+        private boolean mForceEditableForTesting;
 
         private BookmarkItem(BookmarkId id, String title, String url, boolean isFolder,
                 BookmarkId parentId, boolean isEditable, boolean isManaged) {
@@ -236,7 +237,7 @@ public class BookmarkBridge {
 
         /** @return Whether this bookmark can be edited. */
         public boolean isEditable() {
-            return mIsEditable;
+            return mForceEditableForTesting || mIsEditable;
         }
 
         /**@return Whether this bookmark's URL can be edited */
@@ -257,6 +258,11 @@ public class BookmarkBridge {
         public BookmarkId getId() {
             return mId;
         }
+
+        // TODO(https://crbug.com/1019217): Remove when BookmarkModel is stubbed in tests instead.
+        void forceEditableForTesting() {
+            mForceEditableForTesting = true;
+        }
     }
 
     /**
@@ -682,7 +688,7 @@ public class BookmarkBridge {
      * 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 folder or a managed bookmark 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
@@ -702,7 +708,7 @@ public class BookmarkBridge {
      * 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 folder or a managed bookmark 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. If empty, the URL will be used as the title.
@@ -778,6 +784,13 @@ public class BookmarkBridge {
                 mNativeBookmarkBridge, BookmarkBridge.this, parent, newOrder);
     }
 
+    @VisibleForTesting
+    BookmarkId getPartnerFolderId() {
+        assert mIsNativeBookmarkModelLoaded;
+        return BookmarkBridgeJni.get().getPartnerFolderId(
+                mNativeBookmarkBridge, BookmarkBridge.this);
+    }
+
     @CalledByNative
     private void bookmarkModelLoaded() {
         mIsNativeBookmarkModelLoaded = true;
@@ -965,6 +978,7 @@ public class BookmarkBridge {
         BookmarkId getMobileFolderId(long nativeBookmarkBridge, BookmarkBridge caller);
         BookmarkId getOtherFolderId(long nativeBookmarkBridge, BookmarkBridge caller);
         BookmarkId getDesktopFolderId(long nativeBookmarkBridge, BookmarkBridge caller);
+        BookmarkId getPartnerFolderId(long nativeBookmarkBridge, BookmarkBridge caller);
         int getChildCount(long nativeBookmarkBridge, BookmarkBridge caller, long id, int type);
         void getChildIDs(long nativeBookmarkBridge, BookmarkBridge caller, long id, int type,
                 boolean getFolders, boolean getBookmarks, List<BookmarkId> bookmarksList);

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

@@ -169,7 +169,7 @@ abstract class BookmarkRow extends SelectableItemView<BookmarkId>
                     menuItems.add(new Item(getContext(), R.string.bookmark_show_in_folder, true));
                 }
             } else if (mDelegate.getCurrentState() == BookmarkUIState.STATE_FOLDER
-                    && mLocation != Location.SOLO) {
+                    && mLocation != Location.SOLO && canMove) {
                 // Only add move up / move down buttons if there is more than 1 item
                 if (mLocation != Location.TOP) {
                     menuItems.add(new Item(getContext(), R.string.menu_item_move_up, true));

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

@@ -39,6 +39,7 @@ import org.chromium.chrome.test.ChromeJUnit4RunnerDelegate;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.RecyclerViewTestUtils;
 import org.chromium.components.bookmarks.BookmarkId;
+import org.chromium.components.bookmarks.BookmarkType;
 import org.chromium.components.sync.AndroidSyncSettings;
 import org.chromium.components.sync.test.util.MockSyncContentResolverDelegate;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
@@ -588,6 +589,64 @@ public class BookmarkReorderTest extends BookmarkTest {
         onView(withText("Move down")).check(doesNotExist());
     }
 
+    @Test
+    @MediumTest
+    public void testMoveButtonsGoneForPartnerBookmarks() throws Exception {
+        loadFakePartnerBookmarkShimForTesting();
+        BookmarkPromoHeader.forcePromoStateForTests(PromoState.PROMO_NONE);
+        openBookmarkManager();
+
+        // Open partner bookmarks folder.
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> mManager.openFolder(mBookmarkModel.getPartnerFolderId()));
+        RecyclerViewTestUtils.waitForStableRecyclerView(mItemsContainer);
+
+        Assert.assertEquals("Wrong number of items in partner bookmark folder.", 2,
+                getAdapter().getItemCount());
+
+        // Verify that bookmark 1 is editable (so more button can be triggered) but not movable.
+        BookmarkId partnerBookmarkId1 = getReorderAdapter().getIdByPosition(0);
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            BookmarkBridge.BookmarkItem partnerBookmarkItem1 =
+                    mBookmarkModel.getBookmarkById(partnerBookmarkId1);
+            partnerBookmarkItem1.forceEditableForTesting();
+            Assert.assertEquals("Incorrect bookmark type for item 1", BookmarkType.PARTNER,
+                    partnerBookmarkId1.getType());
+            Assert.assertFalse(
+                    "Partner item 1 should not be movable", partnerBookmarkItem1.isMovable());
+            Assert.assertTrue(
+                    "Partner item 1 should be editable", partnerBookmarkItem1.isEditable());
+        });
+
+        // Verify that bookmark 2 is editable (so more button can be triggered) but not movable.
+        View partnerBookmarkView1 = mItemsContainer.findViewHolderForAdapterPosition(0).itemView;
+        View more1 = partnerBookmarkView1.findViewById(R.id.more);
+        TestThreadUtils.runOnUiThreadBlocking(more1::callOnClick);
+        onView(withText("Move up")).check(doesNotExist());
+        onView(withText("Move down")).check(doesNotExist());
+
+        // Verify that bookmark 2 is not movable.
+        BookmarkId partnerBookmarkId2 = getReorderAdapter().getIdByPosition(1);
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            BookmarkBridge.BookmarkItem partnerBookmarkItem2 =
+                    mBookmarkModel.getBookmarkById(partnerBookmarkId2);
+            partnerBookmarkItem2.forceEditableForTesting();
+            Assert.assertEquals("Incorrect bookmark type for item 2", BookmarkType.PARTNER,
+                    partnerBookmarkId2.getType());
+            Assert.assertFalse(
+                    "Partner item 2 should not be movable", partnerBookmarkItem2.isMovable());
+            Assert.assertTrue(
+                    "Partner item 2 should be editable", partnerBookmarkItem2.isEditable());
+        });
+
+        // Verify that bookmark 2 does not have move up/down items.
+        View partnerBookmarkView2 = mItemsContainer.findViewHolderForAdapterPosition(1).itemView;
+        View more2 = partnerBookmarkView2.findViewById(R.id.more);
+        TestThreadUtils.runOnUiThreadBlocking(more2::callOnClick);
+        onView(withText("Move up")).check(doesNotExist());
+        onView(withText("Move down")).check(doesNotExist());
+    }
+
     @Test
     @MediumTest
     public void testTopLevelFolderUpdateAfterSync() throws Exception {

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

@@ -141,6 +141,16 @@ public class BookmarkTest {
         BookmarkTestUtil.waitForBookmarkModelLoaded();
     }
 
+    /**
+     * Loads a non-empty partner bookmarks folder for testing. The partner bookmarks folder will
+     * appear in the mobile bookmarks folder.
+     */
+    protected void loadFakePartnerBookmarkShimForTesting() {
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> { mBookmarkModel.loadFakePartnerBookmarkShimForTesting(); });
+        BookmarkTestUtil.waitForBookmarkModelLoaded();
+    }
+
     @After
     public void tearDown() {
         mTestServer.stopAndDestroyServer();

chrome/browser/android/bookmarks/bookmark_bridge.cc

@@ -415,6 +415,18 @@ ScopedJavaLocalRef<jobject> BookmarkBridge::GetDesktopFolderId(
   return folder_id_obj;
 }
 
+ScopedJavaLocalRef<jobject> BookmarkBridge::GetPartnerFolderId(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj) {
+  DCHECK(partner_bookmarks_shim_->IsLoaded());
+
+  const BookmarkNode* partner_node =
+      partner_bookmarks_shim_->GetPartnerBookmarksRoot();
+  ScopedJavaLocalRef<jobject> folder_id_obj = JavaBookmarkIdCreateBookmarkId(
+      env, partner_node->id(), GetBookmarkType(partner_node));
+  return folder_id_obj;
+}
+
 jint BookmarkBridge::GetChildCount(JNIEnv* env,
                                     const JavaParamRef<jobject>& obj,
                                     jlong id,

chrome/browser/android/bookmarks/bookmark_bridge.h

@@ -96,6 +96,10 @@ class BookmarkBridge : public bookmarks::BaseBookmarkModelObserver,
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj);
 
+  base::android::ScopedJavaLocalRef<jobject> GetPartnerFolderId(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& obj);
+
   void GetChildIDs(JNIEnv* env,
                    const base::android::JavaParamRef<jobject>& obj,
                    jlong id,