[Offline Pages] Refactoring deletion of pages with same urls.

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 78cf6ebe4db5ce36d2d501efcd0a42b6bd9d29b1..0841af8c2aaaa2612d9fb713c78d0ec648c6e234 100644
--- a/components/offline_pages/offline_page_model_impl.cc
+++ b/components/offline_pages/offline_page_model_impl.cc
@@ -740,8 +740,13 @@ void OfflinePageModelImpl::OnAddOfflinePageDone(
   }
   InformSavePageDone(callback, result, offline_page.client_id,
                      offline_page.offline_id);
-  DeletePendingArchiver(archiver);
+  if (result == SavePageResult::SUCCESS) {
+    DeleteExistingPagesWithSameURL(offline_page);
+  } else {
+    PostClearStorageIfNeededTask();
+  }
 
+  DeletePendingArchiver(archiver);
   FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelChanged(this));
 }
 
@@ -808,30 +813,32 @@ void OfflinePageModelImpl::InformSavePageDone(const SavePageCallback& callback,
   ReportSavePageResultHistogramAfterSave(client_id, result);
   archive_manager_->GetStorageStats(
       base::Bind(&ReportStorageHistogramsAfterSave));
+  callback.Run(result, offline_id);
+}
+
+void OfflinePageModelImpl::DeleteExistingPagesWithSameURL(
+    const OfflinePageItem& offline_page) {
   // Remove existing pages generated by the same policy and with same url.
   size_t pages_allowed =
-      policy_controller_->GetPolicy(client_id.name_space).pages_allowed_per_url;
-  if (result == SavePageResult::SUCCESS && pages_allowed != kUnlimitedPages) {
+      policy_controller_->GetPolicy(offline_page.client_id.name_space)
+          .pages_allowed_per_url;
+  if (pages_allowed != kUnlimitedPages) {

[fgorski, 2016/07/07 17:03:46]

nit:

if (pages_allowed == kUnlimited...)
  return;

     GetPagesByOnlineURL(
-        offline_pages_[offline_id].url,
+        offline_page.url,
         base::Bind(&OfflinePageModelImpl::OnPagesFoundWithSameURL,
-                   weak_ptr_factory_.GetWeakPtr(), client_id, offline_id,
+                   weak_ptr_factory_.GetWeakPtr(), offline_page,
                    pages_allowed));
-  } else {
-    PostClearStorageIfNeededTask();
   }
-  callback.Run(result, offline_id);
 }
 
 void OfflinePageModelImpl::OnPagesFoundWithSameURL(
-    const ClientId& client_id,
-    int64_t offline_id,
+    const OfflinePageItem& offline_page,
     size_t pages_allowed,
     const MultipleOfflinePageItemResult& items) {
   std::vector<OfflinePageItem> pages_to_delete;
   for (const auto& item : items) {
-    if (item.offline_id != offline_id &&
-        item.client_id.name_space == client_id.name_space) {
+    if (item.offline_id != offline_page.offline_id &&
+        item.client_id.name_space == offline_page.client_id.name_space) {
       pages_to_delete.push_back(item);
     }
   }