[Offline pages] Detecting offline pages missing bookmarks

components/offline_pages/offline_page_model.cc

Index: components/offline_pages/offline_page_model.cc
diff --git a/components/offline_pages/offline_page_model.cc b/components/offline_pages/offline_page_model.cc
index 242ade295c21c5ffb70575f1147708045042b194..6862deef6584c3702ef5063e7ce30a4c1f5badb0 100644
--- a/components/offline_pages/offline_page_model.cc
+++ b/components/offline_pages/offline_page_model.cc
@@ -17,6 +17,7 @@
 #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"
 
@@ -115,7 +116,9 @@ OfflinePageModel::OfflinePageModel(
     : 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(
@@ -128,6 +131,7 @@ OfflinePageModel::~OfflinePageModel() {
 }
 
 void OfflinePageModel::Start(bookmarks::BookmarkModel* model) {
+  bookmarks_model_ = model;
   scoped_observer_.Add(model);
 }
 
@@ -307,23 +311,26 @@ const OfflinePageItem* OfflinePageModel::GetPageByOnlineURL(
   return nullptr;
 }
 
-void OfflinePageModel::CheckForExternalFileDeletion() {
-  DCHECK(is_loaded_);
+void OfflinePageModel::CheckMetadataConsistency(bool finalize_deletion) {
+  if (metadata_check_in_progress_ || !is_loaded_ || !bookmarks_model_ ||
+      !bookmarks_model_->loaded()) {
+    return;
+  }
 
-  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));
+  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);
+    }
   }
 
-  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)));
+  DeletePagesByBookmarkId(
+      ids_of_pages_to_delete,
+      base::Bind(&OfflinePageModel::CheckForExternalFileDeletion,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 OfflinePageMetadataStore* OfflinePageModel::GetStoreForTesting() {
@@ -462,6 +469,16 @@ void OfflinePageModel::UndoPageDeletion(int64 bookmark_id) {
 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) {
@@ -518,13 +535,13 @@ void OfflinePageModel::OnLoadDone(
     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,
@@ -625,6 +642,24 @@ void OfflinePageModel::InformDeletePageDone(const DeletePageCallback& callback,
     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);
@@ -650,6 +685,8 @@ void OfflinePageModel::OnRemoveOfflinePagesMissingArchiveFileDone(
   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(