[Offline Pages] Removes two-step expiration related.

components/offline_pages/offline_page_storage_manager.cc

Index: components/offline_pages/offline_page_storage_manager.cc
diff --git a/components/offline_pages/offline_page_storage_manager.cc b/components/offline_pages/offline_page_storage_manager.cc
index cc3fd4cd5b7057656144556701aa16e924a98baf..f16f704df9fa6e94ec2bbb6eb7c07ff954d21921 100644
--- a/components/offline_pages/offline_page_storage_manager.cc
+++ b/components/offline_pages/offline_page_storage_manager.cc
@@ -64,66 +64,43 @@ void OfflinePageStorageManager::OnGetAllPagesDoneForClearingPages(
     const ClearStorageCallback& callback,
     const ArchiveManager::StorageStats& stats,
     const MultipleOfflinePageItemResult& pages) {
-  std::vector<int64_t> page_ids_to_expire;
-  std::vector<int64_t> page_ids_to_remove;
-  GetPageIdsToClear(pages, stats, &page_ids_to_expire, &page_ids_to_remove);
-  model_->ExpirePages(
-      page_ids_to_expire, clear_time_,
-      base::Bind(&OfflinePageStorageManager::OnPagesExpired,
-                 weak_ptr_factory_.GetWeakPtr(), callback,
-                 page_ids_to_expire.size(), page_ids_to_remove));
-}
-
-void OfflinePageStorageManager::OnPagesExpired(
-    const ClearStorageCallback& callback,
-    size_t pages_expired,
-    const std::vector<int64_t>& page_ids_to_remove,
-    bool expiration_succeeded) {
-  // We want to delete the outdated page records regardless the expiration
-  // succeeded or not.
+  std::vector<int64_t> page_ids_to_clear;
+  GetPageIdsToClear(pages, stats, &page_ids_to_clear);
   model_->DeletePagesByOfflineId(
-      page_ids_to_remove,
-      base::Bind(&OfflinePageStorageManager::OnOutdatedPagesCleared,
-                 weak_ptr_factory_.GetWeakPtr(), callback, pages_expired,
-                 expiration_succeeded));
+      page_ids_to_clear,
+      base::Bind(&OfflinePageStorageManager::OnExpiredPagesCleared,
+                 weak_ptr_factory_.GetWeakPtr(), callback,
+                 page_ids_to_clear.size()));
 }
 
-void OfflinePageStorageManager::OnOutdatedPagesCleared(
+void OfflinePageStorageManager::OnExpiredPagesCleared(
     const ClearStorageCallback& callback,
     size_t pages_cleared,
-    bool expiration_succeeded,
     DeletePageResult result) {
-  ClearStorageResult clear_result = ClearStorageResult::SUCCESS;
-  if (!expiration_succeeded) {
-    clear_result = ClearStorageResult::EXPIRE_FAILURE;
-    if (result != DeletePageResult::SUCCESS)
-      clear_result = ClearStorageResult::EXPIRE_AND_DELETE_FAILURES;
-  } else if (result != DeletePageResult::SUCCESS) {
-    clear_result = ClearStorageResult::DELETE_FAILURE;
-  }
   last_clear_time_ = clear_time_;
+  ClearStorageResult clear_result = ClearStorageResult::SUCCESS;
+  if (result != DeletePageResult::SUCCESS)
+    clear_result = ClearStorageResult::CLEAR_FAILURE;
   callback.Run(pages_cleared, clear_result);
 }
 
 void OfflinePageStorageManager::GetPageIdsToClear(
     const MultipleOfflinePageItemResult& pages,
     const ArchiveManager::StorageStats& stats,
-    std::vector<int64_t>* page_ids_to_expire,
-    std::vector<int64_t>* page_ids_to_remove) {
+    std::vector<int64_t>* page_ids_to_clear) {
   // TODO(romax): See how persistent should be considered here.
   // Creating a map from namespace to a vector of page items.
   // Sort each vector based on last accessed time and all pages after index
-  // min{size(), page_limit} should be expired. And then start iterating
-  // backwards to expire pages.
+  // min{size(), page_limit} should be deleted.
   std::map<std::string, std::vector<OfflinePageItem>> pages_map;
   std::vector<OfflinePageItem> kept_pages;
   int64_t kept_pages_size = 0;
 
   for (const auto& page : pages) {
-    if (!page.IsExpired()) {
+    if (!IsExpired(page)) {
       pages_map[page.client_id.name_space].push_back(page);
-    } else if (clear_time_ - page.expiration_time >= kRemovePageItemInterval) {
-      page_ids_to_remove->push_back(page.offline_id);
+    } else {
+      page_ids_to_clear->push_back(page.offline_id);
     }
   }
 
@@ -143,14 +120,14 @@ void OfflinePageStorageManager::GetPageIdsToClear(
     size_t pos = 0;
     while (pos < page_list_size &&
            (policy.page_limit == kUnlimitedPages || pos < policy.page_limit) &&
-           !ShouldBeExpired(page_list.at(pos))) {
+           !IsExpired(page_list.at(pos))) {
       kept_pages_size += page_list.at(pos).file_size;
       kept_pages.push_back(page_list.at(pos));
       pos++;
     }
 
     for (; pos < page_list_size; pos++)
-      page_ids_to_expire->push_back(page_list.at(pos).offline_id);
+      page_ids_to_clear->push_back(page_list.at(pos).offline_id);
   }
 
   // If we're still over the clear threshold, we're going to clear remaining
@@ -170,7 +147,7 @@ void OfflinePageStorageManager::GetPageIdsToClear(
     size_t pos = 0;
     while (pos < kept_pages_size && space_to_release > 0) {
       space_to_release -= kept_pages.at(pos).file_size;
-      page_ids_to_expire->push_back(kept_pages.at(pos).offline_id);
+      page_ids_to_clear->push_back(kept_pages.at(pos).offline_id);
       pos++;
     }
   }
@@ -199,8 +176,7 @@ OfflinePageStorageManager::ShouldClearPages(
   return ClearMode::NOT_NEEDED;
 }
 
-bool OfflinePageStorageManager::ShouldBeExpired(
-    const OfflinePageItem& page) const {
+bool OfflinePageStorageManager::IsExpired(const OfflinePageItem& page) const {

[jianli, 2016/11/18 00:15:48]

Do we really need this?

[romax, 2016/11/18 20:50:49]

Yes it's still a helper function to return if the page is expired. WRT 'expire'
I have a comment above.

   const LifetimePolicy& policy =
       policy_controller_->GetPolicy(page.client_id.name_space).lifetime_policy;
   return policy.lifetime_type == LifetimeType::TEMPORARY &&