[Offline pages] Splitting Add and Update methods in OPMStore

components/offline_pages/offline_page_model_impl.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/offline_page_model_impl.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
@@ skipping 314 matching lines @@
 
   // Make a copy of the cached item and update it. The cached item should only
   // be updated upon the successful store operation.
   OfflinePageItem offline_page_item = iter->second;
 
   ReportPageHistogramsAfterAccess(offline_page_item, GetCurrentTime());
 
   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(
     const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback) {
   RunWhenLoaded(base::Bind(&OfflinePageModelImpl::DoDeletePagesByOfflineId,
                            weak_ptr_factory_.GetWeakPtr(), offline_ids,
                            callback));
 }
 
@@ skipping 272 matching lines @@
   archive_manager_->GetAllArchives(
       base::Bind(&OfflinePageModelImpl::CheckMetadataConsistencyForArchivePaths,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void OfflinePageModelImpl::ExpirePages(
     const std::vector<int64_t>& offline_ids,
     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())
       continue;
 
     OfflinePageItem offline_page = iter->second;
     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;
   }
   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,
         base::HistogramBase::kUmaTargetedHistogramFlag);
     histogram->Add((expiration_time - iter->second.creation_time).InMinutes());
     histogram = base::Histogram::FactoryGet(
         AddHistogramSuffix(client_id,
                            "OfflinePages.ExpirePage.TimeSinceLastAccess"),
         1, base::TimeDelta::FromDays(30).InMinutes(), 50,
         base::HistogramBase::kUmaTargetedHistogramFlag);
@@ skipping 45 matching lines @@
 
   if (archiver_result != ArchiverResult::SUCCESSFULLY_CREATED) {
     SavePageResult result = ToSavePageResult(archiver_result);
     InformSavePageDone(callback, result, client_id, offline_id);
     DeletePendingArchiver(archiver);
     return;
   }
   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));
 }
 
 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());
     offline_event_logger_.RecordPageSaved(
         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);
   if (result == SavePageResult::SUCCESS) {
     DeleteExistingPagesWithSameURL(offline_page);
   } else {
     PostClearStorageIfNeededTask();
   }
 
   DeletePendingArchiver(archiver);
@@ skipping 322 matching lines @@
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task);
 }
 
 base::Time OfflinePageModelImpl::GetCurrentTime() const {
   return testing_clock_ ? testing_clock_->Now() : base::Time::Now();
 }
 
 }  // namespace offline_pages