Read later: Load the reading list folder in UI.

This CL starts to implement the glue code in bookmark_bridge.cc, it
loads the reading list folder in bookmark root page.

Bug: 1128074
Change-Id: Ib3259e8c799242f033cd5778876df9211c3f6cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441319
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814095}

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

@@ -7,6 +7,8 @@ package org.chromium.chrome.browser.bookmarks;
 import static androidx.test.espresso.Espresso.onView;
 import static androidx.test.espresso.action.ViewActions.click;
 import static androidx.test.espresso.assertion.ViewAssertions.doesNotExist;
+import static androidx.test.espresso.assertion.ViewAssertions.matches;
+import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
 
 import static org.mockito.Mockito.any;
@@ -59,6 +61,7 @@ import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.bookmarks.BookmarkBridge.BookmarkItem;
 import org.chromium.chrome.browser.bookmarks.BookmarkBridge.BookmarkModelObserver;
 import org.chromium.chrome.browser.bookmarks.BookmarkPromoHeader.PromoState;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
 import org.chromium.chrome.browser.night_mode.ChromeNightModeTestUtils;
 import org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksShim;
@@ -77,6 +80,7 @@ import org.chromium.chrome.test.util.ApplicationTestUtils;
 import org.chromium.chrome.test.util.BookmarkTestUtil;
 import org.chromium.chrome.test.util.ChromeRenderTestRule;
 import org.chromium.chrome.test.util.MenuUtils;
+import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.components.bookmarks.BookmarkId;
 import org.chromium.components.bookmarks.BookmarkType;
 import org.chromium.components.browser_ui.widget.RecyclerViewTestUtils;
@@ -1471,6 +1475,19 @@ public class BookmarkTest {
         });
     }
 
