Remove OfflinePageMetadataStore

OfflinePageMetadataStore was only referenced in the unittest, now it has been
removed. A few methods/types were migrated to OfflinePageMetadataStoreSQL,
others were removed because the were no longer in use.

I removed some tests that only tested the removed methods. Other methods
that were used to test the database and were moved to the test file, like
GetOfflinePages and AddOfflinePage.

offline_page_metadata_store_unittest.cc will be renamed
offline_page_metadata_store_sql_unittest.cc in a follow-up.

Bug: 778813
Change-Id: I6315cd5fb43b8a56b84dcede8c388286dd96e39c
Reviewed-on: https://chromium-review.googlesource.com/949502Reviewed-by: Yafei Duan <romax@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541621}

components/offline_pages/core/BUILD.gn

@@ -56,7 +56,6 @@ static_library("core") {
     "offline_page_client_policy.h",
     "offline_page_item.cc",
     "offline_page_item.h",
-    "offline_page_metadata_store.h",
     "offline_page_metadata_store_sql.cc",
     "offline_page_metadata_store_sql.h",
     "offline_page_model.cc",

components/offline_pages/core/model/delete_page_task.h

@@ -10,7 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "components/offline_pages/core/offline_page_metadata_store.h"
+#include "components/offline_pages/core/offline_page_metadata_store_sql.h"
 #include "components/offline_pages/core/offline_page_model.h"
 #include "components/offline_pages/core/offline_page_types.h"
 #include "components/offline_pages/core/task.h"

components/offline_pages/core/model/offline_page_model_taskified.cc

@@ -30,7 +30,6 @@
 #include "components/offline_pages/core/model/persistent_pages_consistency_check_task.h"
 #include "components/offline_pages/core/model/temporary_pages_consistency_check_task.h"
 #include "components/offline_pages/core/offline_page_feature.h"
-#include "components/offline_pages/core/offline_page_metadata_store.h"
 #include "components/offline_pages/core/offline_page_metadata_store_sql.h"
 #include "components/offline_pages/core/offline_page_model.h"
 #include "components/offline_pages/core/offline_store_utils.h"

components/offline_pages/core/offline_page_metadata_store.h

-// 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.
-
-#ifndef COMPONENTS_OFFLINE_PAGES_CORE_OFFLINE_PAGE_METADATA_STORE_H_
-#define COMPONENTS_OFFLINE_PAGES_CORE_OFFLINE_PAGE_METADATA_STORE_H_
-
-#include <stdint.h>
-
-#include <vector>
-
-#include "base/callback.h"
-#include "components/offline_pages/core/offline_page_item.h"
-#include "components/offline_pages/core/offline_store_types.h"
-
-namespace offline_pages {
-
-typedef StoreUpdateResult<OfflinePageItem> OfflinePagesUpdateResult;
-
-// OfflinePageMetadataStore keeps metadata for the offline pages.
-// Ability to create multiple instances of the store as well as behavior of
-// asynchronous operations when the object is being destroyed, before such
-// operation finishes will depend on implementation. It should be possible to
-// issue multiple asynchronous operations in parallel.
-class OfflinePageMetadataStore {
- public:
-  // This enum is used in an UMA histogram. Hence the entries here shouldn't
-  // be deleted or re-ordered and new ones should be added to the end.
-  enum LoadStatus {
-    LOAD_SUCCEEDED,
-    STORE_INIT_FAILED,
-    STORE_LOAD_FAILED,
-    DATA_PARSING_FAILED,
-
-    // NOTE: always keep this entry at the end.
-    LOAD_STATUS_COUNT
-  };
-
-  typedef base::Callback<void(bool /* success */)> InitializeCallback;
-  typedef base::Callback<void(bool /* success */)> ResetCallback;
-  typedef base::Callback<void(std::vector<OfflinePageItem>)> LoadCallback;
-  typedef base::Callback<void(ItemActionStatus)> AddCallback;
-  typedef base::Callback<void(std::unique_ptr<OfflinePagesUpdateResult>)>
-      UpdateCallback;
-
-  virtual ~OfflinePageMetadataStore(){};
-
-  // Initializes the store. Should be called before any other methods.
-  virtual void Initialize(const InitializeCallback& callback) = 0;
-
-  // Get all of the offline pages from the store.
-  virtual void GetOfflinePages(const LoadCallback& callback) = 0;
-
-  // Asynchronously adds an offline page item metadata to the store.
-  virtual void AddOfflinePage(const OfflinePageItem& offline_page,
-                              const AddCallback& callback) = 0;
-
-  // Asynchronously updates a set of offline page items in the store.
-  virtual void UpdateOfflinePages(const std::vector<OfflinePageItem>& pages,
-                                  const UpdateCallback& callback) = 0;
-
-  // Asynchronously removes offline page metadata from the store.
-  // Result of the update is passed in callback.
-  virtual void RemoveOfflinePages(const std::vector<int64_t>& offline_ids,
-                                  const UpdateCallback& callback) = 0;
-
-  // Resets the store.
-  virtual void Reset(const ResetCallback& callback) = 0;
-
-  // Gets the store state.
-  virtual StoreState state() const = 0;
-};
-
-}  // namespace offline_pages
-
-#endif  // COMPONENTS_OFFLINE_PAGES_CORE_OFFLINE_PAGE_METADATA_STORE_H_

components/offline_pages/core/offline_page_metadata_store_sql.h

@@ -17,7 +17,8 @@
 #include "base/task_runner_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
-#include "components/offline_pages/core/offline_page_metadata_store.h"
+#include "components/offline_pages/core/offline_page_item.h"
+#include "components/offline_pages/core/offline_store_types.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -28,8 +29,10 @@ class Connection;
 }
 
 namespace offline_pages {
-// OfflinePageMetadataStoreSQL is an instance of OfflinePageMetadataStore
-// which is implemented using a SQLite database.
+typedef StoreUpdateResult<OfflinePageItem> OfflinePagesUpdateResult;
+
+// OfflinePageMetadataStoreSQL keeps metadata for the offline pages in an SQLite
+// database.
 //
 // This store has a history of schema updates in pretty much every release.
 // Original schema was delivered in M52. Since then, the following changes
@@ -61,8 +64,22 @@ namespace offline_pages {
 //   |UpgradeFrom54|.
 // * Upgrade should use |UpgradeWithQuery| and simply specify SQL command to
 //   move data from old table (prefixed by temp_) to the new one.
-class OfflinePageMetadataStoreSQL : public OfflinePageMetadataStore {
+class OfflinePageMetadataStoreSQL {
  public:
+  // This enum is used in an UMA histogram. Hence the entries here shouldn't
+  // be deleted or re-ordered and new ones should be added to the end.
+  enum LoadStatus {
+    LOAD_SUCCEEDED,
+    STORE_INIT_FAILED,
+    STORE_LOAD_FAILED,
+    DATA_PARSING_FAILED,
+
+    // NOTE: always keep this entry at the end.
+    LOAD_STATUS_COUNT
+  };
+
+  typedef base::RepeatingCallback<void(bool /* success */)> ResetCallback;
+
   // Definition of the callback that is going to run the core of the command in
   // the |Execute| method.
   template <typename T>
@@ -89,19 +106,7 @@ class OfflinePageMetadataStoreSQL : public OfflinePageMetadataStore {
       scoped_refptr<base::SequencedTaskRunner> background_task_runner,
       const base::FilePath& database_dir);
 
-  ~OfflinePageMetadataStoreSQL() override;
-
-  // Implementation methods.
-  void Initialize(const InitializeCallback& callback) override;
-  void GetOfflinePages(const LoadCallback& callback) override;
-  void AddOfflinePage(const OfflinePageItem& offline_page,
-                      const AddCallback& callback) override;
-  void UpdateOfflinePages(const std::vector<OfflinePageItem>& pages,
-                          const UpdateCallback& callback) override;
-  void RemoveOfflinePages(const std::vector<int64_t>& offline_ids,
-                          const UpdateCallback& callback) override;
-  void Reset(const ResetCallback& callback) override;
-  StoreState state() const override;
+  ~OfflinePageMetadataStoreSQL();
 
   // Executes a |run_callback| on SQL store on the blocking thread, and posts
   // its result back to calling thread through |result_callback|.
@@ -153,6 +158,7 @@ class OfflinePageMetadataStoreSQL : public OfflinePageMetadataStore {
 
   // Helper function used to force incorrect state for testing purposes.
   void SetStateForTesting(StoreState state, bool reset_db);
+  StoreState GetStateForTesting() const;
 
  private:
   // Initializes database and calls callback.