Read later: Implement more ReadingListModelObserver methods.

Implements more ReadingListModelObserver methods. When sync happens, we
need to sync the in-memory bookmark tree.

Bug: 1133504
Change-Id: I87f7c9ec0f52a73b436d3b44eb8bd3278cda78ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518774Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824094}

chrome/browser/reading_list/android/reading_list_manager_impl.cc

@@ -65,7 +65,7 @@ void ReadingListManagerImpl::ReadingListModelLoaded(
   // Constructs the bookmark tree.
   root_->DeleteAll();
   for (const auto& url : model->Keys())
-    AddBookmark(model->GetEntryByURL(url));
+    AddOrUpdateBookmark(model->GetEntryByURL(url));
 
   loaded_ = true;
 
@@ -77,7 +77,22 @@ void ReadingListManagerImpl::ReadingListDidAddEntry(
     const ReadingListModel* model,
     const GURL& url,
     reading_list::EntrySource source) {
-  AddBookmark(model->GetEntryByURL(url));
+  AddOrUpdateBookmark(model->GetEntryByURL(url));
+}
+
+void ReadingListManagerImpl::ReadingListWillRemoveEntry(
+    const ReadingListModel* model,
+    const GURL& url) {
+  RemoveBookmark(url);
+}
+
+void ReadingListManagerImpl::ReadingListDidMoveEntry(
+    const ReadingListModel* model,
+    const GURL& url) {
+  DCHECK(reading_list_model_->loaded());
+  const auto* moved_entry = reading_list_model_->GetEntryByURL(url);
+  DCHECK(moved_entry);
+  AddOrUpdateBookmark(moved_entry);
 }
 
 void ReadingListManagerImpl::AddObserver(Observer* observer) {
@@ -96,7 +111,8 @@ const BookmarkNode* ReadingListManagerImpl::Add(const GURL& url,
   const auto& new_entry = reading_list_model_->AddEntry(
       url, title, reading_list::ADDED_VIA_CURRENT_APP);
   const auto* node = FindBookmarkByURL(new_entry.URL());
-  DCHECK(node) << "Bookmark node should have been created.";
+  DCHECK(node)
+      << "Bookmark node should have been create in ReadingListDidAddEntry().";
   return node;
 }
 
@@ -128,8 +144,6 @@ bool ReadingListManagerImpl::IsReadingListBookmark(
 
 void ReadingListManagerImpl::Delete(const GURL& url) {
   DCHECK(reading_list_model_->loaded());
-
-  RemoveBookmark(url);
   reading_list_model_->RemoveEntryByURL(url);
 }
 
@@ -201,8 +215,7 @@ void ReadingListManagerImpl::RemoveBookmark(const GURL& url) {
     root_->Remove(root_->GetIndexOf(node));
 }
 
-// Adds a reading list entry to the bookmark tree.
-const BookmarkNode* ReadingListManagerImpl::AddBookmark(
+const BookmarkNode* ReadingListManagerImpl::AddOrUpdateBookmark(
     const ReadingListEntry* entry) {
   if (!entry)
     return nullptr;

chrome/browser/reading_list/android/reading_list_manager_impl.h

@@ -30,6 +30,10 @@ class ReadingListManagerImpl : public ReadingListManager,
   void ReadingListDidAddEntry(const ReadingListModel* model,
                               const GURL& url,
                               reading_list::EntrySource source) override;
+  void ReadingListWillRemoveEntry(const ReadingListModel* model,
+                                  const GURL& url) override;
+  void ReadingListDidMoveEntry(const ReadingListModel* model,
+                               const GURL& url) override;
 
   // ReadingListManager implementation.
   void AddObserver(Observer* observer) override;
@@ -54,7 +58,8 @@ class ReadingListManagerImpl : public ReadingListManager,
   bookmarks::BookmarkNode* FindBookmarkByURL(const GURL& url) const;
 
   void RemoveBookmark(const GURL& url);
-  const bookmarks::BookmarkNode* AddBookmark(const ReadingListEntry* entry);
+  const bookmarks::BookmarkNode* AddOrUpdateBookmark(
+      const ReadingListEntry* entry);
 
   // Contains reading list data, outlives this class.
   ReadingListModel* reading_list_model_;

chrome/browser/reading_list/android/reading_list_manager_impl_unittest.cc

@@ -197,8 +197,8 @@ TEST_F(ReadingListManagerImplTest, ReadStatus) {
   EXPECT_FALSE(manager()->GetReadStatus(manager()->GetRoot()));
 }
 
-// Verifies ReadingListDidAddEntry() API that is being called after
-// ReadingListModel::AddEntry() is called.
+// Verifies the bookmark node is added when sync or other source adds the
+// reading list entry from |reading_list_model_|.
 TEST_F(ReadingListManagerImplTest, ReadingListDidAddEntry) {
   GURL url(kURL);
   reading_list_model()->AddEntry(url, kTitle, reading_list::ADDED_VIA_SYNC);
@@ -209,4 +209,38 @@ TEST_F(ReadingListManagerImplTest, ReadingListDidAddEntry) {
   EXPECT_EQ(1u, manager()->size());
 }
 
+// Verifies the bookmark node is deleted when sync or other source deletes the
+// reading list entry from |reading_list_model_|.
+TEST_F(ReadingListManagerImplTest, ReadingListWillRemoveEntry) {
+  GURL url(kURL);
+
+  // Adds a node.
+  manager()->Add(url, kTitle);
+  const auto* node = manager()->Get(url);
+  EXPECT_TRUE(node);
+  EXPECT_EQ(url, node->url());
+  EXPECT_EQ(1u, manager()->size());
+
+  // Removes it from |reading_list_model_|.
+  reading_list_model()->RemoveEntryByURL(url);
+  node = manager()->Get(url);
+  EXPECT_FALSE(node);
+  EXPECT_EQ(0u, manager()->size());
+}
+
+// Verifies the bookmark node is updated when sync or other source updates the
+// reading list entry from |reading_list_model_|.
+TEST_F(ReadingListManagerImplTest, ReadingListWillMoveEntry) {
+  GURL url(kURL);
+
+  // Adds a node.
+  manager()->Add(url, kTitle);
+  const auto* node = manager()->Get(url);
+  EXPECT_TRUE(node);
+  EXPECT_FALSE(manager()->GetReadStatus(node));
+
+  reading_list_model()->SetReadStatus(url, true);
+  EXPECT_TRUE(manager()->GetReadStatus(node));
+}
+
 }  // namespace