[Offline pages] Removing undoing of deleting offline pages

components/offline_pages/offline_page_model.cc

 // Copyright 2015 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.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 25 matching lines @@
   STORE_RELOAD_FAILED,
 
   // NOTE: always keep this entry at the end.
   CLEAR_ALL_STATUS_COUNT
 };
 
 // Threshold for how old offline copy of a page should be before we offer to
 // delete it to free up space.
 const base::TimeDelta kPageCleanUpThreshold = base::TimeDelta::FromDays(30);
 
-// The delay for the final deletion to kick in after the page is marked for
-// deletion. The value set here is a bit longer that the duration of the
-// snackbar that offers undo.
-const base::TimeDelta kFinalDeletionDelay = base::TimeDelta::FromSeconds(6);
-
 // The maximum histogram size for the metrics that measure time between views of
 // a given page.
 const base::TimeDelta kMaxOpenedPageHistogramBucket =
     base::TimeDelta::FromDays(90);
 
 SavePageResult ToSavePageResult(ArchiverResult archiver_result) {
   SavePageResult result;
   switch (archiver_result) {
     case ArchiverResult::SUCCESSFULLY_CREATED:
       result = SavePageResult::SUCCESS;
@@ skipping 43 matching lines @@
   CHECK(base::CreateDirectory(archives_dir));
 }
 
 }  // namespace
 
 // static
 bool OfflinePageModel::CanSavePage(const GURL& url) {
   return url.SchemeIsHTTPOrHTTPS();
 }
 
-// static
-base::TimeDelta OfflinePageModel::GetFinalDeletionDelayForTesting() {
-  return kFinalDeletionDelay;
-}
-
 OfflinePageModel::OfflinePageModel(
     scoped_ptr<OfflinePageMetadataStore> store,
     const base::FilePath& archives_dir,
     const scoped_refptr<base::SequencedTaskRunner>& task_runner)
     : store_(std::move(store)),
       archives_dir_(archives_dir),
       is_loaded_(false),
       task_runner_(task_runner),
       weak_ptr_factory_(this) {
   task_runner_->PostTaskAndReply(
@@ skipping 38 matching lines @@
                  client_id, base::Time::Now(), callback));
   pending_archivers_.push_back(std::move(archiver));
 }
 
 void OfflinePageModel::MarkPageAccessed(int64_t offline_id) {
   DCHECK(is_loaded_);
   auto iter = offline_pages_.find(offline_id);
   if (iter == offline_pages_.end())
     return;
 
-  // MarkPageAccessed should not be called for a page that is being marked for
-  // deletion.
-  DCHECK(!iter->second.IsMarkedForDeletion());
-
   // 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;
 
   base::Time now = base::Time::Now();
   base::TimeDelta time_since_last_accessed =
       now - offline_page_item.last_access_time;
 
   // The last access time was set to same as creation time when the page was
   // created.
   if (offline_page_item.creation_time == offline_page_item.last_access_time) {
     UMA_HISTOGRAM_CUSTOM_COUNTS("OfflinePages.FirstOpenSinceCreated",
                                 time_since_last_accessed.InMinutes(), 1,
                                 kMaxOpenedPageHistogramBucket.InMinutes(), 50);
   } else {
     UMA_HISTOGRAM_CUSTOM_COUNTS("OfflinePages.OpenSinceLastOpen",
                                 time_since_last_accessed.InMinutes(), 1,
                                 kMaxOpenedPageHistogramBucket.InMinutes(), 50);
   }
 
   offline_page_item.last_access_time = now;
   offline_page_item.access_count++;
 
   store_->AddOrUpdateOfflinePage(
       offline_page_item,
       base::Bind(&OfflinePageModel::OnMarkPageAccesseDone,
                  weak_ptr_factory_.GetWeakPtr(), offline_page_item));
 }
 
-void OfflinePageModel::MarkPageForDeletion(int64_t offline_id,
-                                           const DeletePageCallback& callback) {
-  DCHECK(is_loaded_);
-  auto iter = offline_pages_.find(offline_id);
-  if (iter == offline_pages_.end()) {
-    InformDeletePageDone(callback, DeletePageResult::NOT_FOUND);
-    return;
-  }
-
-  // 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;
-  offline_page_item.MarkForDeletion();
-  store_->AddOrUpdateOfflinePage(
-      offline_page_item,
-      base::Bind(&OfflinePageModel::OnMarkPageForDeletionDone,
-                 weak_ptr_factory_.GetWeakPtr(), offline_page_item, callback));
-}
-
 void OfflinePageModel::DeletePagesByOfflineId(
     const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(
         base::Bind(&OfflinePageModel::DoDeletePagesByOfflineId,
                    weak_ptr_factory_.GetWeakPtr(), offline_ids, callback));
 
     return;
   }
