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

components/offline_pages/offline_page_model.cc

Index: components/offline_pages/offline_page_model.cc
diff --git a/components/offline_pages/offline_page_model.cc b/components/offline_pages/offline_page_model.cc
index 18b656c0c929574fe500998b16e1a861d8f4e003..9f1196a787cda963b2c782e38f6fa28be532c960 100644
--- a/components/offline_pages/offline_page_model.cc
+++ b/components/offline_pages/offline_page_model.cc
@@ -18,6 +18,7 @@
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/offline_pages/offline_page_item.h"
+#include "components/offline_pages/proto/offline_pages.pb.h"
 #include "url/gurl.h"
 
 using ArchiverResult = offline_pages::OfflinePageArchiver::ArchiverResult;
@@ -149,7 +150,8 @@ void OfflinePageModel::RemoveObserver(Observer* observer) {
 }
 
 void OfflinePageModel::SavePage(const GURL& url,
-                                int64_t bookmark_id,
+                                int64_t offline_id,
+                                const ClientId& client_id,
                                 scoped_ptr<OfflinePageArchiver> archiver,
                                 const SavePageCallback& callback) {
   DCHECK(is_loaded_);
@@ -165,13 +167,14 @@ void OfflinePageModel::SavePage(const GURL& url,
   archiver->CreateArchive(archives_dir_,
                           base::Bind(&OfflinePageModel::OnCreateArchiveDone,
                                      weak_ptr_factory_.GetWeakPtr(), url,
-                                     bookmark_id, base::Time::Now(), callback));
+                                     offline_id, client_id, base::Time::Now(),
+                                     callback));
   pending_archivers_.push_back(std::move(archiver));
 }
 
-void OfflinePageModel::MarkPageAccessed(int64_t bookmark_id) {
+void OfflinePageModel::MarkPageAccessed(int64_t offline_id) {
   DCHECK(is_loaded_);
-  auto iter = offline_pages_.find(bookmark_id);
+  auto iter = offline_pages_.find(offline_id);
   if (iter == offline_pages_.end())
     return;
 
@@ -190,10 +193,10 @@ void OfflinePageModel::MarkPageAccessed(int64_t bookmark_id) {
                  weak_ptr_factory_.GetWeakPtr(), offline_page_item));
 }
 
