[Offline pages] Splitting Add and Update methods in OPMStore

components/offline_pages/offline_page_model_impl.cc

Index: components/offline_pages/offline_page_model_impl.cc
diff --git a/components/offline_pages/offline_page_model_impl.cc b/components/offline_pages/offline_page_model_impl.cc
index a32ab8a03b9b85c2feb8e0c73d101ea5c35c0818..d26760877e7065c2da28ad58d230534e614c195c 100644
--- a/components/offline_pages/offline_page_model_impl.cc
+++ b/components/offline_pages/offline_page_model_impl.cc
@@ -332,10 +332,11 @@ void OfflinePageModelImpl::MarkPageAccessedWhenLoadDone(int64_t offline_id) {
   offline_page_item.last_access_time = GetCurrentTime();
   offline_page_item.access_count++;
 
-  store_->AddOrUpdateOfflinePage(
-      offline_page_item,
-      base::Bind(&OfflinePageModelImpl::OnMarkPageAccesseDone,
-                 weak_ptr_factory_.GetWeakPtr(), offline_page_item));
+  std::vector<OfflinePageItem> items;

[dewittj, 2016/09/14 00:24:11]

nit:
std::vector<OfflinePageItem> items = { offline_page_item };

[fgorski, 2016/09/14 03:54:22]

Done.

+  items.push_back(offline_page_item);
+  store_->UpdateOfflinePages(
+      items, base::Bind(&OfflinePageModelImpl::OnMarkPageAccesseDone,
+                        weak_ptr_factory_.GetWeakPtr(), offline_page_item));
 }
 
 void OfflinePageModelImpl::DeletePagesByOfflineId(
@@ -628,6 +629,7 @@ void OfflinePageModelImpl::ExpirePages(
     const base::Time& expiration_time,
     const base::Callback<void(bool)>& callback) {
   std::vector<base::FilePath> paths_to_delete;
+  std::vector<OfflinePageItem> items_to_update;
   for (int64_t offline_id : offline_ids) {
     auto iter = offline_pages_.find(offline_id);
     if (iter == offline_pages_.end())
@@ -637,11 +639,14 @@ void OfflinePageModelImpl::ExpirePages(
     paths_to_delete.push_back(offline_page.file_path);
     offline_page.expiration_time = expiration_time;
 
-    store_->AddOrUpdateOfflinePage(
-        offline_page, base::Bind(&OfflinePageModelImpl::OnExpirePageDone,
-                                 weak_ptr_factory_.GetWeakPtr(), offline_id,
-                                 expiration_time));
+    items_to_update.push_back(offline_page);
   }
+
+  store_->UpdateOfflinePages(items_to_update,
+                             base::Bind(&OfflinePageModelImpl::OnExpirePageDone,
+                                        weak_ptr_factory_.GetWeakPtr(),
+                                        items_to_update, expiration_time));
+
   if (paths_to_delete.empty()) {
     callback.Run(true);
     return;
@@ -649,17 +654,22 @@ void OfflinePageModelImpl::ExpirePages(
   archive_manager_->DeleteMultipleArchives(paths_to_delete, callback);
 }
 
-void OfflinePageModelImpl::OnExpirePageDone(int64_t offline_id,
-                                            const base::Time& expiration_time,
-                                            bool success) {
+void OfflinePageModelImpl::OnExpirePageDone(
+    const std::vector<OfflinePageItem>& expired_pages,
+    const base::Time& expiration_time,
+    bool success) {
   UMA_HISTOGRAM_BOOLEAN("OfflinePages.ExpirePage.StoreUpdateResult", success);
   if (!success)
     return;
-  const auto& iter = offline_pages_.find(offline_id);
-  if (iter != offline_pages_.end()) {
+  for (auto& expired_page : expired_pages) {
+    const auto& iter = offline_pages_.find(expired_page.offline_id);
+    if (iter == offline_pages_.end())
+      continue;
+
     iter->second.expiration_time = expiration_time;
     ClientId client_id = iter->second.client_id;
-    offline_event_logger_.RecordPageExpired(std::to_string(offline_id));
+    offline_event_logger_.RecordPageExpired(
+        std::to_string(expired_page.offline_id));
     base::HistogramBase* histogram = base::Histogram::FactoryGet(
         AddHistogramSuffix(client_id, "OfflinePages.ExpirePage.PageLifetime"),
         1, base::TimeDelta::FromDays(30).InMinutes(), 50,
@@ -725,8 +735,8 @@ void OfflinePageModelImpl::OnCreateArchiveDone(const GURL& requested_url,
   OfflinePageItem offline_page_item(url, offline_id, client_id, file_path,
                                     file_size, start_time);
   offline_page_item.title = title;
-  store_->AddOrUpdateOfflinePage(
-      offline_page_item, base::Bind(&OfflinePageModelImpl::OnAddOfflinePageDone,
+  store_->AddOfflinePage(offline_page_item,
+                         base::Bind(&OfflinePageModelImpl::OnAddOfflinePageDone,
                                     weak_ptr_factory_.GetWeakPtr(), archiver,
                                     callback, offline_page_item));
 }
@@ -735,9 +745,9 @@ void OfflinePageModelImpl::OnAddOfflinePageDone(
     OfflinePageArchiver* archiver,
     const SavePageCallback& callback,
     const OfflinePageItem& offline_page,
-    bool success) {
+    OfflinePageMetadataStore::ItemActionStatus status) {
   SavePageResult result;
-  if (success) {
+  if (status == OfflinePageMetadataStore::SUCCESS) {
     offline_pages_[offline_page.offline_id] = offline_page;
     result = SavePageResult::SUCCESS;
     ReportPageHistogramAfterSave(offline_page, GetCurrentTime());
@@ -745,7 +755,9 @@ void OfflinePageModelImpl::OnAddOfflinePageDone(
         offline_page.client_id.name_space, offline_page.url.spec(),
         std::to_string(offline_page.offline_id));
   } else {
-    result = SavePageResult::STORE_FAILURE;
+    result = status == OfflinePageMetadataStore::ALREADY_EXISTS

[dewittj, 2016/09/14 00:24:11]

nit: make an else if or a switch statement and remove ternary operator.

[fgorski, 2016/09/14 03:54:22]

Done.

+                 ? SavePageResult::ALREADY_EXISTS
+                 : SavePageResult::STORE_FAILURE;
   }
   InformSavePageDone(callback, result, offline_page.client_id,
                      offline_page.offline_id);