Read later : Several fixes on the bookmarks UI

This CL fixes :
1 - Fixed layout styling for section header
2 - Added plumbing to mark an article as read
3 - Added fix to show the sign-in promo at the top of reading list
4 - Fixed search state to not show any read/unread section headers

Bug: 1148472, 1148524
Change-Id: I6269b28bf7146acc684437446686c76c88006892
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2535616Reviewed-by: Theresa  <twellington@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827135}

chrome/android/java/res/layout/bookmark_section_header.xml

@@ -7,17 +7,21 @@
     xmlns:android="http://schemas.android.com/apk/res/android"
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
-    android:orientation="vertical">
+    android:orientation="vertical"
+    android:paddingStart="@dimen/list_item_default_margin">
 
     <TextView
         android:id="@+id/title"
         android:layout_width="match_parent"
-        android:layout_height="wrap_content" />
+        android:layout_height="wrap_content"
+        android:textAppearance="@style/TextAppearance.TextMedium.Secondary" />
 
     <TextView
         android:id="@+id/description"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-        style="@style/TextAppearance.TextMedium.Secondary" />
+        android:maxLines="1"
+        android:ellipsize="end"
+        android:textAppearance="@style/TextAppearance.TextSmall.Secondary" />
 
 </LinearLayout>
\ No newline at end of file

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

@@ -848,6 +848,16 @@ public class BookmarkBridge {
                 mNativeBookmarkBridge, BookmarkBridge.this, title, url);
     }
 
+    /**
+     * Helper method to mark an article as read.
+     * @param url The URL of the reading list item.
+     * @param read Whether the article should be marked as read.
+     */
+    public void setReadStatusForReadingList(String url, boolean read) {
+        BookmarkBridgeJni.get().setReadStatus(
+                mNativeBookmarkBridge, BookmarkBridge.this, url, read);
+    }
+
     @VisibleForTesting
     BookmarkId getPartnerFolderId() {
         ThreadUtils.assertOnUiThread();
@@ -1073,6 +1083,8 @@ public class BookmarkBridge {
                 int index, String title, String url);
         BookmarkId addToReadingList(
                 long nativeBookmarkBridge, BookmarkBridge caller, String title, String url);
+        void setReadStatus(
+                long nativeBookmarkBridge, BookmarkBridge caller, String url, boolean read);
         void undo(long nativeBookmarkBridge, BookmarkBridge caller);
         void startGroupingUndos(long nativeBookmarkBridge, BookmarkBridge caller);
         void endGroupingUndos(long nativeBookmarkBridge, BookmarkBridge caller);

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

@@ -303,10 +303,6 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter<BookmarkListEntry>
 
         mSearchText = EMPTY_QUERY;
         mCurrentFolder = folder;
-
-        if (!(folder.equals(mCurrentFolder))) {
-            mCurrentFolder = folder;
-        }
         enableDrag();
 
         if (topLevelFoldersShowing()) {
@@ -323,6 +319,7 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter<BookmarkListEntry>
         // Headers should not appear in Search mode
         // Don't need to notify because we need to redraw everything in the next step
         updateHeader(false);
+        removeSectionHeaders();
         notifyDataSetChanged();
     }
 
@@ -441,6 +438,15 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter<BookmarkListEntry>
         }
     }
 
