Refactor the offline page storage to include client namespace and id.

components/offline_pages/offline_page_model_unittest.cc

Index: components/offline_pages/offline_page_model_unittest.cc
diff --git a/components/offline_pages/offline_page_model_unittest.cc b/components/offline_pages/offline_page_model_unittest.cc
index 253dc67da9e427671da31095bbe886f1f17cad82..0b43a01b9d17c1e8fb293d9532eba510a69407ba 100644
--- a/components/offline_pages/offline_page_model_unittest.cc
+++ b/components/offline_pages/offline_page_model_unittest.cc
@@ -13,6 +13,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_mock_time_task_runner.h"
 #include "base/thread_task_runner_handle.h"
@@ -37,12 +38,12 @@ namespace offline_pages {
 
 namespace {
 const GURL kTestUrl("http://example.com");
-const int64_t kTestPageBookmarkId1 = 1234LL;
+const ClientId kTestPageBookmarkId1(BOOKMARK_NAMESPACE, "1234");
 const GURL kTestUrl2("http://other.page.com");
 const GURL kTestUrl3("http://test.xyz");
 const GURL kFileUrl("file:///foo");
-const int64_t kTestPageBookmarkId2 = 5678LL;
-const int64_t kTestPageBookmarkId3 = 42LL;
+const ClientId kTestPageBookmarkId2(BOOKMARK_NAMESPACE, "5678");
+const ClientId kTestPageBookmarkId3(BOOKMARK_NAMESPACE, "42");
 const int64_t kTestFileSize = 876543LL;
 
 }  // namespace
@@ -62,13 +63,13 @@ class OfflinePageModelTest
   // OfflinePageModel::Observer implementation.
   void OfflinePageModelLoaded(OfflinePageModel* model) override;
   void OfflinePageModelChanged(OfflinePageModel* model) override;
-  void OfflinePageDeleted(int64_t bookmark_id) override;
+  void OfflinePageDeleted(int64_t offline_id) override;
 
   // OfflinePageTestArchiver::Observer implementation.
   void SetLastPathCreatedByArchiver(const base::FilePath& file_path) override;
 
   // OfflinePageModel callbacks.
-  void OnSavePageDone(SavePageResult result);
+  void OnSavePageDone(SavePageResult result, int64_t offline_id);
   void OnDeletePageDone(DeletePageResult result);
   void OnClearAllDone();
 
@@ -94,13 +95,15 @@ class OfflinePageModelTest
 
   OfflinePageTestStore* GetStore();
 
-  void SavePage(const GURL& url, int64_t bookmark_id);
+  void SavePage(const GURL& url, ClientId client_id);
   void SavePageWithArchiverResult(const GURL& url,
-                                  int64_t bookmark_id,
+                                  ClientId client_id,
                                   OfflinePageArchiver::ArchiverResult result);
 
   OfflinePageModel* model() { return model_.get(); }
 
+  int64_t last_save_offline_id() const { return last_save_offline_id_; }
+
   SavePageResult last_save_result() const {
     return last_save_result_;
   }
@@ -109,7 +112,7 @@ class OfflinePageModelTest
     return last_delete_result_;
   }
 
-  int64_t last_deleted_bookmark_id() const { return last_deleted_bookmark_id_; }
+  int64_t last_deleted_offline_id() const { return last_deleted_offline_id_; }
 
   const base::FilePath& last_archiver_path() { return last_archiver_path_; }
 
@@ -120,18 +123,19 @@ class OfflinePageModelTest
 
   scoped_ptr<OfflinePageModel> model_;
   SavePageResult last_save_result_;
+  int64_t last_save_offline_id_;
   DeletePageResult last_delete_result_;
   base::FilePath last_archiver_path_;
-  int64_t last_deleted_bookmark_id_;
+  int64_t last_deleted_offline_id_;
 };
 
 OfflinePageModelTest::OfflinePageModelTest()
     : task_runner_(new base::TestMockTimeTaskRunner),
       task_runner_handle_(task_runner_),
       last_save_result_(SavePageResult::CANCELLED),
+      last_save_offline_id_(-1),
       last_delete_result_(DeletePageResult::CANCELLED),
