Refactor Reading List Model to use URL as key.

components/reading_list/ios/reading_list_model_unittest.mm

Index: components/reading_list/ios/reading_list_model_unittest.mm
diff --git a/components/reading_list/ios/reading_list_model_unittest.mm b/components/reading_list/ios/reading_list_model_unittest.mm
index d56e3248ae5aebe0c3ab62ec7a849eb60d465ce6..970bcc46f3e379e30647b06cb71f6857beeaf679 100644
--- a/components/reading_list/ios/reading_list_model_unittest.mm
+++ b/components/reading_list/ios/reading_list_model_unittest.mm
@@ -9,6 +9,7 @@
 #import "base/test/ios/wait_util.h"
 #include "components/reading_list/ios/reading_list_model_impl.h"
 #include "components/reading_list/ios/reading_list_model_storage.h"
+#include "components/reading_list/ios/reading_list_store_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -25,47 +26,61 @@ class TestReadingListStorageObserver {
 class TestReadingListStorage : public ReadingListModelStorage {
  public:
   TestReadingListStorage(TestReadingListStorageObserver* observer)
-      : read_(new std::vector<ReadingListEntry>()),
-        unread_(new std::vector<ReadingListEntry>()),
+      : entries_(new ReadingListStoreDelegate::ReadingListEntries()),
         observer_(observer) {}
 
   void AddSampleEntries() {
     // Adds timer and interlace read/unread entry creation to avoid having two
     // entries with the same creation timestamp.
     ReadingListEntry unread_a(GURL("http://unread_a.com"), "unread_a");
+    entries_->insert(
+        std::make_pair(GURL("http://unread_a.com"), std::move(unread_a)));
     base::test::ios::SpinRunLoopWithMinDelay(
         base::TimeDelta::FromMilliseconds(5));
+
     ReadingListEntry read_a(GURL("http://read_a.com"), "read_a");
+    read_a.SetRead(true);
+    entries_->insert(
+        std::make_pair(GURL("http://read_a.com"), std::move(read_a)));
     base::test::ios::SpinRunLoopWithMinDelay(
         base::TimeDelta::FromMilliseconds(5));
+
     ReadingListEntry unread_b(GURL("http://unread_b.com"), "unread_b");
+    entries_->insert(
+        std::make_pair(GURL("http://unread_b.com"), std::move(unread_b)));
     base::test::ios::SpinRunLoopWithMinDelay(
         base::TimeDelta::FromMilliseconds(5));
+
     ReadingListEntry read_b(GURL("http://read_b.com"), "read_b");
+    read_b.SetRead(true);
+    entries_->insert(
+        std::make_pair(GURL("http://read_b.com"), std::move(read_b)));
     base::test::ios::SpinRunLoopWithMinDelay(
         base::TimeDelta::FromMilliseconds(5));
+
     ReadingListEntry unread_c(GURL("http://unread_c.com"), "unread_c");
+    entries_->insert(
+        std::make_pair(GURL("http://unread_c.com"), std::move(unread_c)));
     base::test::ios::SpinRunLoopWithMinDelay(
         base::TimeDelta::FromMilliseconds(5));
+
     ReadingListEntry read_c(GURL("http://read_c.com"), "read_c");
+    read_c.SetRead(true);
+    entries_->insert(
+        std::make_pair(GURL("http://read_c.com"), std::move(read_c)));
     base::test::ios::SpinRunLoopWithMinDelay(
         base::TimeDelta::FromMilliseconds(5));
+
     ReadingListEntry unread_d(GURL("http://unread_d.com"), "unread_d");
+    entries_->insert(
+        std::make_pair(GURL("http://unread_d.com"), std::move(unread_d)));
     base::test::ios::SpinRunLoopWithMinDelay(
         base::TimeDelta::FromMilliseconds(5));
-    read_->push_back(std::move(read_c));
-    read_->push_back(std::move(read_a));
-    read_->push_back(std::move(read_b));
-
-    unread_->push_back(std::move(unread_a));
-    unread_->push_back(std::move(unread_d));
-    unread_->push_back(std::move(unread_c));
-    unread_->push_back(std::move(unread_b));
   }
 
   void SetReadingListModel(ReadingListModel* model,
-                           ReadingListStoreDelegate* delegate_) override {
-    delegate_->StoreLoaded(std::move(unread_), std::move(read_));
+                           ReadingListStoreDelegate* delegate) override {
+    delegate->StoreLoaded(std::move(entries_));
   }
 
   syncer::ModelTypeSyncBridge* GetModelTypeSyncBridge() override {
@@ -78,7 +93,7 @@ class TestReadingListStorage : public ReadingListModelStorage {
 
   // Saves or updates an entry. If the entry is not yet in the database, it is
   // created.
-  void SaveEntry(const ReadingListEntry& entry, bool read) override {
+  void SaveEntry(const ReadingListEntry& entry) override {
     observer_->ReadingListDidSaveEntry();
   }
 
@@ -88,8 +103,7 @@ class TestReadingListStorage : public ReadingListModelStorage {
   }
 
  private:
-  std::unique_ptr<std::vector<ReadingListEntry>> read_;
-  std::unique_ptr<std::vector<ReadingListEntry>> unread_;
+  std::unique_ptr<ReadingListStoreDelegate::ReadingListEntries> entries_;
   TestReadingListStorageObserver* observer_;
 };
 
@@ -114,37 +128,31 @@ class ReadingListModelTest : public ReadingListModelObserver,
   void ClearCounts() {
     observer_loaded_ = observer_started_batch_update_ =
         observer_completed_batch_update_ = observer_deleted_ =
-            observer_remove_unread_ = observer_remove_read_ = observer_move_ =
-                observer_add_unread_ = observer_add_read_ =
-                    observer_update_unread_ = observer_update_read_ =
-                        observer_did_apply_ = storage_saved_ =
-                            storage_removed_ = 0;
+            observer_remove_ = observer_move_ = observer_add_ =
+                observer_did_add_ = observer_update_ = observer_did_apply_ =
+                    storage_saved_ = storage_removed_ = 0;
   }
 
   void AssertObserverCount(int observer_loaded,
                            int observer_started_batch_update,
                            int observer_completed_batch_update,
                            int observer_deleted,
-                           int observer_remove_unread,
-                           int observer_remove_read,
+                           int observer_remove,
                            int observer_move,
-                           int observer_add_unread,
-                           int observer_add_read,
-                           int observer_update_unread,
-                           int observer_update_read,
+                           int observer_add,
+                           int observer_update,
                            int observer_did_apply) {
     ASSERT_EQ(observer_loaded, observer_loaded_);
     ASSERT_EQ(observer_started_batch_update, observer_started_batch_update_);
     ASSERT_EQ(observer_completed_batch_update,
               observer_completed_batch_update_);
     ASSERT_EQ(observer_deleted, observer_deleted_);
-    ASSERT_EQ(observer_remove_unread, observer_remove_unread_);
-    ASSERT_EQ(observer_remove_read, observer_remove_read_);
+    ASSERT_EQ(observer_remove, observer_remove_);
     ASSERT_EQ(observer_move, observer_move_);
-    ASSERT_EQ(observer_add_unread, observer_add_unread_);
-    ASSERT_EQ(observer_add_read, observer_add_read_);
-    ASSERT_EQ(observer_update_unread, observer_update_unread_);
-    ASSERT_EQ(observer_update_read, observer_update_read_);
+    // Add and did_add should be the same.
+    ASSERT_EQ(observer_add, observer_add_);
+    ASSERT_EQ(observer_add, observer_did_add_);
+    ASSERT_EQ(observer_update, observer_update_);
     ASSERT_EQ(observer_did_apply, observer_did_apply_);
   }
 
@@ -168,34 +176,25 @@ class ReadingListModelTest : public ReadingListModelObserver,
   void ReadingListModelBeingDeleted(const ReadingListModel* model) override {
     observer_deleted_ += 1;
   }
-  void ReadingListWillRemoveUnreadEntry(const ReadingListModel* model,
-                                        size_t index) override {
-    observer_remove_unread_ += 1;
+  void ReadingListWillRemoveEntry(const ReadingListModel* model,
+                                  const GURL& url) override {
+    observer_remove_ += 1;
   }
   void ReadingListWillMoveEntry(const ReadingListModel* model,
-                                size_t index,
-                                bool read) override {
+                                const GURL& url) override {
     observer_move_ += 1;
   }
-  void ReadingListWillRemoveReadEntry(const ReadingListModel* model,
-                                      size_t index) override {
-    observer_remove_read_ += 1;
-  }
-  void ReadingListWillAddUnreadEntry(const ReadingListModel* model,
-                                     const ReadingListEntry& entry) override {
-    observer_add_unread_ += 1;
+  void ReadingListWillAddEntry(const ReadingListModel* model,
+                               const ReadingListEntry& entry) override {
+    observer_add_ += 1;
   }
-  void ReadingListWillAddReadEntry(const ReadingListModel* model,
-                                   const ReadingListEntry& entry) override {
-    observer_add_read_ += 1;
+  void ReadingListDidAddEntry(const ReadingListModel* model,
+                              const GURL& url) override {
+    observer_did_add_ += 1;
   }
-  void ReadingListWillUpdateUnreadEntry(const ReadingListModel* model,
-                                        size_t index) override {
-    observer_update_unread_ += 1;
-  }
-  void ReadingListWillUpdateReadEntry(const ReadingListModel* model,
-                                      size_t index) override {
-    observer_update_read_ += 1;
+  void ReadingListWillUpdateEntry(const ReadingListModel* model,
+                                  const GURL& url) override {
+    observer_update_ += 1;
   }
   void ReadingListDidApplyChanges(ReadingListModel* model) override {
     observer_did_apply_ += 1;
@@ -204,6 +203,29 @@ class ReadingListModelTest : public ReadingListModelObserver,
   void ReadingListDidSaveEntry() override { storage_saved_ += 1; }
   void ReadingListDidRemoveEntry() override { storage_removed_ += 1; }
 
+  size_t UnreadSize() {
+    size_t size = 0;
+    for (const auto& url : model_->Keys()) {
+      const ReadingListEntry* entry = model_->GetEntryByURL(url);
+      if (!entry->IsRead()) {
+        size++;
+      }
+    }
+    DCHECK_EQ(size, model_->unread_size());
+    return size;
+  }
+
+  size_t ReadSize() {
+    size_t size = 0;
+    for (const auto& url : model_->Keys()) {
+      const ReadingListEntry* entry = model_->GetEntryByURL(url);
+      if (entry->IsRead()) {
+        size++;
+      }
+    }
+    return size;
+  }
+
   void Callback(const ReadingListEntry& entry) {
     EXPECT_EQ(callback_url, entry.URL());
     EXPECT_EQ(callback_title, entry.Title());
@@ -217,13 +239,11 @@ class ReadingListModelTest : public ReadingListModelObserver,
   int observer_started_batch_update_;
   int observer_completed_batch_update_;
   int observer_deleted_;
-  int observer_remove_unread_;
-  int observer_remove_read_;
+  int observer_remove_;
   int observer_move_;
-  int observer_add_unread_;
-  int observer_add_read_;
-  int observer_update_unread_;
-  int observer_update_read_;
+  int observer_add_;
+  int observer_did_add_;
+  int observer_update_;
   int observer_did_apply_;
   int storage_saved_;
   int storage_removed_;
@@ -234,12 +254,12 @@ class ReadingListModelTest : public ReadingListModelObserver,
 
 TEST_F(ReadingListModelTest, EmptyLoaded) {
   EXPECT_TRUE(model_->loaded());
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
-  EXPECT_EQ(0ul, model_->unread_size());
-  EXPECT_EQ(0ul, model_->read_size());
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
   model_->Shutdown();
   EXPECT_FALSE(model_->loaded());
-  AssertObserverCount(1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 1, 0, 0, 0, 0, 0);
 }
 
 TEST_F(ReadingListModelTest, ModelLoaded) {
@@ -248,17 +268,22 @@ TEST_F(ReadingListModelTest, ModelLoaded) {
   storage->AddSampleEntries();
   SetStorage(std::move(storage));
 
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
-  EXPECT_EQ(model_->read_size(), 3u);
-  EXPECT_EQ(model_->GetReadEntryAtIndex(0).Title(), "read_c");
-  EXPECT_EQ(model_->GetReadEntryAtIndex(1).Title(), "read_b");
-  EXPECT_EQ(model_->GetReadEntryAtIndex(2).Title(), "read_a");
-
-  EXPECT_EQ(model_->unread_size(), 4u);
-  EXPECT_EQ(model_->GetUnreadEntryAtIndex(0).Title(), "unread_d");
-  EXPECT_EQ(model_->GetUnreadEntryAtIndex(1).Title(), "unread_c");
-  EXPECT_EQ(model_->GetUnreadEntryAtIndex(2).Title(), "unread_b");
-  EXPECT_EQ(model_->GetUnreadEntryAtIndex(3).Title(), "unread_a");
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  std::map<GURL, std::string> loaded_entries;
+  int size = 0;
+  for (const auto& url : model_->Keys()) {
+    size++;
+    const ReadingListEntry* entry = model_->GetEntryByURL(url);
+    loaded_entries[url] = entry->Title();
+  }
+  EXPECT_EQ(size, 7);
+  EXPECT_EQ(loaded_entries[GURL("http://unread_a.com")], "unread_a");
+  EXPECT_EQ(loaded_entries[GURL("http://unread_b.com")], "unread_b");
+  EXPECT_EQ(loaded_entries[GURL("http://unread_c.com")], "unread_c");
+  EXPECT_EQ(loaded_entries[GURL("http://unread_d.com")], "unread_d");
+  EXPECT_EQ(loaded_entries[GURL("http://read_a.com")], "read_a");
+  EXPECT_EQ(loaded_entries[GURL("http://read_b.com")], "read_b");
+  EXPECT_EQ(loaded_entries[GURL("http://read_c.com")], "read_c");
 }
 
 TEST_F(ReadingListModelTest, AddEntry) {
@@ -271,15 +296,18 @@ TEST_F(ReadingListModelTest, AddEntry) {
   EXPECT_EQ(GURL("http://example.com"), entry.URL());
   EXPECT_EQ("sample Test", entry.Title());
 
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 1, 0, 1);
   AssertStorageCount(1, 0);
-  EXPECT_EQ(1ul, model_->unread_size());
-  EXPECT_EQ(0ul, model_->read_size());
+  EXPECT_EQ(1ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
   EXPECT_TRUE(model_->HasUnseenEntries());
 
-  const ReadingListEntry& other_entry = model_->GetUnreadEntryAtIndex(0);
-  EXPECT_EQ(GURL("http://example.com"), other_entry.URL());
-  EXPECT_EQ("sample Test", other_entry.Title());
+  const ReadingListEntry* other_entry =
+      model_->GetEntryByURL(GURL("http://example.com"));
+  EXPECT_NE(other_entry, nullptr);
+  EXPECT_FALSE(other_entry->IsRead());
+  EXPECT_EQ(GURL("http://example.com"), other_entry->URL());
+  EXPECT_EQ("sample Test", other_entry->Title());
 }
 
 TEST_F(ReadingListModelTest, SyncAddEntry) {
@@ -287,13 +315,14 @@ TEST_F(ReadingListModelTest, SyncAddEntry) {
   SetStorage(std::move(storage));
   auto entry =
       base::MakeUnique<ReadingListEntry>(GURL("http://example.com"), "sample");
+  entry->SetRead(true);
   ClearCounts();
 
-  model_->SyncAddEntry(std::move(entry), true);
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1);
+  model_->SyncAddEntry(std::move(entry));
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 1, 0, 1);
   AssertStorageCount(0, 0);
-  ASSERT_EQ(model_->unread_size(), 0u);
-  ASSERT_EQ(model_->read_size(), 1u);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(1ul, ReadSize());
   ClearCounts();
 }
 
@@ -304,24 +333,25 @@ TEST_F(ReadingListModelTest, SyncMergeEntry) {
   model_->SetEntryDistilledPath(GURL("http://example.com"),
                                 base::FilePath("distilled/page.html"));
   const ReadingListEntry* local_entry =
-      model_->GetEntryFromURL(GURL("http://example.com"), nullptr);
+      model_->GetEntryByURL(GURL("http://example.com"));
   int64_t local_update_time = local_entry->UpdateTime();
 
   base::test::ios::SpinRunLoopWithMinDelay(
       base::TimeDelta::FromMilliseconds(10));
   auto sync_entry =
       base::MakeUnique<ReadingListEntry>(GURL("http://example.com"), "sample");
+  sync_entry->SetRead(true);
   ASSERT_GT(sync_entry->UpdateTime(), local_update_time);
   int64_t sync_update_time = sync_entry->UpdateTime();
   EXPECT_TRUE(sync_entry->DistilledPath().empty());
 
-  EXPECT_EQ(model_->unread_size(), 1u);
-  EXPECT_EQ(model_->read_size(), 0u);
+  EXPECT_EQ(1ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
 
   ReadingListEntry* merged_entry =
-      model_->SyncMergeEntry(std::move(sync_entry), true);
-  EXPECT_EQ(model_->unread_size(), 0u);
-  EXPECT_EQ(model_->read_size(), 1u);
+      model_->SyncMergeEntry(std::move(sync_entry));
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(1ul, ReadSize());
   EXPECT_EQ(merged_entry->DistilledPath(),
             base::FilePath("distilled/page.html"));
   EXPECT_EQ(merged_entry->UpdateTime(), sync_update_time);
@@ -332,28 +362,28 @@ TEST_F(ReadingListModelTest, RemoveEntryByUrl) {
   SetStorage(std::move(storage));
   model_->AddEntry(GURL("http://example.com"), "sample");
   ClearCounts();
-  EXPECT_NE(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
-  EXPECT_EQ(model_->unread_size(), 1u);
+  EXPECT_NE(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
+  EXPECT_EQ(1ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
   model_->RemoveEntryByURL(GURL("http://example.com"));
-  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 0, 1);
   AssertStorageCount(0, 1);
-  EXPECT_EQ(model_->unread_size(), 0u);
-  EXPECT_EQ(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
+  EXPECT_EQ(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
 
   model_->AddEntry(GURL("http://example.com"), "sample");
-  model_->MarkReadByURL(GURL("http://example.com"));
+  model_->SetReadStatus(GURL("http://example.com"), true);
   ClearCounts();
-  EXPECT_NE(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
-  EXPECT_EQ(model_->read_size(), 1u);
+  EXPECT_NE(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(1ul, ReadSize());
   model_->RemoveEntryByURL(GURL("http://example.com"));
-  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 0, 1);
   AssertStorageCount(0, 1);
-  EXPECT_EQ(model_->read_size(), 0u);
-  EXPECT_EQ(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
+  EXPECT_EQ(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
 }
 
 TEST_F(ReadingListModelTest, RemoveSyncEntryByUrl) {
@@ -361,43 +391,46 @@ TEST_F(ReadingListModelTest, RemoveSyncEntryByUrl) {
   SetStorage(std::move(storage));
   model_->AddEntry(GURL("http://example.com"), "sample");
   ClearCounts();
-  EXPECT_NE(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
-  EXPECT_EQ(model_->unread_size(), 1u);
+  EXPECT_NE(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
+  EXPECT_EQ(1ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
   model_->SyncRemoveEntry(GURL("http://example.com"));
-  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 0, 1);
   AssertStorageCount(0, 0);
-  EXPECT_EQ(model_->unread_size(), 0u);
-  EXPECT_EQ(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
+  EXPECT_EQ(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
 
   model_->AddEntry(GURL("http://example.com"), "sample");
-  model_->MarkReadByURL(GURL("http://example.com"));
+  model_->SetReadStatus(GURL("http://example.com"), true);
   ClearCounts();
-  EXPECT_NE(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
-  EXPECT_EQ(model_->read_size(), 1u);
+  EXPECT_NE(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(1ul, ReadSize());
   model_->SyncRemoveEntry(GURL("http://example.com"));
-  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 0, 1);
   AssertStorageCount(0, 0);
-  EXPECT_EQ(model_->read_size(), 0u);
-  EXPECT_EQ(model_->GetEntryFromURL(GURL("http://example.com"), nullptr),
-            nullptr);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
+  EXPECT_EQ(model_->GetEntryByURL(GURL("http://example.com")), nullptr);
 }
 
 TEST_F(ReadingListModelTest, ReadEntry) {
   model_->AddEntry(GURL("http://example.com"), "sample");
 
   ClearCounts();
-  model_->MarkReadByURL(GURL("http://example.com"));
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1);
-  EXPECT_EQ(0ul, model_->unread_size());
-  EXPECT_EQ(1ul, model_->read_size());
+  model_->SetReadStatus(GURL("http://example.com"), true);
+  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 1);
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(1ul, ReadSize());
   EXPECT_FALSE(model_->HasUnseenEntries());
 
-  const ReadingListEntry& other_entry = model_->GetReadEntryAtIndex(0);
-  EXPECT_EQ(GURL("http://example.com"), other_entry.URL());
-  EXPECT_EQ("sample", other_entry.Title());
+  const ReadingListEntry* other_entry =
+      model_->GetEntryByURL(GURL("http://example.com"));
+  EXPECT_NE(other_entry, nullptr);
+  EXPECT_TRUE(other_entry->IsRead());
+  EXPECT_EQ(GURL("http://example.com"), other_entry->URL());
+  EXPECT_EQ("sample", other_entry->Title());
 }
 
 TEST_F(ReadingListModelTest, EntryFromURL) {
@@ -407,54 +440,56 @@ TEST_F(ReadingListModelTest, EntryFromURL) {
   model_->AddEntry(url1, entry1_title);
 
   // Check call with nullptr |read| parameter.
-  const ReadingListEntry* entry1 = model_->GetEntryFromURL(url1, nullptr);
+  const ReadingListEntry* entry1 = model_->GetEntryByURL(url1);
   EXPECT_NE(nullptr, entry1);
   EXPECT_EQ(entry1_title, entry1->Title());
 
-  bool read;
-  entry1 = model_->GetEntryFromURL(url1, &read);
+  entry1 = model_->GetEntryByURL(url1);
   EXPECT_NE(nullptr, entry1);
   EXPECT_EQ(entry1_title, entry1->Title());
-  EXPECT_EQ(read, false);
-  model_->MarkReadByURL(url1);
-  entry1 = model_->GetEntryFromURL(url1, &read);
+  EXPECT_EQ(entry1->IsRead(), false);
+  model_->SetReadStatus(url1, true);
+  entry1 = model_->GetEntryByURL(url1);
   EXPECT_NE(nullptr, entry1);
   EXPECT_EQ(entry1_title, entry1->Title());
-  EXPECT_EQ(read, true);
+  EXPECT_EQ(entry1->IsRead(), true);
 
-  const ReadingListEntry* entry2 = model_->GetEntryFromURL(url2, &read);
+  const ReadingListEntry* entry2 = model_->GetEntryByURL(url2);
   EXPECT_EQ(nullptr, entry2);
 }
 
 TEST_F(ReadingListModelTest, UnreadEntry) {
   // Setup.
   model_->AddEntry(GURL("http://example.com"), "sample");
-  model_->MarkReadByURL(GURL("http://example.com"));
+  model_->SetReadStatus(GURL("http://example.com"), true);
   ClearCounts();
-  ASSERT_EQ(0ul, model_->unread_size());
-  ASSERT_EQ(1ul, model_->read_size());
+  EXPECT_EQ(0ul, UnreadSize());
+  EXPECT_EQ(1ul, ReadSize());
 
   // Action.
-  model_->MarkUnreadByURL(GURL("http://example.com"));
+  model_->SetReadStatus(GURL("http://example.com"), false);
 
   // Tests.
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1);
-  EXPECT_EQ(1ul, model_->unread_size());
-  EXPECT_EQ(0ul, model_->read_size());
+  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 1);
+  EXPECT_EQ(1ul, UnreadSize());
+  EXPECT_EQ(0ul, ReadSize());
   EXPECT_TRUE(model_->HasUnseenEntries());
 
-  const ReadingListEntry& other_entry = model_->GetUnreadEntryAtIndex(0);
-  EXPECT_EQ(GURL("http://example.com"), other_entry.URL());
-  EXPECT_EQ("sample", other_entry.Title());
+  const ReadingListEntry* other_entry =
+      model_->GetEntryByURL(GURL("http://example.com"));
+  EXPECT_NE(other_entry, nullptr);
+  EXPECT_FALSE(other_entry->IsRead());
+  EXPECT_EQ(GURL("http://example.com"), other_entry->URL());
+  EXPECT_EQ("sample", other_entry->Title());
 }
 
 TEST_F(ReadingListModelTest, BatchUpdates) {
   auto token = model_->BeginBatchUpdates();
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0);
   EXPECT_TRUE(model_->IsPerformingBatchUpdates());
 
   delete token.release();
-  AssertObserverCount(1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 1, 0, 0, 0, 0, 0, 0);
   EXPECT_FALSE(model_->IsPerformingBatchUpdates());
 }
 
@@ -464,28 +499,28 @@ TEST_F(ReadingListModelTest, BatchUpdatesReentrant) {
   EXPECT_FALSE(model_->IsPerformingBatchUpdates());
 
   auto token = model_->BeginBatchUpdates();
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0);
   EXPECT_TRUE(model_->IsPerformingBatchUpdates());
 
   auto second_token = model_->BeginBatchUpdates();
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0);
   EXPECT_TRUE(model_->IsPerformingBatchUpdates());
 
   delete token.release();
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0);
   EXPECT_TRUE(model_->IsPerformingBatchUpdates());
 
   delete second_token.release();
-  AssertObserverCount(1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 1, 0, 0, 0, 0, 0, 0);
   EXPECT_FALSE(model_->IsPerformingBatchUpdates());
 
   // Consequent updates send notifications.
   auto third_token = model_->BeginBatchUpdates();
-  AssertObserverCount(1, 2, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 2, 1, 0, 0, 0, 0, 0, 0);
   EXPECT_TRUE(model_->IsPerformingBatchUpdates());
 
   delete third_token.release();
-  AssertObserverCount(1, 2, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 2, 2, 0, 0, 0, 0, 0, 0);
   EXPECT_FALSE(model_->IsPerformingBatchUpdates());
 }
 
@@ -495,17 +530,17 @@ TEST_F(ReadingListModelTest, UpdateEntryTitle) {
   ClearCounts();
 
   model_->SetEntryTitle(gurl, "ping");
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 1, 1);
   EXPECT_EQ("ping", entry.Title());
 }
 
-TEST_F(ReadingListModelTest, UpdateEntryState) {
+TEST_F(ReadingListModelTest, UpdateEntryDistilledState) {
   const GURL gurl("http://example.com");
   const ReadingListEntry& entry = model_->AddEntry(gurl, "sample");
   ClearCounts();
 
   model_->SetEntryDistilledState(gurl, ReadingListEntry::PROCESSING);
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 1, 1);
   EXPECT_EQ(ReadingListEntry::PROCESSING, entry.DistilledState());
 }
 
@@ -515,7 +550,7 @@ TEST_F(ReadingListModelTest, UpdateDistilledPath) {
   ClearCounts();
 
   model_->SetEntryDistilledPath(gurl, base::FilePath("distilled/page.html"));
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 1, 1);
   EXPECT_EQ(ReadingListEntry::PROCESSED, entry.DistilledState());
   EXPECT_EQ(base::FilePath("distilled/page.html"), entry.DistilledPath());
 }
@@ -528,7 +563,7 @@ TEST_F(ReadingListModelTest, UpdateReadEntryTitle) {
   ClearCounts();
 
   model_->SetEntryTitle(gurl, "ping");
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 1, 1);
   EXPECT_EQ("ping", entry.Title());
 }
 
@@ -540,7 +575,7 @@ TEST_F(ReadingListModelTest, UpdateReadEntryState) {
   ClearCounts();
 
   model_->SetEntryDistilledState(gurl, ReadingListEntry::PROCESSING);
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 1, 1);
   EXPECT_EQ(ReadingListEntry::PROCESSING, entry.DistilledState());
 }
 
@@ -552,70 +587,16 @@ TEST_F(ReadingListModelTest, UpdateReadDistilledPath) {
   ClearCounts();
 
   model_->SetEntryDistilledPath(gurl, base::FilePath("distilled/page.html"));
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 1, 1);
   EXPECT_EQ(ReadingListEntry::PROCESSED, entry.DistilledState());
   EXPECT_EQ(base::FilePath("distilled/page.html"), entry.DistilledPath());
 }
 
-// Tests that the callback is called when the entry is unread.
-TEST_F(ReadingListModelTest, CallbackEntryURLUnread) {
-  // Setup.
-  model_->AddEntry(callback_url, callback_title);
-
-  ASSERT_EQ(0UL, model_->read_size());
-  ASSERT_EQ(1UL, model_->unread_size());
-
-  // Action.
-  bool result = model_->CallbackEntryURL(
-      callback_url,
-      base::Bind(&ReadingListModelTest::Callback, base::Unretained(this)));
-
-  // Test.
-  EXPECT_TRUE(result);
-  EXPECT_TRUE(CallbackCalled());
-}
-
-// Tests that the callback is called when the entry is read.
-TEST_F(ReadingListModelTest, CallbackEntryURLRead) {
-  // Setup.
-  model_->AddEntry(callback_url, callback_title);
-  model_->MarkReadByURL(callback_url);
-
-  ASSERT_EQ(1UL, model_->read_size());
-  ASSERT_EQ(0UL, model_->unread_size());
-
-  // Action.
-  bool result = model_->CallbackEntryURL(
-      callback_url,
-      base::Bind(&ReadingListModelTest::Callback, base::Unretained(this)));
-
-  // Test.
-  EXPECT_TRUE(result);
-  EXPECT_TRUE(CallbackCalled());
-}
-
-// Tests that the callback is not called when the entry is not present.
-TEST_F(ReadingListModelTest, CallbackEntryURLNotPresent) {
-  // Setup.
-  const GURL gurl("http://foo.bar");
-  ASSERT_NE(gurl, callback_url);
-  model_->AddEntry(gurl, callback_title);
-
-  // Action.
-  bool result = model_->CallbackEntryURL(
-      callback_url,
-      base::Bind(&ReadingListModelTest::Callback, base::Unretained(this)));
-
-  // Test.
-  EXPECT_FALSE(result);
-  EXPECT_FALSE(CallbackCalled());
-}
-
 // Tests that ReadingListModel calls CallbackModelBeingDeleted when destroyed.
 TEST_F(ReadingListModelTest, CallbackModelBeingDeleted) {
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
   model_.reset();
-  AssertObserverCount(1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 1, 0, 0, 0, 0, 0);
 }
 
 }  // namespace