[Offline pages] Detecting offline pages missing bookmarks

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"
 #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/bookmarks/browser/bookmark_utils.h"
 #include "components/offline_pages/offline_page_item.h"
 #include "url/gurl.h"
 
 using ArchiverResult = offline_pages::OfflinePageArchiver::ArchiverResult;
 using SavePageResult = offline_pages::OfflinePageModel::SavePageResult;
 
 namespace offline_pages {
 
 namespace {
 
@@ skipping 78 matching lines @@
   return url.SchemeIsHTTPOrHTTPS();
 }
 
 OfflinePageModel::OfflinePageModel(
     scoped_ptr<OfflinePageMetadataStore> store,
     const base::FilePath& archives_dir,
     const scoped_refptr<base::SequencedTaskRunner>& task_runner)
     : store_(store.Pass()),
       archives_dir_(archives_dir),
       is_loaded_(false),
+      metadata_check_in_progress_(false),
       task_runner_(task_runner),
+      bookmarks_model_(nullptr),
       scoped_observer_(this),
       weak_ptr_factory_(this) {
   task_runner_->PostTaskAndReply(
       FROM_HERE, base::Bind(EnsureArchivesDirCreated, archives_dir_),
       base::Bind(&OfflinePageModel::OnEnsureArchivesDirCreatedDone,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 OfflinePageModel::~OfflinePageModel() {
 }
 
 void OfflinePageModel::Start(bookmarks::BookmarkModel* model) {
+  bookmarks_model_ = model;
   scoped_observer_.Add(model);
 }
 
 void OfflinePageModel::Shutdown() {
   scoped_observer_.RemoveAll();
 }
 
 void OfflinePageModel::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
@@ skipping 159 matching lines @@
 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)
       return &(iter->second);
   }
   return nullptr;
 }
 
-void OfflinePageModel::CheckForExternalFileDeletion() {
-  DCHECK(is_loaded_);
-
-  std::vector<std::pair<int64, 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));
+void OfflinePageModel::CheckMetadataConsistency(bool finalize_deletion) {
+  if (metadata_check_in_progress_ || !is_loaded_ || !bookmarks_model_ ||
+      !bookmarks_model_->loaded()) {
+    return;
   }
 
-  std::vector<int64>* ids_of_pages_missing_archive_file =
-      new std::vector<int64>();
-  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)));
+  metadata_check_in_progress_ = true;
+
+  std::vector<int64> ids_of_pages_to_delete;
+  for (const auto& iter : offline_pages_) {
+    if (!bookmarks::GetBookmarkNodeByID(bookmarks_model_, iter.first) ||
+        (finalize_deletion && iter.second.IsMarkedForDeletion())) {
+      ids_of_pages_to_delete.push_back(iter.first);
+    }
+  }
+
+  DeletePagesByBookmarkId(
+      ids_of_pages_to_delete,
+      base::Bind(&OfflinePageModel::CheckForExternalFileDeletion,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 OfflinePageMetadataStore* OfflinePageModel::GetStoreForTesting() {
   return store_.get();
 }
 
 void OfflinePageModel::OnCreateArchiveDone(const GURL& requested_url,
                                            int64 bookmark_id,
                                            const base::Time& start_time,
                                            const SavePageCallback& callback,
@@ skipping 118 matching lines @@
   offline_page_item.ClearMarkForDeletion();
   store_->AddOrUpdateOfflinePage(
       offline_page_item,
       base::Bind(&OfflinePageModel::OnUndoOfflinePageDone,
                  weak_ptr_factory_.GetWeakPtr(), offline_page_item));
 }
 
 void OfflinePageModel::BookmarkModelChanged() {
 }
 
+void OfflinePageModel::BookmarkModelLoaded(bookmarks::BookmarkModel* model,
+                                           bool ids_reassigned) {
+  CheckMetadataConsistency(true);
+}
+
+void OfflinePageModel::BookmarkModelBeingDeleted(
+    bookmarks::BookmarkModel* model) {
+  bookmarks_model_ = nullptr;
+}
+
 void OfflinePageModel::BookmarkNodeAdded(bookmarks::BookmarkModel* model,
                                          const bookmarks::BookmarkNode* parent,
                                          int index) {
   const bookmarks::BookmarkNode* node = parent->GetChild(index);
   DCHECK(node);
   UndoPageDeletion(node->id());
 }
 
 void OfflinePageModel::BookmarkNodeRemoved(
     bookmarks::BookmarkModel* model,
@@ skipping 36 matching lines @@
   // 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();
+  // Fix problems with offline page metadata:
+  // * remove pages missing archive files
+  // * remove pages that don't have a matching bookmark node
+  // * remove pages pending finalization.
+  CheckMetadataConsistency(true);
 }
 
 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 80 matching lines @@
 void OfflinePageModel::InformDeletePageDone(const DeletePageCallback& callback,
                                             DeletePageResult result) {
   UMA_HISTOGRAM_ENUMERATION(
       "OfflinePages.DeletePageResult",
       static_cast<int>(result),
       static_cast<int>(DeletePageResult::RESULT_COUNT));
   if (!callback.is_null())
     callback.Run(result);
 }
 
+void OfflinePageModel::CheckForExternalFileDeletion(
+    DeletePageResult /* result */) {
+  std::vector<std::pair<int64, 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>* ids_of_pages_missing_archive_file =
+      new std::vector<int64>();
+  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::OnFindPagesMissingArchiveFile(
     const std::vector<int64>* ids_of_pages_missing_archive_file) {
   DCHECK(ids_of_pages_missing_archive_file);
   if (ids_of_pages_missing_archive_file->empty())
     return;
 
   DeletePageCallback done_callback(
       base::Bind(&OfflinePageModel::OnRemoveOfflinePagesMissingArchiveFileDone,
                  weak_ptr_factory_.GetWeakPtr(),
                  *ids_of_pages_missing_archive_file));
 
   store_->RemoveOfflinePages(
       *ids_of_pages_missing_archive_file,
       base::Bind(&OfflinePageModel::OnRemoveOfflinePagesDone,
                  weak_ptr_factory_.GetWeakPtr(),
                  *ids_of_pages_missing_archive_file,
                  done_callback));
 }
 
 void OfflinePageModel::OnRemoveOfflinePagesMissingArchiveFileDone(
     const std::vector<int64>& bookmark_ids,
     OfflinePageModel::DeletePageResult /* result */) {
   for (int64 bookmark_id : bookmark_ids) {
     FOR_EACH_OBSERVER(Observer, observers_, OfflinePageDeleted(bookmark_id));
   }
+  // Metadata check is concluded here.
+  metadata_check_in_progress_ = false;
 }
 
 void OfflinePageModel::OnRemoveAllFilesDoneForClearAll(
     const base::Closure& callback,
     DeletePageResult result) {
   store_->Reset(base::Bind(&OfflinePageModel::OnResetStoreDoneForClearAll,
                            weak_ptr_factory_.GetWeakPtr(),
                            callback));
 }
 
@@ skipping 26 matching lines @@
 }
 
 void OfflinePageModel::CacheLoadedData(
     const std::vector<OfflinePageItem>& offline_pages) {
   offline_pages_.clear();
   for (const auto& offline_page : offline_pages)
     offline_pages_[offline_page.bookmark_id] = offline_page;
 }
 
 }  // namespace offline_pages