@@ skipping 51 matching lines @@
   DoDeletePagesByURLPredicate(predicate, callback);
 }
 
 void OfflinePageModel::DoDeletePagesByURLPredicate(
     const base::Callback<bool(const GURL&)>& predicate,
     const DeletePageCallback& callback) {
   DCHECK(is_loaded_);
 
   std::vector<int64_t> offline_ids;
   for (const auto& id_page_pair : offline_pages_) {
-    if (!id_page_pair.second.IsMarkedForDeletion() &&
-        predicate.Run(id_page_pair.second.url)) {
+    if (predicate.Run(id_page_pair.second.url))
       offline_ids.push_back(id_page_pair.first);
-    }
   }
   DoDeletePagesByOfflineId(offline_ids, callback);
 }
 
 void OfflinePageModel::HasPages(const std::string& name_space,
                                 const HasPagesCallback& callback) {
   RunWhenLoaded(base::Bind(&OfflinePageModel::HasPagesAfterLoadDone,
                            weak_ptr_factory_.GetWeakPtr(), name_space,
                            callback));
 }
 
 void OfflinePageModel::HasPagesAfterLoadDone(
     const std::string& name_space,
     const HasPagesCallback& callback) const {
   callback.Run(MaybeHasPages(name_space));
 }
 
 bool OfflinePageModel::MaybeHasPages(const std::string& name_space) const {
   if (!is_loaded_)
     return false;
 
   for (const auto& id_page_pair : offline_pages_) {
-    if (!id_page_pair.second.IsMarkedForDeletion() &&
-        id_page_pair.second.client_id.name_space == name_space) {
+    if (id_page_pair.second.client_id.name_space == name_space)
       return true;
-    }
   }
 
   return false;
 }
 
 void OfflinePageModel::GetAllPages(const GetAllPagesCallback& callback) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(
         base::Bind(&OfflinePageModel::GetAllPagesAfterLoadDone,
                    weak_ptr_factory_.GetWeakPtr(), callback));
     return;
   }
 
   GetAllPagesAfterLoadDone(callback);
 }
 
 void OfflinePageModel::GetAllPagesAfterLoadDone(
     const GetAllPagesCallback& callback) {
   DCHECK(is_loaded_);
 
   std::vector<OfflinePageItem> offline_pages;
-  for (const auto& id_page_pair : offline_pages_) {
-    if (id_page_pair.second.IsMarkedForDeletion())
-      continue;
+  for (const auto& id_page_pair : offline_pages_)
     offline_pages.push_back(id_page_pair.second);
-  }
 
   callback.Run(offline_pages);
 }
 
 const std::vector<OfflinePageItem> OfflinePageModel::GetPagesToCleanUp() const {
   DCHECK(is_loaded_);
   std::vector<OfflinePageItem> offline_pages;
   base::Time now = base::Time::Now();
   for (const auto& id_page_pair : offline_pages_) {
-    if (!id_page_pair.second.IsMarkedForDeletion() &&
-        now - id_page_pair.second.last_access_time > kPageCleanUpThreshold) {
+    if (now - id_page_pair.second.last_access_time > kPageCleanUpThreshold)
       offline_pages.push_back(id_page_pair.second);
-    }
   }
   return offline_pages;
 }
 
+// TODO(fgorski): Remove include_deleted, as it no longer makes sense.
 const std::vector<int64_t> OfflinePageModel::GetOfflineIdsForClientId(
     const ClientId& client_id,
     bool include_deleted) const {
   DCHECK(is_loaded_);
   std::vector<int64_t> results;
 
   // We want only all pages, including those marked for deletion.
   // TODO(bburns): actually use an index rather than linear scan.
   for (const auto& id_page_pair : offline_pages_) {
-    if (id_page_pair.second.client_id == client_id) {
-      if (include_deleted || !id_page_pair.second.IsMarkedForDeletion()) {
-        results.push_back(id_page_pair.second.offline_id);
-      }
-    }
+    if (id_page_pair.second.client_id == client_id)
+      results.push_back(id_page_pair.second.offline_id);
   }
   return results;
 }
 
 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;
