[Offline pages] Detecting missing offline copy

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/time/time.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
@@ skipping 42 matching lines @@
   DCHECK(success);
   for (const auto& file_path : paths_to_delete) {
     // Make sure delete happens on the left of || so that it is always executed.
     *success = base::DeleteFile(file_path, false) || *success;
   }
 }
 
 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(
     scoped_ptr<OfflinePageMetadataStore> store,
     const scoped_refptr<base::SequencedTaskRunner>& task_runner)
     : store_(store.Pass()),
       is_loaded_(false),
       task_runner_(task_runner),
       scoped_observer_(this),
       weak_ptr_factory_(this) {
@@ skipping 116 matching lines @@
     const GURL& offline_url) const {
   for (auto iter = offline_pages_.begin();
        iter != offline_pages_.end();
        ++iter) {
     if (iter->second.GetOfflineURL() == offline_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));
+  }
+
+  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();
 }
 
 void OfflinePageModel::OnCreateArchiveDone(const GURL& requested_url,
                                            int64 bookmark_id,
                                            const SavePageCallback& callback,
                                            OfflinePageArchiver* archiver,
                                            ArchiverResult archiver_result,
                                            const GURL& url,
@@ skipping 78 matching lines @@
     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();
 
   FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelLoaded(this));
+
+  CheckForExternalFileDeletion();
 }
 
 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 36 matching lines @@
                                 1,
                                 base::TimeDelta::FromDays(365).InMinutes(),
                                 100);
     offline_pages_.erase(iter);
   }
   // Deleting multiple pages always succeeds when it gets to this point.
   InformDeletePageDone(
       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,
                                             DeletePageResult result) {
   UMA_HISTOGRAM_ENUMERATION(
       "OfflinePages.DeletePageResult",
       static_cast<int>(result),
       static_cast<int>(DeletePageResult::RESULT_COUNT));
   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