[Offline pages] Resetting offline page metadata store to handle LOAD/INIT failures

components/offline_pages/offline_page_model_impl.cc

Index: components/offline_pages/offline_page_model_impl.cc
diff --git a/components/offline_pages/offline_page_model_impl.cc b/components/offline_pages/offline_page_model_impl.cc
index 9df2141e42d0a78c368c778df1e86e674993211d..090b1931009b3e21ce681fcec32326c206c58542 100644
--- a/components/offline_pages/offline_page_model_impl.cc
+++ b/components/offline_pages/offline_page_model_impl.cc
@@ -647,7 +647,6 @@ void OfflinePageModelImpl::GetPagesByOnlineURLWhenLoadDone(
 }
 
 void OfflinePageModelImpl::CheckMetadataConsistency() {
-  DCHECK(is_loaded_);
   archive_manager_->GetAllArchives(
       base::Bind(&OfflinePageModelImpl::CheckMetadataConsistencyForArchivePaths,
                  weak_ptr_factory_.GetWeakPtr()));
@@ -816,6 +815,24 @@ void OfflinePageModelImpl::OnEnsureArchivesDirCreatedDone(
   UMA_HISTOGRAM_TIMES("OfflinePages.Model.ArchiveDirCreationTime",
                       base::TimeTicks::Now() - start_time);
 
+  store_->Initialize(base::Bind(&OfflinePageModelImpl::OnStoreInitialized,
+                                weak_ptr_factory_.GetWeakPtr(), start_time));
+}
+
+void OfflinePageModelImpl::OnStoreInitialized(const base::TimeTicks& start_time,
+                                              StoreState state) {
+  if (state == StoreState::FAILED_RESET) {
+    CompleteLoad();
+    return;
+  }
+
+  if (state == StoreState::FAILED_LOADING) {
+    store_->Reset(base::Bind(&OfflinePageModelImpl::OnStoreInitialized,
+                             weak_ptr_factory_.GetWeakPtr(), start_time));
+    return;
+  }
+
+  DCHECK_EQ(state, StoreState::LOADED);
   store_->GetOfflinePages(base::Bind(&OfflinePageModelImpl::OnLoadDone,
                                      weak_ptr_factory_.GetWeakPtr(),
                                      start_time));
@@ -823,31 +840,16 @@ void OfflinePageModelImpl::OnEnsureArchivesDirCreatedDone(
 
 void OfflinePageModelImpl::OnLoadDone(
     const base::TimeTicks& start_time,
-    OfflinePageMetadataStore::LoadStatus load_status,
     const std::vector<OfflinePageItem>& offline_pages) {
   DCHECK(!is_loaded_);
-  is_loaded_ = true;
-
-  // TODO(jianli): rebuild the store upon failure.
-
-  if (load_status == OfflinePageMetadataStore::LOAD_SUCCEEDED)
-    CacheLoadedData(offline_pages);
 
   UMA_HISTOGRAM_TIMES("OfflinePages.Model.ConstructionToLoadedEventTime",
                       base::TimeTicks::Now() - start_time);
 
-  // Create Storage Manager.
-  storage_manager_.reset(new OfflinePageStorageManager(
-      this, GetPolicyController(), archive_manager_.get()));
-
-  // Run all the delayed tasks.
-  for (const auto& delayed_task : delayed_tasks_)
-    delayed_task.Run();
-  delayed_tasks_.clear();
-
-  for (Observer& observer : observers_)
-    observer.OfflinePageModelLoaded(this);
+  CacheLoadedData(offline_pages);
+  CompleteLoad();
 
+  // Ensure necessary cleanup operations are started.
   CheckMetadataConsistency();
 
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(

[fgorski, 2016/11/14 21:59:47]

Move to CompleteLoad, as we want to clear storage even when the store load was
unsuccessful.

[fgorski, 2016/11/14 22:24:09]

Done.

@@ -858,6 +860,19 @@ void OfflinePageModelImpl::OnLoadDone(
       kStorageManagerStartingDelay);
 }
 
+void OfflinePageModelImpl::CompleteLoad() {
+  is_loaded_ = true;
+
+  // Inform observers the load is done.
+  for (Observer& observer : observers_)
+    observer.OfflinePageModelLoaded(this);
+
+  // Run all the delayed tasks.
+  for (const auto& delayed_task : delayed_tasks_)
+    delayed_task.Run();
+  delayed_tasks_.clear();
+}
+
 void OfflinePageModelImpl::InformSavePageDone(const SavePageCallback& callback,
                                               SavePageResult result,
                                               const ClientId& client_id,
@@ -1053,6 +1068,11 @@ void OfflinePageModelImpl::CacheLoadedData(
 
 void OfflinePageModelImpl::ClearStorageIfNeeded(

[fgorski, 2016/11/14 21:59:47]

make this method call itself.

[fgorski, 2016/11/14 22:24:10]

Done through Post...Task method instead.
Intention was to either run now or post delayed, but we already had a necessary
method for that.

     const ClearStorageCallback& callback) {
+  // Create Storage Manager if necessary.
+  if (!storage_manager_) {
+    storage_manager_.reset(new OfflinePageStorageManager(
+        this, GetPolicyController(), archive_manager_.get()));
+  }
   storage_manager_->ClearPagesIfNeeded(callback);
 }