[Offline pages] Resetting offline page metadata store if initial load fails

components/offline_pages/offline_page_metadata_store_sql.cc

Index: components/offline_pages/offline_page_metadata_store_sql.cc
diff --git a/components/offline_pages/offline_page_metadata_store_sql.cc b/components/offline_pages/offline_page_metadata_store_sql.cc
index 7e22cd176398c354bfed36fb277db12db2c5cb2c..af45bb16c8735c728e0fc4c5741a3b6527b9b4fb 100644
--- a/components/offline_pages/offline_page_metadata_store_sql.cc
+++ b/components/offline_pages/offline_page_metadata_store_sql.cc
@@ -254,22 +254,6 @@ bool InitDatabase(sql::Connection* db, base::FilePath path) {
   return CreateSchema(db);
 }
 
-void NotifyLoadResult(scoped_refptr<base::SingleThreadTaskRunner> runner,
-                      const OfflinePageMetadataStore::LoadCallback& callback,
-                      OfflinePageMetadataStore::LoadStatus status,
-                      const std::vector<OfflinePageItem>& result) {
-  // TODO(bburns): Switch to SQL specific UMA metrics.
-  UMA_HISTOGRAM_ENUMERATION("OfflinePages.LoadStatus", status,
-                            OfflinePageMetadataStore::LOAD_STATUS_COUNT);
-  if (status == OfflinePageMetadataStore::LOAD_SUCCEEDED) {
-    UMA_HISTOGRAM_COUNTS("OfflinePages.SavedPageCount",
-                         static_cast<int32_t>(result.size()));
-  } else {
-    DVLOG(1) << "Offline pages database loading failed: " << status;
-  }
-  runner->PostTask(FROM_HERE, base::Bind(callback, status, result));
-}
-
 void OpenConnectionSync(sql::Connection* db,
                         scoped_refptr<base::SingleThreadTaskRunner> runner,
                         const base::FilePath& path,
@@ -298,7 +282,7 @@ bool GetPageByOfflineIdSync(sql::Connection* db,
 void GetOfflinePagesSync(
     sql::Connection* db,
     scoped_refptr<base::SingleThreadTaskRunner> runner,
-    const OfflinePageMetadataStore::LoadCallback& callback) {
+    const OfflinePageMetadataStore::GetOfflinePagesCallback& callback) {
   const char kSql[] = "SELECT * FROM " OFFLINE_PAGES_TABLE_NAME;
 
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
@@ -307,14 +291,12 @@ void GetOfflinePagesSync(
   while (statement.Step())
     result.push_back(MakeOfflinePageItem(&statement));
 
-  if (statement.Succeeded()) {
-    NotifyLoadResult(runner, callback, OfflinePageMetadataStore::LOAD_SUCCEEDED,
-                     result);
-  } else {
+  StoreState store_state =
+      statement.Succeeded() ? StoreState::LOADED : StoreState::FAILED_OPERATION;
+  if (!statement.Succeeded())
     result.clear();
-    NotifyLoadResult(runner, callback,
-                     OfflinePageMetadataStore::STORE_LOAD_FAILED, result);
-  }
+
+  runner->PostTask(FROM_HERE, base::Bind(callback, store_state, result));
 }
 
 void AddOfflinePageSync(sql::Connection* db,
@@ -437,12 +419,13 @@ void ResetSync(sql::Connection* db,
   bool success = db->Raze();
   db->Close();
   StoreState state;
-  if (success) {
-    state = InitDatabase(db, db_file_path) ? StoreState::LOADED
-                                           : StoreState::FAILED_LOADING;
-  } else {
+  if (!success)
     state = StoreState::FAILED_RESET;
-  }
+  else if (InitDatabase(db, db_file_path))
+    state = StoreState::LOADED;
+  else
+    state = StoreState::FAILED_RESET;
+
   runner->PostTask(FROM_HERE, base::Bind(callback, state));
 }
 
@@ -466,9 +449,11 @@ OfflinePageMetadataStoreSQL::~OfflinePageMetadataStoreSQL() {
 }
 
 void OfflinePageMetadataStoreSQL::GetOfflinePages(
-    const LoadCallback& callback) {
-  if (!CheckDb(base::Bind(
-          callback, STORE_INIT_FAILED, std::vector<OfflinePageItem>()))) {
+    const GetOfflinePagesCallback& callback) {
+  if (!CheckStoreState()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(callback, state(), std::vector<OfflinePageItem>()));
     return;
   }
 
@@ -480,8 +465,11 @@ void OfflinePageMetadataStoreSQL::GetOfflinePages(
 void OfflinePageMetadataStoreSQL::AddOfflinePage(
     const OfflinePageItem& offline_page,
     const AddCallback& callback) {
-  if (!CheckDb(base::Bind(callback, ItemActionStatus::STORE_ERROR)))
+  if (!CheckStoreState()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(callback, ItemActionStatus::STORE_ERROR));
     return;
+  }
 
   background_task_runner_->PostTask(
       FROM_HERE,
@@ -524,8 +512,11 @@ void OfflinePageMetadataStoreSQL::RemoveOfflinePages(
 }
 
 void OfflinePageMetadataStoreSQL::Reset(const ResetCallback& callback) {
-  if (!CheckDb(base::Bind(callback, false)))
+  if (!CheckStoreState()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  base::Bind(callback, false));
     return;
+  }
 
   background_task_runner_->PostTask(
       FROM_HERE,
@@ -535,17 +526,6 @@ void OfflinePageMetadataStoreSQL::Reset(const ResetCallback& callback) {
                             weak_ptr_factory_.GetWeakPtr(), callback)));
 }
 
-StoreState OfflinePageMetadataStoreSQL::state() const {
-  return state_;
-}
-
-void OfflinePageMetadataStoreSQL::SetStateForTesting(StoreState state,
-                                                     bool reset_db) {
-  state_ = state;
-  if (reset_db)
-    db_.reset(nullptr);
-}
-
 void OfflinePageMetadataStoreSQL::OpenConnection() {
   DCHECK(!db_);
   db_.reset(new sql::Connection());
@@ -565,9 +545,6 @@ void OfflinePageMetadataStoreSQL::OnOpenConnectionDone(StoreState state) {
   // Unfortunately we were not able to open DB connection.
   if (state != StoreState::LOADED)
     db_.reset();
-
-  // TODO(fgorski): This might be a place to start store recovery. Alternatively
-  // that can be attempted in the OfflinePageModel.
 }
 
 void OfflinePageMetadataStoreSQL::OnResetDone(const ResetCallback& callback,
@@ -576,12 +553,21 @@ void OfflinePageMetadataStoreSQL::OnResetDone(const ResetCallback& callback,
   callback.Run(state == StoreState::LOADED);
 }
 
-bool OfflinePageMetadataStoreSQL::CheckDb(const base::Closure& callback) {
-  if (!db_.get() || state_ != StoreState::LOADED) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback);
-    return false;
-  }
-  return true;
+bool OfflinePageMetadataStoreSQL::CheckStoreState() {
+  // If db is null, store cannot be loaded.
+  DCHECK(db_.get() || state_ != StoreState::LOADED);
+  return db_.get() && state_ == StoreState::LOADED;
+}
+
+StoreState OfflinePageMetadataStoreSQL::state() const {
+  return state_;
+}
+
+void OfflinePageMetadataStoreSQL::SetStateForTesting(StoreState state,
+                                                     bool reset_db) {
+  state_ = state;
+  if (reset_db)
+    db_.reset(nullptr);
 }
 
 }  // namespace offline_pages