Commit 18b34a3e authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

Revert "Read later: Implement more ReadingListModelObserver methods."

This reverts commit 00592994.

Reason for revert: Turned android-asan red. crbug.com/1147311

Original change's description:
> 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/+/2518774
> Reviewed-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}

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

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

Bug: 1133504
Change-Id: Icf7dfee5df7d35fe59f9e806b72c8a69f1d93f08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532461
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826290}
parent db9ae794
...@@ -65,7 +65,7 @@ void ReadingListManagerImpl::ReadingListModelLoaded( ...@@ -65,7 +65,7 @@ void ReadingListManagerImpl::ReadingListModelLoaded(
// Constructs the bookmark tree. // Constructs the bookmark tree.
root_->DeleteAll(); root_->DeleteAll();
for (const auto& url : model->Keys()) for (const auto& url : model->Keys())
AddOrUpdateBookmark(model->GetEntryByURL(url)); AddBookmark(model->GetEntryByURL(url));
loaded_ = true; loaded_ = true;
...@@ -77,22 +77,7 @@ void ReadingListManagerImpl::ReadingListDidAddEntry( ...@@ -77,22 +77,7 @@ void ReadingListManagerImpl::ReadingListDidAddEntry(
const ReadingListModel* model, const ReadingListModel* model,
const GURL& url, const GURL& url,
reading_list::EntrySource source) { reading_list::EntrySource source) {
AddOrUpdateBookmark(model->GetEntryByURL(url)); AddBookmark(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) { void ReadingListManagerImpl::AddObserver(Observer* observer) {
...@@ -111,8 +96,7 @@ const BookmarkNode* ReadingListManagerImpl::Add(const GURL& url, ...@@ -111,8 +96,7 @@ const BookmarkNode* ReadingListManagerImpl::Add(const GURL& url,
const auto& new_entry = reading_list_model_->AddEntry( const auto& new_entry = reading_list_model_->AddEntry(
url, title, reading_list::ADDED_VIA_CURRENT_APP); url, title, reading_list::ADDED_VIA_CURRENT_APP);
const auto* node = FindBookmarkByURL(new_entry.URL()); const auto* node = FindBookmarkByURL(new_entry.URL());
DCHECK(node) DCHECK(node) << "Bookmark node should have been created.";
<< "Bookmark node should have been create in ReadingListDidAddEntry().";
return node; return node;
} }
...@@ -144,6 +128,8 @@ bool ReadingListManagerImpl::IsReadingListBookmark( ...@@ -144,6 +128,8 @@ bool ReadingListManagerImpl::IsReadingListBookmark(
void ReadingListManagerImpl::Delete(const GURL& url) { void ReadingListManagerImpl::Delete(const GURL& url) {
DCHECK(reading_list_model_->loaded()); DCHECK(reading_list_model_->loaded());
RemoveBookmark(url);
reading_list_model_->RemoveEntryByURL(url); reading_list_model_->RemoveEntryByURL(url);
} }
...@@ -215,7 +201,8 @@ void ReadingListManagerImpl::RemoveBookmark(const GURL& url) { ...@@ -215,7 +201,8 @@ void ReadingListManagerImpl::RemoveBookmark(const GURL& url) {
root_->Remove(root_->GetIndexOf(node)); root_->Remove(root_->GetIndexOf(node));
} }
const BookmarkNode* ReadingListManagerImpl::AddOrUpdateBookmark( // Adds a reading list entry to the bookmark tree.
const BookmarkNode* ReadingListManagerImpl::AddBookmark(
const ReadingListEntry* entry) { const ReadingListEntry* entry) {
if (!entry) if (!entry)
return nullptr; return nullptr;
......
...@@ -30,10 +30,6 @@ class ReadingListManagerImpl : public ReadingListManager, ...@@ -30,10 +30,6 @@ class ReadingListManagerImpl : public ReadingListManager,
void ReadingListDidAddEntry(const ReadingListModel* model, void ReadingListDidAddEntry(const ReadingListModel* model,
const GURL& url, const GURL& url,
reading_list::EntrySource source) override; 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. // ReadingListManager implementation.
void AddObserver(Observer* observer) override; void AddObserver(Observer* observer) override;
...@@ -58,8 +54,7 @@ class ReadingListManagerImpl : public ReadingListManager, ...@@ -58,8 +54,7 @@ class ReadingListManagerImpl : public ReadingListManager,
bookmarks::BookmarkNode* FindBookmarkByURL(const GURL& url) const; bookmarks::BookmarkNode* FindBookmarkByURL(const GURL& url) const;
void RemoveBookmark(const GURL& url); void RemoveBookmark(const GURL& url);
const bookmarks::BookmarkNode* AddOrUpdateBookmark( const bookmarks::BookmarkNode* AddBookmark(const ReadingListEntry* entry);
const ReadingListEntry* entry);
// Contains reading list data, outlives this class. // Contains reading list data, outlives this class.
ReadingListModel* reading_list_model_; ReadingListModel* reading_list_model_;
......
...@@ -197,8 +197,8 @@ TEST_F(ReadingListManagerImplTest, ReadStatus) { ...@@ -197,8 +197,8 @@ TEST_F(ReadingListManagerImplTest, ReadStatus) {
EXPECT_FALSE(manager()->GetReadStatus(manager()->GetRoot())); EXPECT_FALSE(manager()->GetReadStatus(manager()->GetRoot()));
} }
// Verifies the bookmark node is added when sync or other source adds the // Verifies ReadingListDidAddEntry() API that is being called after
// reading list entry from |reading_list_model_|. // ReadingListModel::AddEntry() is called.
TEST_F(ReadingListManagerImplTest, ReadingListDidAddEntry) { TEST_F(ReadingListManagerImplTest, ReadingListDidAddEntry) {
GURL url(kURL); GURL url(kURL);
reading_list_model()->AddEntry(url, kTitle, reading_list::ADDED_VIA_SYNC); reading_list_model()->AddEntry(url, kTitle, reading_list::ADDED_VIA_SYNC);
...@@ -209,38 +209,4 @@ TEST_F(ReadingListManagerImplTest, ReadingListDidAddEntry) { ...@@ -209,38 +209,4 @@ TEST_F(ReadingListManagerImplTest, ReadingListDidAddEntry) {
EXPECT_EQ(1u, manager()->size()); 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 } // namespace
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment