Support undoing offline page deletion

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 "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/sequenced_task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/offline_pages/offline_page_item.h"
 #include "components/offline_pages/offline_page_metadata_store.h"
 #include "url/gurl.h"
 
 using ArchiverResult = offline_pages::OfflinePageArchiver::ArchiverResult;
 using SavePageResult = offline_pages::OfflinePageModel::SavePageResult;
 
 namespace offline_pages {
 
 namespace {
 
 // 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);
+
 SavePageResult ToSavePageResult(ArchiverResult archiver_result) {
   SavePageResult result;
   switch (archiver_result) {
     case ArchiverResult::SUCCESSFULLY_CREATED:
       result = SavePageResult::SUCCESS;
       break;
     case ArchiverResult::ERROR_DEVICE_FULL:
       result = SavePageResult::DEVICE_FULL;
       break;
     case ArchiverResult::ERROR_CONTENT_UNAVAILABLE:
@@ skipping 75 matching lines @@
   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;
   offline_page_item.last_access_time = base::Time::Now();
   offline_page_item.access_count++;
   store_->AddOrUpdateOfflinePage(
       offline_page_item,
-      base::Bind(&OfflinePageModel::OnUpdateOfflinePageDone,
+      base::Bind(&OfflinePageModel::OnMarkPageAccesseDone,
                  weak_ptr_factory_.GetWeakPtr(), offline_page_item));
 }
 
+void OfflinePageModel::MarkPageForDeletion(
+    int64 bookmark_id,
+    const DeletePageCallback& callback) {
+  DCHECK(is_loaded_);
+  auto iter = offline_pages_.find(bookmark_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.flags = static_cast<OfflinePageItem::Flags>(

[fgorski, 2015/09/25 20:30:50]

create MarkForDeletion method in OfflinePageItem

[jianli, 2015/09/25 20:52:35]

Done.

+      static_cast<int>(offline_page_item.flags) |
+          OfflinePageItem::MARKED_FOR_DELETION);
+  store_->AddOrUpdateOfflinePage(
+      offline_page_item,
+      base::Bind(&OfflinePageModel::OnMarkPageForDeletionDone,
+                 weak_ptr_factory_.GetWeakPtr(), offline_page_item, callback));
+}
+
 void OfflinePageModel::DeletePageByBookmarkId(
     int64 bookmark_id,
     const DeletePageCallback& callback) {
   DCHECK(is_loaded_);
   std::vector<int64> bookmark_ids_to_delete;
   bookmark_ids_to_delete.push_back(bookmark_id);
   DeletePagesByBookmarkId(bookmark_ids_to_delete, callback);
 }
 
 void OfflinePageModel::DeletePagesByBookmarkId(
@@ skipping 21 matching lines @@
       base::Bind(&OfflinePageModel::OnDeleteArchiveFilesDone,
                  weak_ptr_factory_.GetWeakPtr(),
                  bookmark_ids,
                  callback,
                  base::Owned(success)));
 }
 
 const std::vector<OfflinePageItem> OfflinePageModel::GetAllPages() const {
   DCHECK(is_loaded_);
   std::vector<OfflinePageItem> offline_pages;
-  for (const auto& id_page_pair : offline_pages_)
+  for (const auto& id_page_pair : offline_pages_) {
+    if (id_page_pair.second.IsMarkedForDeletion())
+      continue;
     offline_pages.push_back(id_page_pair.second);
+  }
   return 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 (now - id_page_pair.second.creation_time > kPageCleanUpThreshold)
       offline_pages.push_back(id_page_pair.second);
@@ skipping 65 matching lines @@
     result = SavePageResult::SUCCESS;
     UMA_HISTOGRAM_MEMORY_KB(
         "OfflinePages.PageSize", offline_page.file_size / 1024);
   } else {
     result = SavePageResult::STORE_FAILURE;
   }
   InformSavePageDone(callback, result);
   DeletePendingArchiver(archiver);
 }
 
-void OfflinePageModel::OnUpdateOfflinePageDone(
+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.bookmark_id] = offline_page_item;
 }
 
+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.bookmark_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);
+}
+
+void OfflinePageModel::OnUndoOfflinePageDone(
+    const OfflinePageItem& offline_page, int64 old_bookmark_id, bool success) {
+  if (!success)
+    return;
+  offline_pages_.erase(old_bookmark_id);
+  offline_pages_[offline_page.bookmark_id] = offline_page;
+  // No need to report UMA for the undo operation.

[fgorski, 2015/09/25 20:30:50]

Why?

[jianli, 2015/09/25 20:52:35]

I meant not do report the UMA like in OnAddOfflinePageDone. But probably we want
to report something else. Removed this comment.

+}
+
+void OfflinePageModel::FinalizePageDeletion() {
+  std::vector<int64> bookmark_ids_pending_deletion;
+  for (const auto& id_page_pair : offline_pages_) {
+    if (!id_page_pair.second.IsMarkedForDeletion())
+      continue;
+    bookmark_ids_pending_deletion.push_back(id_page_pair.second.bookmark_id);
+  }
+  DeletePagesByBookmarkId(bookmark_ids_pending_deletion, DeletePageCallback());
+}
+
 void OfflinePageModel::BookmarkModelChanged() {
 }
 
 void OfflinePageModel::BookmarkNodeRemoved(
     bookmarks::BookmarkModel* model,
     const bookmarks::BookmarkNode* parent,
     int old_index,
     const bookmarks::BookmarkNode* node,
     const std::set<GURL>& removed_urls) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(
-        base::Bind(&OfflinePageModel::DeletePageByBookmarkId,
+        base::Bind(&OfflinePageModel::MarkPageForDeletion,
                    weak_ptr_factory_.GetWeakPtr(),
                    node->id(),
                    base::Bind(&EmptyDeleteCallback)));
     return;
   }
