[Offline pages] Detecting missing offline copy

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 17e499bde753056e803db37f02ff624147ada661..34c2636d82573b01bfdf338ad5191f995617a46e 100644
--- a/components/offline_pages/offline_page_model.cc
+++ b/components/offline_pages/offline_page_model.cc
@@ -5,6 +5,7 @@
 #include "components/offline_pages/offline_page_model.h"
 
 #include <algorithm>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
@@ -67,6 +68,17 @@ void DeleteArchiveFiles(const std::vector<base::FilePath>& paths_to_delete,
 void EmptyDeleteCallback(OfflinePageModel::DeletePageResult /* result */) {
 }
 
+void ListPagesMissingArchiveFile(

[jianli, 2015/10/14 22:30:23]

FindMissingArchiveFiles?

[fgorski, 2015/10/15 19:12:29]

No, we are not looking for missing archive files, we are looking for pages (that
are) missing archive files, hence "list pages missing archive files". 

+    const std::vector<std::pair<int64, base::FilePath>>& id_path_pairs,
+    std::vector<int64>* pages_missing_archive_file) {
+  DCHECK(pages_missing_archive_file);
+
+  for (const auto& id_path : id_path_pairs) {
+    if (!base::PathExists(id_path.second))

[jianli, 2015/10/14 22:30:23]

I think we should also call base::DirectoryExists to find out if it is not for a
directory since it is likely that the same name could be used for a directory by
the external program.

[fgorski, 2015/10/15 19:12:29]

Done.

+      pages_missing_archive_file->push_back(id_path.first);
+  }
+}
+
 }  // namespace
 
 OfflinePageModel::OfflinePageModel(
@@ -203,6 +215,24 @@ const OfflinePageItem* OfflinePageModel::GetPageByOfflineURL(
   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));
+  }
+
+  std::vector<int64>* pages_missing_archive_file = new std::vector<int64>();
+  task_runner_->PostTaskAndReply(
+      FROM_HERE, base::Bind(&ListPagesMissingArchiveFile, id_path_pairs,
+                            pages_missing_archive_file),
+      base::Bind(&OfflinePageModel::OnListPagesMissingArchiveFileDone,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 base::Owned(pages_missing_archive_file)));
+}
+
 OfflinePageMetadataStore* OfflinePageModel::GetStoreForTesting() {
   return store_.get();
 }
@@ -301,6 +331,8 @@ void OfflinePageModel::OnLoadDone(
   delayed_tasks_.clear();
 
   FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelLoaded(this));
+
+  CheckForExternalFileDeletion();
 }
 
 void OfflinePageModel::InformSavePageDone(const SavePageCallback& callback,
@@ -357,6 +389,10 @@ void OfflinePageModel::OnRemoveOfflinePagesDone(
       callback,
       (success || bookmark_ids.size() > 1) ? DeletePageResult::SUCCESS
                                            : DeletePageResult::STORE_FAILURE);
+
+  for (int64 bookmark_id : bookmark_ids) {
+    FOR_EACH_OBSERVER(Observer, observers_, OfflinePageDeleted(bookmark_id));

[jianli, 2015/10/15 00:24:36]

Per the discussion with Ian, I realized that OnRemoveOfflinePagesDone could be
triggered from FinalizePageDeletion in my patch. I don't think we want to
trigger the delete event for the internal cleanup. We can work offline to see
how to resolve this.

[fgorski, 2015/10/15 19:12:29]

OK, let's discuss this.

+  }
 }
 
 void OfflinePageModel::InformDeletePageDone(const DeletePageCallback& callback,
@@ -368,4 +404,17 @@ void OfflinePageModel::InformDeletePageDone(const DeletePageCallback& callback,
   callback.Run(result);
 }
 
+void OfflinePageModel::OnListPagesMissingArchiveFileDone(
+    const std::vector<int64>* pages_missing_archive_file) {

[jianli, 2015/10/14 22:30:23]

pages_missing_archive_file => bookmark_ids_for_missing_archive_file?

[fgorski, 2015/10/15 19:12:29]

Done. Went with ids_of_pages_missing_archive_file

+  DCHECK(pages_missing_archive_file);
+  if (pages_missing_archive_file->empty())
+    return;
+
+  store_->RemoveOfflinePages(
+      *pages_missing_archive_file,
+      base::Bind(&OfflinePageModel::OnRemoveOfflinePagesDone,
+                 weak_ptr_factory_.GetWeakPtr(), *pages_missing_archive_file,
+                 base::Bind(&EmptyDeleteCallback)));
+}
+
 }  // namespace offline_pages