-      last_deleted_bookmark_id_(-1) {
-}
+      last_deleted_offline_id_(-1) {}
 
 OfflinePageModelTest::~OfflinePageModelTest() {
 }
@@ -157,8 +161,8 @@ void OfflinePageModelTest::OfflinePageModelChanged(OfflinePageModel* model) {
   ASSERT_EQ(model_.get(), model);
 }
 
-void OfflinePageModelTest::OfflinePageDeleted(int64_t bookmark_id) {
-  last_deleted_bookmark_id_ = bookmark_id;
+void OfflinePageModelTest::OfflinePageDeleted(int64_t offline_id) {
+  last_deleted_offline_id_ = offline_id;
 }
 
 void OfflinePageModelTest::SetLastPathCreatedByArchiver(
@@ -167,8 +171,10 @@ void OfflinePageModelTest::SetLastPathCreatedByArchiver(
 }
 
 void OfflinePageModelTest::OnSavePageDone(
-    OfflinePageModel::SavePageResult result) {
+    OfflinePageModel::SavePageResult result,
+    int64_t offline_id) {
   last_save_result_ = result;
+  last_save_offline_id_ = offline_id;
 }
 
 void OfflinePageModelTest::OnDeletePageDone(DeletePageResult result) {
@@ -228,20 +234,19 @@ OfflinePageTestStore* OfflinePageModelTest::GetStore() {
   return static_cast<OfflinePageTestStore*>(model()->GetStoreForTesting());
 }
 
-void OfflinePageModelTest::SavePage(const GURL& url, int64_t bookmark_id) {
+void OfflinePageModelTest::SavePage(const GURL& url, ClientId client_id) {
   SavePageWithArchiverResult(
-      url,
-      bookmark_id,
+      url, client_id,
       OfflinePageArchiver::ArchiverResult::SUCCESSFULLY_CREATED);
 }
 
 void OfflinePageModelTest::SavePageWithArchiverResult(
     const GURL& url,
-    int64_t bookmark_id,
+    ClientId client_id,
     OfflinePageArchiver::ArchiverResult result) {
   scoped_ptr<OfflinePageTestArchiver> archiver(BuildArchiver(url, result));
   model()->SavePage(
-      url, bookmark_id, std::move(archiver),
+      url, client_id, std::move(archiver),
       base::Bind(&OfflinePageModelTest::OnSavePageDone, AsWeakPtr()));
   PumpLoop();
 }
@@ -253,7 +258,9 @@ TEST_F(OfflinePageModelTest, SavePageSuccessful) {
 
   OfflinePageTestStore* store = GetStore();
   EXPECT_EQ(kTestUrl, store->last_saved_page().url);
-  EXPECT_EQ(kTestPageBookmarkId1, store->last_saved_page().bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1.id, store->last_saved_page().client_id.id);
+  EXPECT_EQ(kTestPageBookmarkId1.name_space,
+            store->last_saved_page().client_id.name_space);
   // Save last_archiver_path since it will be referred to later.
   base::FilePath archiver_path = last_archiver_path();
   EXPECT_EQ(archiver_path, store->last_saved_page().file_path);
@@ -265,7 +272,9 @@ TEST_F(OfflinePageModelTest, SavePageSuccessful) {
 
   EXPECT_EQ(1UL, offline_pages.size());
   EXPECT_EQ(kTestUrl, offline_pages[0].url);
-  EXPECT_EQ(kTestPageBookmarkId1, offline_pages[0].bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1.id, offline_pages[0].client_id.id);
+  EXPECT_EQ(kTestPageBookmarkId1.name_space,
+            offline_pages[0].client_id.name_space);
   EXPECT_EQ(archiver_path, offline_pages[0].file_path);
   EXPECT_EQ(kTestFileSize, offline_pages[0].file_size);
   EXPECT_EQ(0, offline_pages[0].access_count);
@@ -350,7 +359,7 @@ TEST_F(OfflinePageModelTest, SavePageOfflineArchiverTwoPages) {
   OfflinePageTestStore* store = GetStore();
 
   EXPECT_EQ(kTestUrl2, store->last_saved_page().url);
-  EXPECT_EQ(kTestPageBookmarkId2, store->last_saved_page().bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId2, store->last_saved_page().client_id);
   base::FilePath archiver_path2 = last_archiver_path();
   EXPECT_EQ(archiver_path2, store->last_saved_page().file_path);
   EXPECT_EQ(kTestFileSize, store->last_saved_page().file_size);
@@ -363,7 +372,7 @@ TEST_F(OfflinePageModelTest, SavePageOfflineArchiverTwoPages) {
   PumpLoop();
 
   EXPECT_EQ(kTestUrl, store->last_saved_page().url);
-  EXPECT_EQ(kTestPageBookmarkId1, store->last_saved_page().bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1, store->last_saved_page().client_id);
   base::FilePath archiver_path = last_archiver_path();
   EXPECT_EQ(archiver_path, store->last_saved_page().file_path);
   EXPECT_EQ(kTestFileSize, store->last_saved_page().file_size);
@@ -375,13 +384,13 @@ TEST_F(OfflinePageModelTest, SavePageOfflineArchiverTwoPages) {
 
   EXPECT_EQ(2UL, offline_pages.size());
   EXPECT_EQ(kTestUrl, offline_pages[0].url);
-  EXPECT_EQ(kTestPageBookmarkId1, offline_pages[0].bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1, offline_pages[0].client_id);
   EXPECT_EQ(archiver_path, offline_pages[0].file_path);
   EXPECT_EQ(kTestFileSize, offline_pages[0].file_size);
   EXPECT_EQ(0, offline_pages[0].access_count);
   EXPECT_EQ(0, offline_pages[0].flags);
   EXPECT_EQ(kTestUrl2, offline_pages[1].url);
-  EXPECT_EQ(kTestPageBookmarkId2, offline_pages[1].bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId2, offline_pages[1].client_id);
   EXPECT_EQ(archiver_path2, offline_pages[1].file_path);
   EXPECT_EQ(kTestFileSize, offline_pages[1].file_size);
   EXPECT_EQ(0, offline_pages[1].access_count);
@@ -392,14 +401,14 @@ TEST_F(OfflinePageModelTest, MarkPageAccessed) {
   SavePage(kTestUrl, kTestPageBookmarkId1);
 
   // This will increase access_count by one.
-  model()->MarkPageAccessed(kTestPageBookmarkId1);
+  model()->MarkPageAccessed(last_save_offline_id());
   PumpLoop();
 
   const std::vector<OfflinePageItem>& offline_pages = model()->GetAllPages();
 
   EXPECT_EQ(1UL, offline_pages.size());
   EXPECT_EQ(kTestUrl, offline_pages[0].url);
-  EXPECT_EQ(kTestPageBookmarkId1, offline_pages[0].bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1, offline_pages[0].client_id);
   EXPECT_EQ(kTestFileSize, offline_pages[0].file_size);
   EXPECT_EQ(1, offline_pages[0].access_count);
 }
@@ -411,7 +420,7 @@ TEST_F(OfflinePageModelTest, MarkPageForDeletion) {
 
   // Delete the page with undo tiggerred.
   model()->MarkPageForDeletion(
-      kTestPageBookmarkId1,
+      last_save_offline_id(),
       base::Bind(&OfflinePageModelTest::OnDeletePageDone, AsWeakPtr()));
   PumpLoop();
 
@@ -421,11 +430,11 @@ TEST_F(OfflinePageModelTest, MarkPageForDeletion) {
 
   EXPECT_FALSE(model()->HasOfflinePages());
   EXPECT_EQ(nullptr, model()->GetPageByOnlineURL(kTestUrl));
-  EXPECT_EQ(nullptr, model()->GetPageByBookmarkId(kTestPageBookmarkId1));
+  EXPECT_EQ(nullptr, model()->GetPageByOfflineId(last_save_offline_id()));
   EXPECT_EQ(nullptr, model()->GetPageByOfflineURL(offline_url));
 
   // Undo the deletion.
-  model()->UndoPageDeletion(kTestPageBookmarkId1);
+  model()->UndoPageDeletion(last_save_offline_id());
   PumpLoop();
 
   // GetAllPages will now return the restored page.
@@ -444,7 +453,7 @@ TEST_F(OfflinePageModelTest, FinalizePageDeletion) {
 
   // Mark the page for deletion.
   model()->MarkPageForDeletion(
-      kTestPageBookmarkId1,
+      last_save_offline_id(),
       base::Bind(&OfflinePageModelTest::OnDeletePageDone, AsWeakPtr()));
   PumpLoop();
 
@@ -475,6 +484,7 @@ TEST_F(OfflinePageModelTest, DeletePageSuccessful) {
 
   // Save one page.
   SavePage(kTestUrl, kTestPageBookmarkId1);
+  int64_t offline1 = last_save_offline_id();
   EXPECT_EQ(SavePageResult::SUCCESS, last_save_result());
   EXPECT_EQ(1u, store->GetAllPages().size());
 
@@ -482,41 +492,41 @@ TEST_F(OfflinePageModelTest, DeletePageSuccessful) {
 
   // Save another page.
   SavePage(kTestUrl2, kTestPageBookmarkId2);
+  int64_t offline2 = last_save_offline_id();
   EXPECT_EQ(SavePageResult::SUCCESS, last_save_result());
   EXPECT_EQ(2u, store->GetAllPages().size());
 
   ResetResults();
 
   // Delete one page.
-  model()->DeletePageByBookmarkId(
-      kTestPageBookmarkId1, base::Bind(&OfflinePageModelTest::OnDeletePageDone,
-                                       AsWeakPtr()));
+  model()->DeletePageByOfflineId(
+      offline1,
+      base::Bind(&OfflinePageModelTest::OnDeletePageDone, AsWeakPtr()));
 
   PumpLoop();
 
-  EXPECT_EQ(last_deleted_bookmark_id(), kTestPageBookmarkId1);
+  EXPECT_EQ(last_deleted_offline_id(), offline1);
   EXPECT_EQ(DeletePageResult::SUCCESS, last_delete_result());
   ASSERT_EQ(1u, store->GetAllPages().size());
   EXPECT_EQ(kTestUrl2, store->GetAllPages()[0].url);
 
   // Delete another page.
-  model()->DeletePageByBookmarkId(
-      kTestPageBookmarkId2, base::Bind(&OfflinePageModelTest::OnDeletePageDone,
-                                       AsWeakPtr()));
+  model()->DeletePageByOfflineId(
+      offline2,
+      base::Bind(&OfflinePageModelTest::OnDeletePageDone, AsWeakPtr()));
 
   ResetResults();
 
   PumpLoop();
 
-  EXPECT_EQ(last_deleted_bookmark_id(), kTestPageBookmarkId2);
+  EXPECT_EQ(last_deleted_offline_id(), offline2);
   EXPECT_EQ(DeletePageResult::SUCCESS, last_delete_result());
   EXPECT_EQ(0u, store->GetAllPages().size());
 }
 
 TEST_F(OfflinePageModelTest, DeletePageNotFound) {
-  model()->DeletePageByBookmarkId(
-      kTestPageBookmarkId1, base::Bind(&OfflinePageModelTest::OnDeletePageDone,
-                                       AsWeakPtr()));
+  model()->DeletePageByOfflineId(
+      1234LL, base::Bind(&OfflinePageModelTest::OnDeletePageDone, AsWeakPtr()));
   EXPECT_EQ(DeletePageResult::NOT_FOUND, last_delete_result());
 }
 
@@ -529,9 +539,9 @@ TEST_F(OfflinePageModelTest, DeletePageStoreFailureOnRemove) {
   // Try to delete this page.
   GetStore()->set_test_scenario(
       OfflinePageTestStore::TestScenario::REMOVE_FAILED);
-  model()->DeletePageByBookmarkId(
-      kTestPageBookmarkId1, base::Bind(&OfflinePageModelTest::OnDeletePageDone,
-                                       AsWeakPtr()));
+  model()->DeletePageByOfflineId(
+      last_save_offline_id(),
+      base::Bind(&OfflinePageModelTest::OnDeletePageDone, AsWeakPtr()));
   PumpLoop();
   EXPECT_EQ(DeletePageResult::STORE_FAILURE, last_delete_result());
 }
@@ -543,13 +553,13 @@ TEST_F(OfflinePageModelTest, DetectThatOfflineCopyIsMissing) {
   ResetResults();
 
   const OfflinePageItem* page =
-      model()->GetPageByBookmarkId(kTestPageBookmarkId1);
+      model()->GetPageByOfflineId(last_save_offline_id());
   // Delete the offline copy of the page and check the metadata.
   base::DeleteFile(page->file_path, false);
   model()->CheckForExternalFileDeletion();
   PumpLoop();
 
-  EXPECT_EQ(last_deleted_bookmark_id(), kTestPageBookmarkId1);
+  EXPECT_EQ(last_deleted_offline_id(), page->offline_id);
   EXPECT_EQ(0UL, model()->GetAllPages().size());
 }
 
@@ -560,14 +570,14 @@ TEST_F(OfflinePageModelTest, DetectThatOfflineCopyIsMissingAfterLoad) {
   ResetResults();
 
   const OfflinePageItem* page =
-      model()->GetPageByBookmarkId(kTestPageBookmarkId1);
+      model()->GetPageByOfflineId(last_save_offline_id());
   // Delete the offline copy of the page and check the metadata.
   base::DeleteFile(page->file_path, false);
   // Reseting the model should trigger the metadata consistency check as well.
   ResetModel();
   PumpLoop();
 
-  EXPECT_EQ(last_deleted_bookmark_id(), kTestPageBookmarkId1);
+  EXPECT_EQ(last_deleted_offline_id(), page->offline_id);
   EXPECT_EQ(0UL, model()->GetAllPages().size());
 }
 
@@ -580,6 +590,7 @@ TEST_F(OfflinePageModelTest, DeleteMultiplePages) {
   model()->SavePage(
       kTestUrl, kTestPageBookmarkId1, std::move(archiver),
       base::Bind(&OfflinePageModelTest::OnSavePageDone, AsWeakPtr()));
+  int64_t offline1 = last_save_offline_id();
   PumpLoop();
 
   scoped_ptr<OfflinePageTestArchiver> archiver2(BuildArchiver(
@@ -587,6 +598,7 @@ TEST_F(OfflinePageModelTest, DeleteMultiplePages) {
   model()->SavePage(
       kTestUrl2, kTestPageBookmarkId2, std::move(archiver2),
       base::Bind(&OfflinePageModelTest::OnSavePageDone, AsWeakPtr()));
+  int64_t offline2 = last_save_offline_id();
   PumpLoop();
 
   scoped_ptr<OfflinePageTestArchiver> archiver3(BuildArchiver(
@@ -600,12 +612,12 @@ TEST_F(OfflinePageModelTest, DeleteMultiplePages) {
 
   // Delete multiple pages.
   std::vector<int64_t> ids_to_delete;
-  ids_to_delete.push_back(kTestPageBookmarkId2);
-  ids_to_delete.push_back(kTestPageBookmarkId1);
+  ids_to_delete.push_back(offline2);
+  ids_to_delete.push_back(offline1);
   ids_to_delete.push_back(23434LL);  // Non-existent ID.
-  model()->DeletePagesByBookmarkId(
-      ids_to_delete, base::Bind(&OfflinePageModelTest::OnDeletePageDone,
-                                AsWeakPtr()));
+  model()->DeletePagesByOfflineId(
+      ids_to_delete,
+      base::Bind(&OfflinePageModelTest::OnDeletePageDone, AsWeakPtr()));
   PumpLoop();
 
   // Success is expected if at least one page is deleted successfully.
@@ -613,29 +625,31 @@ TEST_F(OfflinePageModelTest, DeleteMultiplePages) {
   EXPECT_EQ(1u, store->GetAllPages().size());
 }
 
-TEST_F(OfflinePageModelTest, GetPageByBookmarkId) {
+TEST_F(OfflinePageModelTest, GetPageByOfflineId) {
   SavePage(kTestUrl, kTestPageBookmarkId1);
+  int64_t offline1 = last_save_offline_id();
   SavePage(kTestUrl2, kTestPageBookmarkId2);
+  int64_t offline2 = last_save_offline_id();
 
-  const OfflinePageItem* page =
-      model()->GetPageByBookmarkId(kTestPageBookmarkId1);
+  const OfflinePageItem* page = model()->GetPageByOfflineId(offline1);
   ASSERT_TRUE(page);
   EXPECT_EQ(kTestUrl, page->url);
-  EXPECT_EQ(kTestPageBookmarkId1, page->bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1, page->client_id);
   EXPECT_EQ(kTestFileSize, page->file_size);
 
-  page = model()->GetPageByBookmarkId(kTestPageBookmarkId2);
+  page = model()->GetPageByOfflineId(offline2);
   ASSERT_TRUE(page);
   EXPECT_EQ(kTestUrl2, page->url);
-  EXPECT_EQ(kTestPageBookmarkId2, page->bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId2, page->client_id);
   EXPECT_EQ(kTestFileSize, page->file_size);
 
-  page = model()->GetPageByBookmarkId(-42);
+  page = model()->GetPageByOfflineId(-42);
   EXPECT_FALSE(page);
 }
 
 TEST_F(OfflinePageModelTest, GetPageByOfflineURL) {
   SavePage(kTestUrl, kTestPageBookmarkId1);
+  int64_t offline1 = last_save_offline_id();
 
   OfflinePageTestStore* store = GetStore();
   GURL offline_url = store->last_saved_page().GetOfflineURL();
@@ -643,16 +657,19 @@ TEST_F(OfflinePageModelTest, GetPageByOfflineURL) {
   SavePage(kTestUrl2, kTestPageBookmarkId2);
 
   GURL offline_url2 = store->last_saved_page().GetOfflineURL();
+  int64_t offline2 = last_save_offline_id();
 
   const OfflinePageItem* page = model()->GetPageByOfflineURL(offline_url2);
   EXPECT_TRUE(page);
   EXPECT_EQ(kTestUrl2, page->url);
-  EXPECT_EQ(kTestPageBookmarkId2, page->bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId2, page->client_id);
+  EXPECT_EQ(offline2, page->offline_id);
 
   page = model()->GetPageByOfflineURL(offline_url);
   EXPECT_TRUE(page);
   EXPECT_EQ(kTestUrl, page->url);
-  EXPECT_EQ(kTestPageBookmarkId1, page->bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId2, page->client_id);
+  EXPECT_EQ(offline1, page->offline_id);
 
   page = model()->GetPageByOfflineURL(GURL("http://foo"));
   EXPECT_FALSE(page);
@@ -665,12 +682,12 @@ TEST_F(OfflinePageModelTest, GetPageByOnlineURL) {
   const OfflinePageItem* page = model()->GetPageByOnlineURL(kTestUrl2);
   EXPECT_TRUE(page);
   EXPECT_EQ(kTestUrl2, page->url);
-  EXPECT_EQ(kTestPageBookmarkId2, page->bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId2, page->client_id);
 
   page = model()->GetPageByOnlineURL(kTestUrl);
   EXPECT_TRUE(page);
   EXPECT_EQ(kTestUrl, page->url);
-  EXPECT_EQ(kTestPageBookmarkId1, page->bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1, page->client_id);
 
   page = model()->GetPageByOnlineURL(GURL("http://foo"));
   EXPECT_FALSE(page);
@@ -682,15 +699,15 @@ TEST_F(OfflinePageModelTest, GetPagesToCleanUp) {
   base::Time now = base::Time::Now();
 
   SavePage(kTestUrl, kTestPageBookmarkId1);
-  GetStore()->UpdateLastAccessTime(kTestPageBookmarkId1,
+  GetStore()->UpdateLastAccessTime(last_save_offline_id(),
                                    now - base::TimeDelta::FromDays(40));
 
   SavePage(kTestUrl2, kTestPageBookmarkId2);
-  GetStore()->UpdateLastAccessTime(kTestPageBookmarkId2,
+  GetStore()->UpdateLastAccessTime(last_save_offline_id(),
                                    now - base::TimeDelta::FromDays(31));
 
   SavePage(kTestUrl3, kTestPageBookmarkId3);
-  GetStore()->UpdateLastAccessTime(kTestPageBookmarkId3,
+  GetStore()->UpdateLastAccessTime(last_save_offline_id(),
                                    now - base::TimeDelta::FromDays(29));
 
   ResetModel();
@@ -700,9 +717,9 @@ TEST_F(OfflinePageModelTest, GetPagesToCleanUp) {
   std::vector<OfflinePageItem> pages_to_clean_up = model()->GetPagesToCleanUp();
   EXPECT_EQ(2UL, pages_to_clean_up.size());
   EXPECT_EQ(kTestUrl, pages_to_clean_up[0].url);
-  EXPECT_EQ(kTestPageBookmarkId1, pages_to_clean_up[0].bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId1, pages_to_clean_up[0].client_id);
   EXPECT_EQ(kTestUrl2, pages_to_clean_up[1].url);
-  EXPECT_EQ(kTestPageBookmarkId2, pages_to_clean_up[1].bookmark_id);
+  EXPECT_EQ(kTestPageBookmarkId2, pages_to_clean_up[1].client_id);
 }
 
 TEST_F(OfflinePageModelTest, CanSavePage) {
@@ -833,7 +850,8 @@ TEST_F(OfflinePageModelBookmarkChangeTest, ChangeBookmakeTitle) {
   EXPECT_EQ(0UL, model()->GetAllPages().size());
 
   // Adds an offline copy for the bookmark.
-  SavePage(kTestUrl, bookmark_node->id());
+  SavePage(kTestUrl, ClientId(BOOKMARK_NAMESPACE,
+                              base::Int64ToString(bookmark_node->id())));
   EXPECT_EQ(1UL, model()->GetAllPages().size());
 
   // Changes the bookmark title. The offline copy was not affected.
@@ -855,7 +873,8 @@ TEST_F(OfflinePageModelBookmarkChangeTest, ChangeBookmakeURL) {
   EXPECT_EQ(0UL, model()->GetAllPages().size());
 
   // Adds an offline copy for the bookmark.
-  SavePage(kTestUrl, bookmark_node->id());
+  SavePage(kTestUrl, ClientId(BOOKMARK_NAMESPACE,
+                              base::Int64ToString(bookmark_node->id())));
   EXPECT_EQ(1UL, model()->GetAllPages().size());
 
   // The offline copy should be removed upon the bookmark URL change.
@@ -877,7 +896,8 @@ TEST_F(OfflinePageModelBookmarkChangeTest, RemoveBookmark) {
 
   // Creates a bookmark with offline copy.
   bookmark_node = CreateBookmarkNode(kTestUrl);
-  SavePage(kTestUrl, bookmark_node->id());
+  SavePage(kTestUrl, ClientId(BOOKMARK_NAMESPACE,
+                              base::Int64ToString(bookmark_node->id())));
   EXPECT_EQ(1UL, model()->GetAllPages().size());
 
   // The offline copy should also be removed upon the bookmark removal.
@@ -903,7 +923,8 @@ TEST_F(OfflinePageModelBookmarkChangeTest, UndoBookmarkRemoval) {
 
   // Creates a bookmark with offline copy.
   bookmark_node = CreateBookmarkNode(kTestUrl);
-  SavePage(kTestUrl, bookmark_node->id());
+  SavePage(kTestUrl, ClientId(BOOKMARK_NAMESPACE,
+                              base::Int64ToString(bookmark_node->id())));
   EXPECT_EQ(1UL, model()->GetAllPages().size());
 
   // The offline copy should also be removed upon the bookmark removal.
@@ -917,4 +938,7 @@ TEST_F(OfflinePageModelBookmarkChangeTest, UndoBookmarkRemoval) {
   EXPECT_EQ(1UL, model()->GetAllPages().size());
 }
 
+// TODO(bburns): Add a unit test for returning multiple results for a

[dewittj, 2016/02/26 18:42:56]

Why is this a TODO and not just a test now?  There should be at least one test
for the GetOfflineIdsForClientId public API method.

[bburns, 2016/02/27 00:48:19]

Test added.

+// single client id.
+
 }  // namespace offline_pages