Revert "Read later: Add a method to notify update for reading list backend."

This reverts commit d4fbfee2.

Reason for revert: Reverting because it's on top of https://chromium-review.googlesource.com/c/chromium/src/+/2518774 , which needs to be reverted, because of crbug.com/1147311

Original change's description:
> Read later: Add a method to notify update for reading list backend.
>
> Adds an observer method to notify update for reading list backend.
>
> Bug: 1139133
> Change-Id: Ic5483e075086d48334a9e347125048b5699c2560
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519151
> Reviewed-by: Theresa  <twellington@chromium.org>
> Reviewed-by: Brandon Wylie <wylieb@chromium.org>
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825086}

TBR=dtrainor@chromium.org,twellington@chromium.org,wylieb@chromium.org,xingliu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1139133
Change-Id: I89026b82f1760fbb83e80b8723a9329d323211b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532074Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826267}

chrome/browser/android/bookmarks/bookmark_bridge.cc

@@ -1158,6 +1158,9 @@ void BookmarkBridge::ExtensiveBookmarkChangesEnded(BookmarkModel* model) {
 }
 
 void BookmarkBridge::PartnerShimChanged(PartnerBookmarksShim* shim) {
+  if (!IsLoaded())
+    return;
+
   BookmarkModelChanged();
 }
 
@@ -1173,10 +1176,6 @@ void BookmarkBridge::ReadingListLoaded() {
   NotifyIfDoneLoading();
 }
 
-void BookmarkBridge::ReadingListChanged() {
-  BookmarkModelChanged();
-}
-
 void BookmarkBridge::ReorderChildren(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,

chrome/browser/android/bookmarks/bookmark_bridge.h

@@ -296,7 +296,6 @@ class BookmarkBridge : public bookmarks::BaseBookmarkModelObserver,
 
   // Override ReadingListManager::Observer
   void ReadingListLoaded() override;
-  void ReadingListChanged() override;
 
   void DestroyJavaObject();
 

chrome/browser/reading_list/android/reading_list_manager.h

@@ -32,9 +32,6 @@ class ReadingListManager : public KeyedService {
     // Called when the reading list backend is loaded.
     virtual void ReadingListLoaded() {}
 
-    // Called when the reading list backend is changed.
-    virtual void ReadingListChanged() {}
-
     Observer() = default;
     ~Observer() override = default;
   };

chrome/browser/reading_list/android/reading_list_manager_impl.cc

@@ -47,10 +47,7 @@ bool SyncToBookmark(const ReadingListEntry& entry, BookmarkNode* bookmark) {
 
 ReadingListManagerImpl::ReadingListManagerImpl(
     ReadingListModel* reading_list_model)
-    : reading_list_model_(reading_list_model),
-      maximum_id_(0L),
-      loaded_(false),
-      performing_batch_update_(false) {
+    : 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());
@@ -98,28 +95,6 @@ void ReadingListManagerImpl::ReadingListDidMoveEntry(
   AddOrUpdateBookmark(moved_entry);
 }
 
-void ReadingListManagerImpl::ReadingListDidApplyChanges(
-    ReadingListModel* model) {
-  // Ignores ReadingListDidApplyChanges() invocations during batch update.
-  if (performing_batch_update_)
-    return;
-
-  NotifyReadingListChanged();
-}
-
-void ReadingListManagerImpl::ReadingListModelBeganBatchUpdates(
-    const ReadingListModel* model) {
-  performing_batch_update_ = true;
-}
-
-void ReadingListManagerImpl::ReadingListModelCompletedBatchUpdates(
-    const ReadingListModel* model) {
-  performing_batch_update_ = false;
-
-  // Batch update is done, notify the observer only once.
-  NotifyReadingListChanged();
-}
-
 void ReadingListManagerImpl::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
@@ -258,8 +233,3 @@ const BookmarkNode* ReadingListManagerImpl::AddOrUpdateBookmark(
   bool success = SyncToBookmark(*entry, new_node.get());
   return success ? root_->Add(std::move(new_node)) : nullptr;
 }
-
-void ReadingListManagerImpl::NotifyReadingListChanged() {
-  for (Observer& observer : observers_)
-    observer.ReadingListChanged();
-}

chrome/browser/reading_list/android/reading_list_manager_impl.h

@@ -34,11 +34,6 @@ class ReadingListManagerImpl : public ReadingListManager,
                                   const GURL& url) override;
   void ReadingListDidMoveEntry(const ReadingListModel* model,
                                const GURL& url) override;
-  void ReadingListDidApplyChanges(ReadingListModel* model) override;
-  void ReadingListModelBeganBatchUpdates(
-      const ReadingListModel* model) override;
-  void ReadingListModelCompletedBatchUpdates(
-      const ReadingListModel* model) override;
 
   // ReadingListManager implementation.
   void AddObserver(Observer* observer) override;
@@ -65,7 +60,6 @@ class ReadingListManagerImpl : public ReadingListManager,
   void RemoveBookmark(const GURL& url);
   const bookmarks::BookmarkNode* AddOrUpdateBookmark(
       const ReadingListEntry* entry);
-  void NotifyReadingListChanged();
 
   // Contains reading list data, outlives this class.
   ReadingListModel* reading_list_model_;
@@ -79,9 +73,6 @@ class ReadingListManagerImpl : public ReadingListManager,
   // Whether the |reading_list_model_| is loaded.
   bool loaded_;
 
-  // Whether |reading_list_model_| is in batch update mode.
-  bool performing_batch_update_;
-
   base::ObserverList<Observer> observers_;
 };
 