+    @Test
+    @SmallTest
+    @Features.EnableFeatures({ChromeFeatureList.READ_LATER})
+    public void testReadingListFolderShown() throws Exception {
+        BookmarkPromoHeader.forcePromoStateForTests(PromoState.PROMO_NONE);
+        openBookmarkManager();
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> mManager.openFolder(mBookmarkModel.getRootFolderId()));
+
+        // Reading list should show in the root folder.
+        onView(withText("Reading list")).check(matches(isDisplayed()));
+    }
+
     /**
      * Adds a bookmark in the scenario where we have partner bookmarks.
      *

chrome/browser/android/bookmarks/bookmark_bridge.cc

@@ -24,6 +24,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/android/chrome_jni_headers/BookmarkBridge_jni.h"
 #include "chrome/browser/android/bookmarks/partner_bookmarks_reader.h"
+#include "chrome/browser/android/reading_list/reading_list_manager_factory.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
@@ -84,7 +85,7 @@ class BookmarkTitleComparer {
     }
   }
 
-private:
+ private:
   BookmarkBridge* bookmark_bridge_;  // weak
   const icu::Collator* collator_;
 };
@@ -122,6 +123,10 @@ BookmarkBridge::BookmarkBridge(JNIEnv* env,
       chrome::GetBrowserContextRedirectedInIncognito(profile_));
   partner_bookmarks_shim_->AddObserver(this);
 
+  reading_list_manager_ =
+      ReadingListManagerFactory::GetForBrowserContext(profile_);
+  reading_list_manager_->AddObserver(this);
+
   pref_change_registrar_.Init(profile_->GetPrefs());
   pref_change_registrar_.Add(
       bookmarks::prefs::kEditBookmarksEnabled,
@@ -141,6 +146,7 @@ BookmarkBridge::~BookmarkBridge() {
   bookmark_model_->RemoveObserver(this);
   if (partner_bookmarks_shim_)
     partner_bookmarks_shim_->RemoveObserver(this);
+  reading_list_manager_->RemoveObserver(this);
 }
 
 void BookmarkBridge::Destroy(JNIEnv*, const JavaParamRef<jobject>&) {
@@ -150,8 +156,8 @@ void BookmarkBridge::Destroy(JNIEnv*, const JavaParamRef<jobject>&) {
 static jlong JNI_BookmarkBridge_Init(JNIEnv* env,
                                      const JavaParamRef<jobject>& obj,
                                      const JavaParamRef<jobject>& j_profile) {
-  BookmarkBridge* delegate = new BookmarkBridge(env, obj, j_profile);
-  return reinterpret_cast<intptr_t>(delegate);
+  BookmarkBridge* bridge = new BookmarkBridge(env, obj, j_profile);
+  return reinterpret_cast<intptr_t>(bridge);
 }
 
 jlong BookmarkBridge::GetBookmarkIdForWebContents(
@@ -283,6 +289,9 @@ void BookmarkBridge::GetTopLevelFolderIDs(
       top_level_folders.push_back(
           partner_bookmarks_shim_->GetPartnerBookmarksRoot());
     }
+    if (reading_list_manager_->GetRoot()) {
+      top_level_folders.push_back(reading_list_manager_->GetRoot());
+    }
   }
   std::size_t special_count = top_level_folders.size();
 
@@ -809,7 +818,7 @@ void BookmarkBridge::StartGroupingUndos(JNIEnv* env,
                                          const JavaParamRef<jobject>& obj) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(IsLoaded());
-  DCHECK(!grouped_bookmark_actions_.get()); // shouldn't have started already
+  DCHECK(!grouped_bookmark_actions_.get());  // shouldn't have started already
   grouped_bookmark_actions_.reset(
       new bookmarks::ScopedGroupBookmarkActions(bookmark_model_));
 }
@@ -818,7 +827,7 @@ void BookmarkBridge::EndGroupingUndos(JNIEnv* env,
                                        const JavaParamRef<jobject>& obj) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(IsLoaded());
-  DCHECK(grouped_bookmark_actions_.get()); // should only call after start
+  DCHECK(grouped_bookmark_actions_.get());  // should only call after start
   grouped_bookmark_actions_.reset();
 }
 
@@ -864,6 +873,8 @@ const BookmarkNode* BookmarkBridge::GetNodeByID(long node_id, int type) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (type == BookmarkType::BOOKMARK_TYPE_PARTNER) {
     node = partner_bookmarks_shim_->GetNodeByID(static_cast<int64_t>(node_id));
+  } else if (type == BookmarkType::BOOKMARK_TYPE_READING_LIST) {
+    node = reading_list_manager_->GetNodeByID(static_cast<int64_t>(node_id));
   } else {
     node = bookmarks::GetBookmarkNodeByID(bookmark_model_,
                                           static_cast<int64_t>(node_id));
@@ -918,18 +929,23 @@ bool BookmarkBridge::IsManaged(const BookmarkNode* node) const {
 
 const BookmarkNode* BookmarkBridge::GetParentNode(const BookmarkNode* node) {
   DCHECK(IsLoaded());
-  if (node == partner_bookmarks_shim_->GetPartnerBookmarksRoot()) {
+  if (node == partner_bookmarks_shim_->GetPartnerBookmarksRoot())
     return bookmark_model_->mobile_node();
-  } else {
-    return node->parent();
-  }
+
+  if (node == reading_list_manager_->GetRoot())
+    return bookmark_model_->root_node();
+
+  return node->parent();
 }
 
 int BookmarkBridge::GetBookmarkType(const BookmarkNode* node) {
   if (partner_bookmarks_shim_->IsPartnerBookmark(node))
     return BookmarkType::BOOKMARK_TYPE_PARTNER;
-  else
-    return BookmarkType::BOOKMARK_TYPE_NORMAL;
+
+  if (reading_list_manager_->IsReadingListBookmark(node))
+    return BookmarkType::BOOKMARK_TYPE_READING_LIST;
+
+  return BookmarkType::BOOKMARK_TYPE_NORMAL;
 }
 
 bool BookmarkBridge::IsReachable(const BookmarkNode* node) const {
@@ -939,7 +955,8 @@ bool BookmarkBridge::IsReachable(const BookmarkNode* node) const {
 }
 
 bool BookmarkBridge::IsLoaded() const {
-  return (bookmark_model_->loaded() && partner_bookmarks_shim_->IsLoaded());
+  return (bookmark_model_->loaded() && partner_bookmarks_shim_->IsLoaded() &&
+          reading_list_manager_->IsLoaded());
 }
 
 bool BookmarkBridge::IsFolderAvailable(
@@ -1121,6 +1138,10 @@ void BookmarkBridge::ShimBeingDeleted(PartnerBookmarksShim* shim) {
   partner_bookmarks_shim_ = nullptr;
 }
 
+void BookmarkBridge::ReadingListLoaded() {
+  NotifyIfDoneLoading();
+}
+
 void BookmarkBridge::ReorderChildren(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,

chrome/browser/android/bookmarks/bookmark_bridge.h

@@ -16,6 +16,7 @@
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/android/bookmarks/partner_bookmarks_shim.h"
+#include "chrome/browser/reading_list/android/reading_list_manager.h"
 #include "components/bookmarks/browser/base_bookmark_model_observer.h"
 #include "components/bookmarks/common/android/bookmark_id.h"
 #include "components/prefs/pref_change_registrar.h"
@@ -32,7 +33,8 @@ class Profile;
 // bookmark page. This fetches the bookmarks, title, urls, folder
 // hierarchy.
 class BookmarkBridge : public bookmarks::BaseBookmarkModelObserver,
-                        public PartnerBookmarksShim::Observer {
+                       public PartnerBookmarksShim::Observer,
+                       public ReadingListManager::Observer {
  public:
   BookmarkBridge(JNIEnv* env,
                  const base::android::JavaRef<jobject>& obj,
@@ -279,6 +281,9 @@ class BookmarkBridge : public bookmarks::BaseBookmarkModelObserver,
   void PartnerShimLoaded(PartnerBookmarksShim* shim) override;
   void ShimBeingDeleted(PartnerBookmarksShim* shim) override;
 
+  // Override ReadingListManager::Observer
+  void ReadingListLoaded() override;
+
   Profile* profile_;
   JavaObjectWeakGlobalRef weak_java_ref_;
   bookmarks::BookmarkModel* bookmark_model_;  // weak
@@ -291,6 +296,9 @@ class BookmarkBridge : public bookmarks::BaseBookmarkModelObserver,
   // This is owned by profile.
   PartnerBookmarksShim* partner_bookmarks_shim_;
 
+  // Holds reading list data. A keyed service owned by the profile.
+  ReadingListManager* reading_list_manager_;
+
   DISALLOW_COPY_AND_ASSIGN(BookmarkBridge);
 };
 

chrome/browser/android/reading_list/reading_list_manager_factory.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/reading_list/android/empty_reading_list_manager.h"
 #include "chrome/browser/reading_list/android/reading_list_manager_impl.h"
 #include "chrome/browser/ui/read_later/reading_list_model_factory.h"
@@ -42,3 +43,8 @@ KeyedService* ReadingListManagerFactory::BuildServiceInstanceFor(
       ReadingListModelFactory::GetForBrowserContext(context);
   return new ReadingListManagerImpl(reading_list_model);
 }
+
+content::BrowserContext* ReadingListManagerFactory::GetBrowserContextToUse(
+    content::BrowserContext* context) const {
+  return chrome::GetBrowserContextRedirectedInIncognito(context);
+}

chrome/browser/android/reading_list/reading_list_manager_factory.h

@@ -27,8 +27,11 @@ class ReadingListManagerFactory : public BrowserContextKeyedServiceFactory {
   ReadingListManagerFactory& operator=(const ReadingListManagerFactory&) =
       delete;
 
+  // BrowserContextKeyedServiceFactory overrides.
   KeyedService* BuildServiceInstanceFor(
       content::BrowserContext* context) const override;
+  content::BrowserContext* GetBrowserContextToUse(
+      content::BrowserContext* context) const override;
 };
 
 #endif  // CHROME_BROWSER_ANDROID_READING_LIST_READING_LIST_MANAGER_FACTORY_H_

chrome/browser/reading_list/android/BUILD.gn

@@ -22,9 +22,11 @@ source_set("reading_list") {
   # This target should not depend on anything in //chrome/* except the proto library.
   deps = [
     "//base",
+    "//chrome/app:generated_resources",
     "//components/bookmarks/browser",
     "//components/keyed_service/core",
     "//components/reading_list/core",
+    "//ui/base:base",
     "//url",
   ]
 }

chrome/browser/reading_list/android/empty_reading_list_manager.cc

@@ -8,16 +8,31 @@ EmptyReadingListManager::EmptyReadingListManager() = default;
 
 EmptyReadingListManager::~EmptyReadingListManager() = default;
 
+void EmptyReadingListManager::AddObserver(Observer* observer) {}
+
+void EmptyReadingListManager::RemoveObserver(Observer* observer) {}
+
 const bookmarks::BookmarkNode* EmptyReadingListManager::Add(
     const GURL& url,
     const std::string& title) {
   return nullptr;
 }
 
-const bookmarks::BookmarkNode* EmptyReadingListManager::Get(const GURL& url) {
+const bookmarks::BookmarkNode* EmptyReadingListManager::Get(
+    const GURL& url) const {
+  return nullptr;
+}
+
+const bookmarks::BookmarkNode* EmptyReadingListManager::GetNodeByID(
+    int64_t id) const {
   return nullptr;
 }
 
+bool EmptyReadingListManager::IsReadingListBookmark(
+    const bookmarks::BookmarkNode* node) const {
+  return false;
+}
+
 void EmptyReadingListManager::Delete(const GURL& url) {}
 
 const bookmarks::BookmarkNode* EmptyReadingListManager::GetRoot() const {
@@ -33,3 +48,7 @@ size_t EmptyReadingListManager::unread_size() const {
 }
 
 void EmptyReadingListManager::SetReadStatus(const GURL& url, bool read) {}
+
+bool EmptyReadingListManager::IsLoaded() const {
+  return true;
+}

chrome/browser/reading_list/android/empty_reading_list_manager.h

@@ -16,14 +16,20 @@ class EmptyReadingListManager : public ReadingListManager {
 
  private:
   // ReadingListManager implementation.
+  void AddObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
   const bookmarks::BookmarkNode* Add(const GURL& url,
                                      const std::string& title) override;
-  const bookmarks::BookmarkNode* Get(const GURL& url) override;
+  const bookmarks::BookmarkNode* Get(const GURL& url) const override;
+  const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const override;
+  bool IsReadingListBookmark(
+      const bookmarks::BookmarkNode* node) const override;
   void Delete(const GURL& url) override;
   const bookmarks::BookmarkNode* GetRoot() const override;
   size_t size() const override;
   size_t unread_size() const override;
   void SetReadStatus(const GURL& url, bool read) override;
+  bool IsLoaded() const override;
 };
 
 #endif  // CHROME_BROWSER_READING_LIST_ANDROID_EMPTY_READING_LIST_MANAGER_H_

chrome/browser/reading_list/android/reading_list_manager.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/observer_list.h"
 #include "components/keyed_service/core/keyed_service.h"
 
 namespace bookmarks {
@@ -25,6 +26,20 @@ class ReadingListManager : public KeyedService {
   ReadingListManager(const ReadingListManager&) = delete;
   ReadingListManager& operator=(const ReadingListManager&) = delete;
 
+  // The observer that listens to reading list manager events.
+  class Observer : public base::CheckedObserver {
+   public:
+    // Called when the reading list backend is loaded.
+    virtual void ReadingListLoaded() {}
+
+    Observer() = default;
+    ~Observer() override = default;
+  };
+
+  // Adds/Removes observers.
+  virtual void AddObserver(Observer* observer) = 0;
+  virtual void RemoveObserver(Observer* observer) = 0;
+
   // Adds a reading list article to the unread section, and return the bookmark
   // node representation. The bookmark node is owned by this class. If there is
   // a duplicate URL, swaps the current reading list item. Returns nullptr on
@@ -35,7 +50,16 @@ class ReadingListManager : public KeyedService {
   // Gets the bookmark node representation of a reading list article. The
   // bookmark node is owned by this class. Returns nullptr if no such reading
   // list.
-  virtual const bookmarks::BookmarkNode* Get(const GURL& url) = 0;
+  virtual const bookmarks::BookmarkNode* Get(const GURL& url) const = 0;
+
+  // Gets the bookmark node for the given |id|. The returned node can be the
+  // root folder node. Returns nullptr if no match.
+  virtual const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const = 0;
+
+  // Returns whether the bookmark node is maintained in reading list manager.
+  // Will return true if |node| is the root for reading list nodes.
+  virtual bool IsReadingListBookmark(
+      const bookmarks::BookmarkNode* node) const = 0;
 
   // Deletes a reading list article.
   virtual void Delete(const GURL& url) = 0;
@@ -55,6 +79,9 @@ class ReadingListManager : public KeyedService {
   // Sets the read status for a reading list article. No op if such reading list
   // article doesn't exist.
   virtual void SetReadStatus(const GURL& url, bool read) = 0;
+
+  // Returns whether the reading list manager is loaded.
+  virtual bool IsLoaded() const = 0;
 };
 
 #endif  // CHROME_BROWSER_READING_LIST_ANDROID_READING_LIST_MANAGER_H_

chrome/browser/reading_list/android/reading_list_manager_impl.cc

@@ -9,8 +9,10 @@
 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/reading_list/core/reading_list_model.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
 using BookmarkNode = bookmarks::BookmarkNode;
@@ -44,10 +46,11 @@ bool SyncToBookmark(const ReadingListEntry& entry, BookmarkNode* bookmark) {
 
 ReadingListManagerImpl::ReadingListManagerImpl(
     ReadingListModel* reading_list_model)
-    : reading_list_model_(reading_list_model), maximum_id_(0L) {
+    : reading_list_model_(reading_list_model), maximum_id_(0L), loaded_(false) {
   DCHECK(reading_list_model_);
   root_ = std::make_unique<BookmarkNode>(maximum_id_++, base::GenerateGUID(),
                                          GURL());
+  root_->SetTitle(l10n_util::GetStringUTF16(IDS_READ_LATER_TITLE));
   DCHECK(root_->is_folder());
   reading_list_model_->AddObserver(this);
 }
@@ -62,6 +65,19 @@ void ReadingListManagerImpl::ReadingListModelLoaded(
   root_->DeleteAll();
   for (const auto& url : model->Keys())
     AddBookmark(model->GetEntryByURL(url));
+
+  loaded_ = true;
+
+  for (Observer& observer : observers_)
+    observer.ReadingListLoaded();
+}
+
+void ReadingListManagerImpl::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void ReadingListManagerImpl::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
 }
 
 const BookmarkNode* ReadingListManagerImpl::Add(const GURL& url,
@@ -74,11 +90,33 @@ const BookmarkNode* ReadingListManagerImpl::Add(const GURL& url,
   return AddBookmark(&new_entry);
 }
 
-const BookmarkNode* ReadingListManagerImpl::Get(const GURL& url) {
+const BookmarkNode* ReadingListManagerImpl::Get(const GURL& url) const {
   DCHECK(reading_list_model_->loaded());
   return FindBookmarkByURL(url);
 }
 
+const BookmarkNode* ReadingListManagerImpl::GetNodeByID(int64_t id) const {
+  if (root_->id() == id)
+    return root_.get();
+
+  for (const auto& child : root_->children()) {
+    if (child->id() == id)
+      return child.get();
+  }
+
+  return nullptr;
+}
+
+bool ReadingListManagerImpl::IsReadingListBookmark(
+    const BookmarkNode* node) const {
+  if (!node)
+    return false;
+  if (root_.get() == node)
+    return true;
+
+  return Get(node->url());
+}
+
 void ReadingListManagerImpl::Delete(const GURL& url) {
   DCHECK(reading_list_model_->loaded());
 
@@ -115,7 +153,14 @@ void ReadingListManagerImpl::SetReadStatus(const GURL& url, bool read) {
   }
 }
 
-BookmarkNode* ReadingListManagerImpl::FindBookmarkByURL(const GURL& url) {
+bool ReadingListManagerImpl::IsLoaded() const {
+  return loaded_;
+}
+
+BookmarkNode* ReadingListManagerImpl::FindBookmarkByURL(const GURL& url) const {
+  if (!url.is_valid())
+    return nullptr;
+
   for (const auto& child : root_->children()) {
     if (url == child->url())
       return child.get();

chrome/browser/reading_list/android/reading_list_manager_impl.h

@@ -17,7 +17,7 @@ class ReadingListModel;
 // 1. Holds a in memory bookmark node tree. Contains a folder root and reading
 // list nodes as children. Only has one level of children.
 // 2. Talk to reading list model, and sync with the in memory bookmark tree.
-// 3. TODO(xingliu): Add an observer and broadcast events to caller.
+// 3. Talk to observers to report model change events.
 class ReadingListManagerImpl : public ReadingListManager,
                                public ReadingListModelObserver {
  public:
@@ -29,19 +29,25 @@ class ReadingListManagerImpl : public ReadingListManager,
   void ReadingListModelLoaded(const ReadingListModel* model) override;
 
   // ReadingListManager implementation.
+  void AddObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
   const bookmarks::BookmarkNode* Add(const GURL& url,
                                      const std::string& title) override;
-  const bookmarks::BookmarkNode* Get(const GURL& url) override;
+  const bookmarks::BookmarkNode* Get(const GURL& url) const override;
+  const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const override;
+  bool IsReadingListBookmark(
+      const bookmarks::BookmarkNode* node) const override;
   void Delete(const GURL& url) override;
   const bookmarks::BookmarkNode* GetRoot() const override;
   size_t size() const override;
   size_t unread_size() const override;
   void SetReadStatus(const GURL& url, bool read) override;
+  bool IsLoaded() const override;
 
   // Finds the child in the bookmark tree by URL. Returns nullptr if not found.
   // Not recursive since the reading list bookmark tree only has a folder root
   // node and one level of children.
-  bookmarks::BookmarkNode* FindBookmarkByURL(const GURL& url);
+  bookmarks::BookmarkNode* FindBookmarkByURL(const GURL& url) const;
 
   void RemoveBookmark(const GURL& url);
   const bookmarks::BookmarkNode* AddBookmark(const ReadingListEntry* entry);
@@ -54,6 +60,11 @@ class ReadingListManagerImpl : public ReadingListManager,
 
   // Auto increment bookmark id. Will not be persisted and only used in memory.
   int64_t maximum_id_;
+
+  // Whether the |reading_list_model_| is loaded.
+  bool loaded_;
+
+  base::ObserverList<Observer> observers_;
 };
 
 #endif  // CHROME_BROWSER_READING_LIST_ANDROID_READING_LIST_MANAGER_IMPL_H_

chrome/browser/reading_list/android/reading_list_manager_impl_unittest.cc

@@ -10,8 +10,10 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/simple_test_clock.h"
+#include "chrome/browser/reading_list/android/reading_list_manager.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/reading_list/core/reading_list_model_impl.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using BookmarkNode = bookmarks::BookmarkNode;
@@ -28,6 +30,15 @@ constexpr char kReadStatusKey[] = "read_status";
 constexpr char kReadStatusRead[] = "true";
 constexpr char kReadStatusUnread[] = "false";
 
+class MockObserver : public ReadingListManager::Observer {
+ public:
+  MockObserver() = default;
+  ~MockObserver() override = default;
+
+  // ReadingListManager::Observer implementation.
+  MOCK_METHOD(void, ReadingListLoaded, (), (override));
+};
+
 class ReadingListManagerImplTest : public testing::Test {
  public:
   ReadingListManagerImplTest() = default;
@@ -38,19 +49,25 @@ class ReadingListManagerImplTest : public testing::Test {
         /*storage_layer=*/nullptr, /*pref_service=*/nullptr, &clock_);
     manager_ =
         std::make_unique<ReadingListManagerImpl>(reading_list_model_.get());