-  DeletePageByBookmarkId(node->id(), base::Bind(&EmptyDeleteCallback));
+  MarkPageForDeletion(node->id(), base::Bind(&EmptyDeleteCallback));
+}
+
+void OfflinePageModel::BookmarkNodeRenumbered(bookmarks::BookmarkModel* model,
+                                              int64 old_bookmark_id,
+                                              int64 new_bookmark_id) {
+  // The old bookmark that is marked for deletion should still exist.
+  auto old_page_item_iter = offline_pages_.find(old_bookmark_id);
+  if (old_page_item_iter == offline_pages_.end() ||
+      !old_page_item_iter->second.IsMarkedForDeletion()) {
+    NOTREACHED();
+    return;
+  }
+
+  // The new bookmark should not exist in our metadata record.
+  if (offline_pages_.find(new_bookmark_id) != offline_pages_.end()) {
+    NOTREACHED();
+    return;
+  }
+
+  // Write the old metadata with the new bookmark ID to the store.
+  OfflinePageItem new_page_item = old_page_item_iter->second;
+  new_page_item.bookmark_id = new_bookmark_id;
+  new_page_item.ClearMarkForDeletion();
+  store_->AddOrUpdateOfflinePage(
+      new_page_item,
+      base::Bind(&OfflinePageModel::OnUndoOfflinePageDone,
+                 weak_ptr_factory_.GetWeakPtr(), new_page_item,
+                 old_bookmark_id));
 }
 
 void OfflinePageModel::OnLoadDone(
     bool success,
     const std::vector<OfflinePageItem>& offline_pages) {
   DCHECK(!is_loaded_);
   is_loaded_ = true;
 
   if (success) {
     for (const auto& offline_page : offline_pages)
       offline_pages_[offline_page.bookmark_id] = offline_page;
   }
 
   // Run all the delayed tasks.
   for (const auto& delayed_task : delayed_tasks_)
     delayed_task.Run();
   delayed_tasks_.clear();
 
+  // If there're left over pages that are marked for deletion, but not yet

[fgorski, 2015/09/25 20:30:50]

there are pages

[jianli, 2015/09/25 20:52:36]

Done.

+  // deleted and Chrome restarts. Do it now.

[fgorski, 2015/09/25 20:30:50]

and service is restarted, delete the pages now.

[jianli, 2015/09/25 20:52:35]

Done.

+  FinalizePageDeletion();
+
   FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelLoaded(this));
 }
 
 void OfflinePageModel::InformSavePageDone(const SavePageCallback& callback,
                                           SavePageResult result) {
   UMA_HISTOGRAM_ENUMERATION(
       "OfflinePages.SavePageResult",
       static_cast<int>(result),
       static_cast<int>(SavePageResult::RESULT_COUNT));
   callback.Run(result);
@@ skipping 45 matching lines @@
       (success || bookmark_ids.size() > 1) ? DeletePageResult::SUCCESS
                                            : DeletePageResult::STORE_FAILURE);
 }
 
 void OfflinePageModel::InformDeletePageDone(const DeletePageCallback& callback,
                                             DeletePageResult result) {
   UMA_HISTOGRAM_ENUMERATION(
       "OfflinePages.DeletePageResult",
       static_cast<int>(result),
       static_cast<int>(DeletePageResult::RESULT_COUNT));
-  callback.Run(result);
+  if (!callback.is_null())
+    callback.Run(result);
 }
 
 }  // namespace offline_pages