-void OfflinePageModel::MarkPageForDeletion(int64_t bookmark_id,
+void OfflinePageModel::MarkPageForDeletion(int64_t offline_id,
                                            const DeletePageCallback& callback) {
   DCHECK(is_loaded_);
-  auto iter = offline_pages_.find(bookmark_id);
+  auto iter = offline_pages_.find(offline_id);
   if (iter == offline_pages_.end()) {
     InformDeletePageDone(callback, DeletePageResult::NOT_FOUND);
     return;
@@ -209,23 +212,23 @@ void OfflinePageModel::MarkPageForDeletion(int64_t bookmark_id,
                  weak_ptr_factory_.GetWeakPtr(), offline_page_item, callback));
 }
 
-void OfflinePageModel::DeletePageByBookmarkId(
-    int64_t bookmark_id,
+void OfflinePageModel::DeletePageByOfflineId(
+    int64_t offline_id,
     const DeletePageCallback& callback) {
   DCHECK(is_loaded_);
-  std::vector<int64_t> bookmark_ids_to_delete;
-  bookmark_ids_to_delete.push_back(bookmark_id);
-  DeletePagesByBookmarkId(bookmark_ids_to_delete, callback);
+  std::vector<int64_t> offline_ids_to_delete;
+  offline_ids_to_delete.push_back(offline_id);
+  DeletePagesByOfflineId(offline_ids_to_delete, callback);
 }
 
-void OfflinePageModel::DeletePagesByBookmarkId(
-    const std::vector<int64_t>& bookmark_ids,
+void OfflinePageModel::DeletePagesByOfflineId(
+    const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback) {
   DCHECK(is_loaded_);
 
   std::vector<base::FilePath> paths_to_delete;
-  for (const auto& bookmark_id : bookmark_ids) {
-    auto iter = offline_pages_.find(bookmark_id);
+  for (const auto& offline_id : offline_ids) {
+    auto iter = offline_pages_.find(offline_id);
     if (iter != offline_pages_.end()) {
       paths_to_delete.push_back(iter->second.file_path);
     }
@@ -242,7 +245,7 @@ void OfflinePageModel::DeletePagesByBookmarkId(
       base::Bind(&DeleteArchiveFiles, paths_to_delete, success),
       base::Bind(&OfflinePageModel::OnDeleteArchiveFilesDone,
                  weak_ptr_factory_.GetWeakPtr(),
-                 bookmark_ids,
+                 offline_ids,
                  callback,
                  base::Owned(success)));
 }
@@ -250,11 +253,11 @@ void OfflinePageModel::DeletePagesByBookmarkId(
 void OfflinePageModel::ClearAll(const base::Closure& callback) {
   DCHECK(is_loaded_);
 
-  std::vector<int64_t> bookmark_ids;
+  std::vector<int64_t> offline_ids;
   for (const auto& id_page_pair : offline_pages_)
-    bookmark_ids.push_back(id_page_pair.first);
-  DeletePagesByBookmarkId(
-      bookmark_ids,
+    offline_ids.push_back(id_page_pair.first);
+  DeletePagesByOfflineId(
+      offline_ids,
       base::Bind(&OfflinePageModel::OnRemoveAllFilesDoneForClearAll,
                  weak_ptr_factory_.GetWeakPtr(),
                  callback));
@@ -295,9 +298,9 @@ const std::vector<OfflinePageItem> OfflinePageModel::GetPagesToCleanUp() const {
   return offline_pages;
 }
 
-const OfflinePageItem* OfflinePageModel::GetPageByBookmarkId(
-    int64_t bookmark_id) const {
-  const auto iter = offline_pages_.find(bookmark_id);
+const OfflinePageItem* OfflinePageModel::GetPageByOfflineId(
+    int64_t offline_id) const {
+  const auto iter = offline_pages_.find(offline_id);
   return iter != offline_pages_.end() && !iter->second.IsMarkedForDeletion()
              ? &(iter->second)
              : nullptr;
@@ -350,7 +353,8 @@ OfflinePageMetadataStore* OfflinePageModel::GetStoreForTesting() {
 }
 
 void OfflinePageModel::OnCreateArchiveDone(const GURL& requested_url,
-                                           int64_t bookmark_id,
+                                           int64_t offline_id,
+                                           const ClientId& client_id,
                                            const base::Time& start_time,
                                            const SavePageCallback& callback,
                                            OfflinePageArchiver* archiver,
@@ -374,8 +378,8 @@ void OfflinePageModel::OnCreateArchiveDone(const GURL& requested_url,
     return;
   }
 
-  OfflinePageItem offline_page_item(url, bookmark_id, file_path, file_size,
-                                    start_time);
+  OfflinePageItem offline_page_item(url, offline_id, client_id, file_path,
+                                    file_size, start_time);
   store_->AddOrUpdateOfflinePage(
       offline_page_item,
       base::Bind(&OfflinePageModel::OnAddOfflinePageDone,
@@ -389,7 +393,7 @@ void OfflinePageModel::OnAddOfflinePageDone(OfflinePageArchiver* archiver,
                                             bool success) {
   SavePageResult result;
   if (success) {
-    offline_pages_[offline_page.bookmark_id] = offline_page;
+    offline_pages_[offline_page.offline_id] = offline_page;
     result = SavePageResult::SUCCESS;
     UMA_HISTOGRAM_TIMES(
         "OfflinePages.SavePageTime",
@@ -409,7 +413,7 @@ void OfflinePageModel::OnMarkPageAccesseDone(
     const OfflinePageItem& offline_page_item, bool success) {
   // Update the item in the cache only upon success.
   if (success)
-    offline_pages_[offline_page_item.bookmark_id] = offline_page_item;
+    offline_pages_[offline_page_item.offline_id] = offline_page_item;
 
   // No need to fire OfflinePageModelChanged event since updating access info
   // should not have any impact to the UI.
@@ -421,7 +425,7 @@ void OfflinePageModel::OnMarkPageForDeletionDone(
     bool success) {
   // Update the item in the cache only upon success.
   if (success)
-    offline_pages_[offline_page_item.bookmark_id] = offline_page_item;
+    offline_pages_[offline_page_item.offline_id] = offline_page_item;
 
   InformDeletePageDone(callback, success ? DeletePageResult::SUCCESS
                                          : DeletePageResult::STORE_FAILURE);
@@ -437,30 +441,30 @@ void OfflinePageModel::OnMarkPageForDeletionDone(
       kFinalDeletionDelay);
 
   FOR_EACH_OBSERVER(
-      Observer, observers_, OfflinePageDeleted(offline_page_item.bookmark_id));
+      Observer, observers_, OfflinePageDeleted(offline_page_item.offline_id));
 }
 
 void OfflinePageModel::OnUndoOfflinePageDone(
     const OfflinePageItem& offline_page, bool success) {
   if (!success)
     return;
-  offline_pages_[offline_page.bookmark_id] = offline_page;
+  offline_pages_[offline_page.offline_id] = offline_page;
 
   FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelChanged(this));
 }
 
 void OfflinePageModel::FinalizePageDeletion() {
-  std::vector<int64_t> bookmark_ids_pending_deletion;
+  std::vector<int64_t> offline_ids_pending_deletion;
   for (const auto& id_page_pair : offline_pages_) {
     if (!id_page_pair.second.IsMarkedForDeletion())
       continue;
-    bookmark_ids_pending_deletion.push_back(id_page_pair.second.bookmark_id);
+    offline_ids_pending_deletion.push_back(id_page_pair.second.offline_id);
   }
-  DeletePagesByBookmarkId(bookmark_ids_pending_deletion, DeletePageCallback());
+  DeletePagesByOfflineId(offline_ids_pending_deletion, DeletePageCallback());
 }
 
-void OfflinePageModel::UndoPageDeletion(int64_t bookmark_id) {
-  auto iter = offline_pages_.find(bookmark_id);
+void OfflinePageModel::UndoPageDeletion(int64_t offline_id) {
+  auto iter = offline_pages_.find(offline_id);
   if (iter == offline_pages_.end())
     return;
 
@@ -513,7 +517,7 @@ void OfflinePageModel::BookmarkNodeChanged(
   // the latter, we need to invalidate the offline copy.
   auto iter = offline_pages_.find(node->id());
   if (iter != offline_pages_.end() && iter->second.url != node->url())
-    DeletePageByBookmarkId(node->id(), DeletePageCallback());
+    DeletePageByOfflineId(node->id(), DeletePageCallback());
 }
 
 void OfflinePageModel::OnEnsureArchivesDirCreatedDone() {
@@ -561,7 +565,7 @@ void OfflinePageModel::DeletePendingArchiver(OfflinePageArchiver* archiver) {
 }
 
 void OfflinePageModel::OnDeleteArchiveFilesDone(
-    const std::vector<int64_t>& bookmark_ids,
+    const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback,
     const bool* success) {
   DCHECK(success);
@@ -572,21 +576,21 @@ void OfflinePageModel::OnDeleteArchiveFilesDone(
   }
 
   store_->RemoveOfflinePages(
-      bookmark_ids,
+      offline_ids,
       base::Bind(&OfflinePageModel::OnRemoveOfflinePagesDone,
-                 weak_ptr_factory_.GetWeakPtr(), bookmark_ids, callback));
+                 weak_ptr_factory_.GetWeakPtr(), offline_ids, callback));
 }
 
 void OfflinePageModel::OnRemoveOfflinePagesDone(
-    const std::vector<int64_t>& bookmark_ids,
+    const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback,
     bool success) {
   // Delete the offline page from the in memory cache regardless of success in
   // store.
   base::Time now = base::Time::Now();
   int64_t total_size = 0;
-  for (int64_t bookmark_id : bookmark_ids) {
-    auto iter = offline_pages_.find(bookmark_id);
+  for (int64_t offline_id : offline_ids) {
+    auto iter = offline_pages_.find(offline_id);
     if (iter == offline_pages_.end())
       continue;
     total_size += iter->second.file_size;
@@ -617,20 +621,20 @@ void OfflinePageModel::OnRemoveOfflinePagesDone(
     // yet informed the observer that the offline page is deleted.
     if (!iter->second.IsMarkedForDeletion()) {
       FOR_EACH_OBSERVER(Observer, observers_,
-                        OfflinePageDeleted(iter->second.bookmark_id));
+                        OfflinePageDeleted(iter->second.offline_id));
     }
     offline_pages_.erase(iter);
   }
-  if (bookmark_ids.size() > 1) {
+  if (offline_ids.size() > 1) {
     UMA_HISTOGRAM_COUNTS(
-        "OfflinePages.BatchDelete.Count", bookmark_ids.size());
+        "OfflinePages.BatchDelete.Count", offline_ids.size());
     UMA_HISTOGRAM_MEMORY_KB(
         "OfflinePages.BatchDelete.TotalPageSize", total_size / 1024);
   }
   // Deleting multiple pages always succeeds when it gets to this point.
   InformDeletePageDone(
       callback,
-      (success || bookmark_ids.size() > 1) ? DeletePageResult::SUCCESS
+      (success || offline_ids.size() > 1) ? DeletePageResult::SUCCESS
                                            : DeletePageResult::STORE_FAILURE);

[fgorski, 2016/02/12 21:43:41]

nit: align

[bburns, 2016/02/20 01:14:18]

Done.

 }
 
@@ -664,10 +668,10 @@ void OfflinePageModel::OnFindPagesMissingArchiveFile(
 }
 
 void OfflinePageModel::OnRemoveOfflinePagesMissingArchiveFileDone(
-    const std::vector<int64_t>& bookmark_ids,
+    const std::vector<int64_t>& offline_ids,
     OfflinePageModel::DeletePageResult /* result */) {
-  for (int64_t bookmark_id : bookmark_ids) {
-    FOR_EACH_OBSERVER(Observer, observers_, OfflinePageDeleted(bookmark_id));
+  for (int64_t offline_id : offline_ids) {
+    FOR_EACH_OBSERVER(Observer, observers_, OfflinePageDeleted(offline_id));
   }
 }
 
@@ -711,7 +715,7 @@ void OfflinePageModel::CacheLoadedData(
     const std::vector<OfflinePageItem>& offline_pages) {
   offline_pages_.clear();
   for (const auto& offline_page : offline_pages)
-    offline_pages_[offline_page.bookmark_id] = offline_page;
+    offline_pages_[offline_page.offline_id] = offline_page;
 }
 
 }  // namespace offline_pages