+    manager_->AddObserver(observer());
+    EXPECT_TRUE(manager()->IsLoaded());
   }
 
+  void TearDown() override { manager_->RemoveObserver(observer()); }
+
  protected:
   ReadingListManager* manager() { return manager_.get(); }
   ReadingListModelImpl* reading_list_model() {
     return reading_list_model_.get();
   }
   base::SimpleTestClock* clock() { return &clock_; }
+  MockObserver* observer() { return &observer_; }
 
  private:
   base::SimpleTestClock clock_;
   std::unique_ptr<ReadingListModelImpl> reading_list_model_;
   std::unique_ptr<ReadingListManager> manager_;
+  MockObserver observer_;
 };
 
 // Verifies the states without any reading list data.
@@ -96,6 +113,9 @@ TEST_F(ReadingListManagerImplTest, AddGetDelete) {
   EXPECT_EQ(kReadStatusUnread, read_status)
       << "By default the reading list node is marked as unread.";
 
+  // Gets an invalid URL.
+  EXPECT_EQ(nullptr, manager()->Get(GURL("invalid spec")));
+
   // Deletes the node.
   manager()->Delete(url);
   EXPECT_EQ(0u, manager()->size());
@@ -103,6 +123,26 @@ TEST_F(ReadingListManagerImplTest, AddGetDelete) {
   EXPECT_TRUE(manager()->GetRoot()->children().empty());
 }
 
+// Verifies GetNodeByID() and IsReadingListBookmark() works correctly.
+TEST_F(ReadingListManagerImplTest, GetNodeByIDIsReadingListBookmark) {
+  GURL url(kURL);
+  const auto* node = manager()->Add(url, kTitle);
+
+  // Find the root.
+  EXPECT_EQ(manager()->GetRoot(),
+            manager()->GetNodeByID(manager()->GetRoot()->id()));
+  EXPECT_TRUE(manager()->IsReadingListBookmark(manager()->GetRoot()));
+
+  // Find existing node.
+  EXPECT_EQ(node, manager()->GetNodeByID(node->id()));
+  EXPECT_TRUE(manager()->IsReadingListBookmark(node));
+
+  // Non existing node.
+  node = manager()->GetNodeByID(12345);
+  EXPECT_FALSE(node);
+  EXPECT_FALSE(manager()->IsReadingListBookmark(node));
+}
+
 // Verifies Add() the same URL twice will not invalidate returned pointers, and
 // the content is updated.
 TEST_F(ReadingListManagerImplTest, AddTwice) {