+  return iter != offline_pages_.end() ? &(iter->second) : nullptr;
 }
 
 const OfflinePageItem* OfflinePageModel::GetPageByOfflineURL(
     const GURL& offline_url) const {
-  for (auto iter = offline_pages_.begin();
-       iter != offline_pages_.end();
-       ++iter) {
-    if (iter->second.GetOfflineURL() == offline_url &&
-        !iter->second.IsMarkedForDeletion()) {
-      return &(iter->second);
-    }
+  for (const auto& id_page_pair : offline_pages_) {
+    if (id_page_pair.second.GetOfflineURL() == offline_url)
+      return &(id_page_pair.second);
   }
   return nullptr;
 }
 
 const OfflinePageItem* OfflinePageModel::GetPageByOnlineURL(
     const GURL& online_url) const {
-  for (auto iter = offline_pages_.begin(); iter != offline_pages_.end();
-       ++iter) {
-    if (iter->second.url == online_url && !iter->second.IsMarkedForDeletion())
-      return &(iter->second);
+  for (const auto& id_page_pair : offline_pages_) {
+    if (id_page_pair.second.url == online_url)
+      return &(id_page_pair.second);
   }
   return nullptr;
 }
 
 void OfflinePageModel::CheckForExternalFileDeletion() {
   DCHECK(is_loaded_);
 
   std::vector<std::pair<int64_t, base::FilePath>> id_path_pairs;
   for (const auto& id_page_pair : offline_pages_) {
     id_path_pairs.push_back(
         std::make_pair(id_page_pair.first, id_page_pair.second.file_path));
   }
 
   std::vector<int64_t>* ids_of_pages_missing_archive_file =
       new std::vector<int64_t>();
   task_runner_->PostTaskAndReply(
       FROM_HERE, base::Bind(&FindPagesMissingArchiveFile, id_path_pairs,
                             ids_of_pages_missing_archive_file),
       base::Bind(&OfflinePageModel::OnFindPagesMissingArchiveFile,
                  weak_ptr_factory_.GetWeakPtr(),
                  base::Owned(ids_of_pages_missing_archive_file)));
 }
 
 void OfflinePageModel::RecordStorageHistograms(int64_t total_space_bytes,
                                                int64_t free_space_bytes,
                                                bool reporting_after_delete) {
   // Total space taken by offline pages.
   int64_t total_page_size = 0;
   for (const auto& id_page_pair : offline_pages_) {
-    if (!id_page_pair.second.IsMarkedForDeletion())
-      continue;
     total_page_size += id_page_pair.second.file_size;
   }
 
   int total_page_size_mb = static_cast<int>(total_page_size / (1024 * 1024));
   UMA_HISTOGRAM_COUNTS_10000("OfflinePages.TotalPageSize", total_page_size_mb);
 
   // How much of the total space the offline pages take.
   int total_page_size_percentage =
       static_cast<int>(1.0 * total_page_size / total_space_bytes * 100);
   UMA_HISTOGRAM_PERCENTAGE("OfflinePages.TotalPageSizePercentage",
@@ skipping 75 matching lines @@
 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.offline_id] = offline_page_item;
 
   // No need to fire OfflinePageModelChanged event since updating access info
   // should not have any impact to the UI.
 }
 
-void OfflinePageModel::OnMarkPageForDeletionDone(
-    const OfflinePageItem& offline_page_item,
-    const DeletePageCallback& callback,
-    bool success) {
-  // Update the item in the cache only upon success.
-  if (success)
-    offline_pages_[offline_page_item.offline_id] = offline_page_item;
-
-  InformDeletePageDone(callback, success ? DeletePageResult::SUCCESS
-                                         : DeletePageResult::STORE_FAILURE);
-
-  if (!success)
-    return;
-
-  // Schedule to do the final deletion.
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&OfflinePageModel::FinalizePageDeletion,
-                 weak_ptr_factory_.GetWeakPtr()),
-      kFinalDeletionDelay);
-
-  FOR_EACH_OBSERVER(Observer, observers_,
-                    OfflinePageDeleted(offline_page_item.offline_id,
-                                       offline_page_item.client_id));
-}
-
-void OfflinePageModel::OnUndoOfflinePageDone(
-    const OfflinePageItem& offline_page, bool success) {
-  if (!success)
-    return;
-  offline_pages_[offline_page.offline_id] = offline_page;
-
-  FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelChanged(this));
-}
-
-void OfflinePageModel::FinalizePageDeletion() {
-  std::vector<int64_t> offline_ids_pending_deletion;
-  for (const auto& id_page_pair : offline_pages_) {
-    if (!id_page_pair.second.IsMarkedForDeletion())
-      continue;
-    offline_ids_pending_deletion.push_back(id_page_pair.second.offline_id);
-  }
-  DeletePagesByOfflineId(offline_ids_pending_deletion, DeletePageCallback());
-}
-
-void OfflinePageModel::UndoPageDeletion(int64_t offline_id) {
-  auto iter = offline_pages_.find(offline_id);
-  if (iter == offline_pages_.end())
-    return;
-
-  // 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;
-  if (!offline_page_item.IsMarkedForDeletion())
-    return;
-
-  // Clear the flag to bring it back.
-  offline_page_item.ClearMarkForDeletion();
-  store_->AddOrUpdateOfflinePage(
-      offline_page_item,
-      base::Bind(&OfflinePageModel::OnUndoOfflinePageDone,
-                 weak_ptr_factory_.GetWeakPtr(), offline_page_item));
-}
-
 void OfflinePageModel::OnEnsureArchivesDirCreatedDone() {
   store_->Load(base::Bind(&OfflinePageModel::OnLoadDone,
                           weak_ptr_factory_.GetWeakPtr()));
 }
 
 void OfflinePageModel::OnLoadDone(
     OfflinePageMetadataStore::LoadStatus load_status,
     const std::vector<OfflinePageItem>& offline_pages) {
   DCHECK(!is_loaded_);
   is_loaded_ = true;
 
   // TODO(jianli): rebuild the store upon failure.
 
   if (load_status == OfflinePageMetadataStore::LOAD_SUCCEEDED)
     CacheLoadedData(offline_pages);
 
   // Run all the delayed tasks.
   for (const auto& delayed_task : delayed_tasks_)
     delayed_task.Run();
   delayed_tasks_.clear();
 
-  // If there are pages that are marked for deletion, but not yet deleted and
-  // OfflinePageModel gets reloaded. Delete the pages now.
-  FinalizePageDeletion();
-
   FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelLoaded(this));
 
   CheckForExternalFileDeletion();
 }
 
 void OfflinePageModel::InformSavePageDone(const SavePageCallback& callback,
                                           SavePageResult result,
                                           int64_t offline_id) {
   UMA_HISTOGRAM_ENUMERATION(
       "OfflinePages.SavePageResult",
@@ skipping 53 matching lines @@
         "OfflinePages.DeletePage.LastOpenToCreated",
         (iter->second.last_access_time - iter->second.creation_time).
             InMinutes(),
         1,
         base::TimeDelta::FromDays(365).InMinutes(),
         100);
     UMA_HISTOGRAM_MEMORY_KB(
         "OfflinePages.DeletePage.PageSize", iter->second.file_size / 1024);
     UMA_HISTOGRAM_COUNTS(
         "OfflinePages.DeletePage.AccessCount", iter->second.access_count);
-    // If the page is not marked for deletion at this point, the model has not
-    // yet informed the observer that the offline page is deleted.
-    if (!iter->second.IsMarkedForDeletion()) {
-      FOR_EACH_OBSERVER(
-          Observer, observers_,
-          OfflinePageDeleted(iter->second.offline_id, iter->second.client_id));
-    }
+    FOR_EACH_OBSERVER(
+        Observer, observers_,
+        OfflinePageDeleted(iter->second.offline_id, iter->second.client_id));
     offline_pages_.erase(iter);
   }
   if (offline_ids.size() > 1) {
     UMA_HISTOGRAM_COUNTS("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 || offline_ids.size() > 1)
                                      ? DeletePageResult::SUCCESS
@@ skipping 90 matching lines @@
     const std::vector<OfflinePageItem>& offline_pages) {
   offline_pages_.clear();
   for (const auto& offline_page : offline_pages)
     offline_pages_[offline_page.offline_id] = offline_page;
 }
 
 int64_t OfflinePageModel::GenerateOfflineId() {
   return base::RandGenerator(std::numeric_limits<int64_t>::max()) + 1;
 }
 
-void OfflinePageModel::MarkPagesForDeletion(
-    const std::vector<int64_t>& offline_ids,
-    const DeletePageCallback& callback) {
-  if (!is_loaded_) {
-    for (size_t i = 0; i < offline_ids.size(); i++) {
-      delayed_tasks_.push_back(
-          base::Bind(&OfflinePageModel::MarkPageForDeletion,
-                     weak_ptr_factory_.GetWeakPtr(), offline_ids[i], callback));
-    }
-    return;
-  }
-  for (const auto& id : offline_ids) {
-    MarkPageForDeletion(id, callback);
-  }
-}
-
 void OfflinePageModel::RunWhenLoaded(const base::Closure& task) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(task);
     return;
   }
 
   task_runner_->PostTask(FROM_HERE, task);
 }
 
 }  // namespace offline_pages