chrome/browser/reading_list/android/reading_list_manager_impl_unittest.cc

@@ -38,7 +38,6 @@ class MockObserver : public ReadingListManager::Observer {
 
   // ReadingListManager::Observer implementation.
   MOCK_METHOD(void, ReadingListLoaded, (), (override));
-  MOCK_METHOD(void, ReadingListChanged, (), (override));
 };
 
 class ReadingListManagerImplTest : public testing::Test {
@@ -65,21 +64,6 @@ class ReadingListManagerImplTest : public testing::Test {
   base::SimpleTestClock* clock() { return &clock_; }
   MockObserver* observer() { return &observer_; }
 
-  const BookmarkNode* Add(const GURL& url, const std::string& title) {
-    EXPECT_CALL(*observer(), ReadingListChanged());
-    return manager()->Add(url, title);
-  }
-
-  void Delete(const GURL& url) {
-    EXPECT_CALL(*observer(), ReadingListChanged());
-    manager()->Delete(url);
-  }
-
-  void SetReadStatus(const GURL& url, bool read) {
-    EXPECT_CALL(*observer(), ReadingListChanged());
-    manager()->SetReadStatus(url, read);
-  }
-
  private:
   base::SimpleTestClock clock_;
   std::unique_ptr<ReadingListModelImpl> reading_list_model_;
@@ -114,7 +98,7 @@ TEST_F(ReadingListManagerImplTest, Load) {
 TEST_F(ReadingListManagerImplTest, AddGetDelete) {
   // Adds a node.
   GURL url(kURL);
-  Add(url, kTitle);
+  manager()->Add(url, kTitle);
   EXPECT_EQ(1u, manager()->size());
   EXPECT_EQ(1u, manager()->unread_size());
   EXPECT_EQ(1u, manager()->GetRoot()->children().size())
@@ -134,7 +118,7 @@ TEST_F(ReadingListManagerImplTest, AddGetDelete) {
   EXPECT_EQ(nullptr, manager()->Get(GURL("invalid spec")));
 
   // Deletes the node.
-  Delete(url);
+  manager()->Delete(url);
   EXPECT_EQ(0u, manager()->size());
   EXPECT_EQ(0u, manager()->unread_size());
   EXPECT_TRUE(manager()->GetRoot()->children().empty());
@@ -143,7 +127,7 @@ TEST_F(ReadingListManagerImplTest, AddGetDelete) {
 // Verifies GetNodeByID() and IsReadingListBookmark() works correctly.
 TEST_F(ReadingListManagerImplTest, GetNodeByIDIsReadingListBookmark) {
   GURL url(kURL);
-  const auto* node = Add(url, kTitle);
+  const auto* node = manager()->Add(url, kTitle);
 
   // Find the root.
   EXPECT_EQ(manager()->GetRoot(),
@@ -170,8 +154,8 @@ TEST_F(ReadingListManagerImplTest, GetNodeByIDIsReadingListBookmark) {
 TEST_F(ReadingListManagerImplTest, AddTwice) {
   // Adds a node.
   GURL url(kURL);
-  const auto* node = Add(url, kTitle);
-  const auto* new_node = Add(url, kTitle1);
+  const auto* node = manager()->Add(url, kTitle);
+  const auto* new_node = manager()->Add(url, kTitle1);
   EXPECT_EQ(node, new_node) << "Add same URL shouldn't invalidate pointers.";
   EXPECT_EQ(kTitle1, base::UTF16ToUTF8(node->GetTitle()));
 }
@@ -179,15 +163,14 @@ TEST_F(ReadingListManagerImplTest, AddTwice) {
 // Verifes SetReadStatus()/GetReadStatus() API.
 TEST_F(ReadingListManagerImplTest, ReadStatus) {
   GURL url(kURL);
-
-  // No op when no reading list entries.
   manager()->SetReadStatus(url, true);
   EXPECT_EQ(0u, manager()->size());
 
-  // Add a node and mark as read.
-  Add(url, kTitle);
-  SetReadStatus(url, true);
+  // Add a node.
+  manager()->Add(url, kTitle);
+  manager()->SetReadStatus(url, true);
 
+  // Mark as read.
   const BookmarkNode* node = manager()->Get(url);
   ASSERT_TRUE(node);
   EXPECT_EQ(url, node->url());
@@ -198,7 +181,7 @@ TEST_F(ReadingListManagerImplTest, ReadStatus) {
   EXPECT_TRUE(manager()->GetReadStatus(node));
 
   // Mark as unread.
-  SetReadStatus(url, false);
+  manager()->SetReadStatus(url, false);
   node = manager()->Get(url);
   node->GetMetaInfo(kReadStatusKey, &read_status);
   EXPECT_EQ(kReadStatusUnread, read_status);
@@ -218,7 +201,6 @@ TEST_F(ReadingListManagerImplTest, ReadStatus) {
 // reading list entry from |reading_list_model_|.
 TEST_F(ReadingListManagerImplTest, ReadingListDidAddEntry) {
   GURL url(kURL);
-  EXPECT_CALL(*observer(), ReadingListChanged()).RetiresOnSaturation();
   reading_list_model()->AddEntry(url, kTitle, reading_list::ADDED_VIA_SYNC);
 
   const auto* node = manager()->Get(url);
@@ -233,13 +215,13 @@ TEST_F(ReadingListManagerImplTest, ReadingListWillRemoveEntry) {
   GURL url(kURL);
 
   // Adds a node.
-  const auto* node = Add(url, kTitle);
+  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_|.
-  EXPECT_CALL(*observer(), ReadingListChanged()).RetiresOnSaturation();
   reading_list_model()->RemoveEntryByURL(url);
   node = manager()->Get(url);
   EXPECT_FALSE(node);
@@ -252,11 +234,12 @@ TEST_F(ReadingListManagerImplTest, ReadingListWillMoveEntry) {
   GURL url(kURL);
 
   // Adds a node.
-  const auto* node = Add(url, kTitle);
+  manager()->Add(url, kTitle);
+  const auto* node = manager()->Get(url);
   EXPECT_TRUE(node);
   EXPECT_FALSE(manager()->GetReadStatus(node));
 
-  SetReadStatus(url, true);
+  reading_list_model()->SetReadStatus(url, true);
   EXPECT_TRUE(manager()->GetReadStatus(node));
 }