+    /** Removes all section headers from the current list. */
+    private void removeSectionHeaders() {
+        for (int i = mElements.size() - 1; i >= 0; i--) {
+            if (mElements.get(i).getViewType() == ViewType.SECTION_HEADER) {
+                mElements.remove(i);
+            }
+        }
+    }
+
     private void populateTopLevelFoldersList() {
         BookmarkId desktopNodeId = mDelegate.getModel().getDesktopFolderId();
         BookmarkId mobileNodeId = mDelegate.getModel().getMobileFolderId();

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

@@ -281,8 +281,11 @@ public class BookmarkUtils {
         RecordHistogram.recordEnumeratedHistogram(
                 "Bookmarks.OpenBookmarkType", bookmarkId.getType(), BookmarkType.LAST + 1);
 
-        String url = model.getBookmarkById(bookmarkId).getUrl();
-        openUrl(context, url, openBookmarkComponentName);
+        BookmarkItem bookmarkItem = model.getBookmarkById(bookmarkId);
+        if (bookmarkItem.getId().getType() == BookmarkType.READING_LIST) {
+            model.setReadStatusForReadingList(bookmarkItem.getUrl(), true);
+        }
+        openUrl(context, bookmarkItem.getUrl(), openBookmarkComponentName);
 
         return true;
     }

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

@@ -31,16 +31,28 @@ class ReadingListSectionHeader {
     public static void maybeSortAndInsertSectionHeaders(
             List<BookmarkListEntry> listItems, Context context) {
         if (listItems.isEmpty()) return;
-        sort(listItems);
+
+        // The topmost item(s) could be promo headers.
+        int readingListStartIndex = 0;
+        for (BookmarkListEntry listItem : listItems) {
+            boolean isReadingListItem = listItem.getBookmarkItem() != null
+                    && listItem.getBookmarkItem().getId().getType() == BookmarkType.READING_LIST;
+            if (isReadingListItem) break;
+            readingListStartIndex++;
+        }
+
+        assert readingListStartIndex < listItems.size();
+        sort(listItems, readingListStartIndex);
 
         // Add a section header at the top.
-        assert listItems.get(0).getBookmarkItem().getId().getType() == BookmarkType.READING_LIST;
-        boolean isRead = listItems.get(0).getBookmarkItem().isRead();
-        listItems.add(0, createReadingListSectionHeader(isRead, context));
+        assert listItems.get(readingListStartIndex).getBookmarkItem().getId().getType()
+                == BookmarkType.READING_LIST;
+        boolean isRead = listItems.get(readingListStartIndex).getBookmarkItem().isRead();
+        listItems.add(readingListStartIndex, createReadingListSectionHeader(isRead, context));
         if (isRead) return;
 
         // Search for the first read element, and insert the read section header.
-        for (int i = 2; i < listItems.size(); i++) {
+        for (int i = readingListStartIndex + 2; i < listItems.size(); i++) {
             BookmarkListEntry listItem = listItems.get(i);
             assert listItem.getBookmarkItem().getId().getType() == BookmarkType.READING_LIST;
             if (listItem.getBookmarkItem().isRead()) {
@@ -53,9 +65,9 @@ class ReadingListSectionHeader {
     /**
      * Sorts the given {@code listItems} to show unread items ahead of read items.
      */
-    private static void sort(List<BookmarkListEntry> listItems) {
+    private static void sort(List<BookmarkListEntry> listItems, int readingListStartIndex) {
         // TODO(crbug.com/1147259): Sort items by creation time possibly.
-        Collections.sort(listItems, (lhs, rhs) -> {
+        Collections.sort(listItems.subList(readingListStartIndex, listItems.size()), (lhs, rhs) -> {
             // Unread items are shown first.
             boolean lhsRead = lhs.getBookmarkItem().isRead();
             boolean rhsRead = rhs.getBookmarkItem().isRead();
@@ -67,6 +79,6 @@ class ReadingListSectionHeader {
     private static BookmarkListEntry createReadingListSectionHeader(boolean read, Context context) {
         return BookmarkListEntry.createSectionHeader(
                 read ? R.string.reading_list_read : R.string.reading_list_unread,
-                read ? R.string.reading_list_ready_for_offline : null, context);
+                read ? null : R.string.reading_list_ready_for_offline, context);
     }
 }

chrome/browser/android/bookmarks/bookmark_bridge.cc

@@ -834,6 +834,17 @@ ScopedJavaLocalRef<jobject> BookmarkBridge::AddToReadingList(
   return j_bookark_id;
 }
 
+void BookmarkBridge::SetReadStatus(JNIEnv* env,
+                                   const JavaParamRef<jobject>& obj,
+                                   const JavaParamRef<jstring>& j_url,
+                                   jboolean j_read) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(IsLoaded());
+
+  reading_list_manager_->SetReadStatus(
+      GURL(base::android::ConvertJavaStringToUTF16(env, j_url)), j_read);
+}
+
 void BookmarkBridge::Undo(JNIEnv* env, const JavaParamRef<jobject>& obj) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(IsLoaded());

chrome/browser/android/bookmarks/bookmark_bridge.h

@@ -219,6 +219,11 @@ class BookmarkBridge : public bookmarks::BaseBookmarkModelObserver,
       const base::android::JavaParamRef<jstring>& j_title,
       const base::android::JavaParamRef<jstring>& j_url);
 
+  void SetReadStatus(JNIEnv* env,
+                     const base::android::JavaParamRef<jobject>& obj,
+                     const base::android::JavaParamRef<jstring>& j_url,
+                     jboolean j_read);
+
   void Undo(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
 
   void StartGroupingUndos(JNIEnv* env,