initial add of SQL based storage

components/offline_pages/offline_page_metadata_store_impl_unittest.cc

Index: components/offline_pages/offline_page_metadata_store_impl_unittest.cc
diff --git a/components/offline_pages/offline_page_metadata_store_impl_unittest.cc b/components/offline_pages/offline_page_metadata_store_impl_unittest.cc
index e519f2e9c7c8363d8795c92e12b8d19b44a54842..57dca46b7fdb9c7ee8e802ee8487ffcaa1632615 100644
--- a/components/offline_pages/offline_page_metadata_store_impl_unittest.cc
+++ b/components/offline_pages/offline_page_metadata_store_impl_unittest.cc
@@ -16,8 +16,10 @@
 #include "base/thread_task_runner_handle.h"
 #include "components/leveldb_proto/proto_database_impl.h"
 #include "components/offline_pages/offline_page_item.h"
+#include "components/offline_pages/offline_page_metadata_store_sql.h"
 #include "components/offline_pages/offline_page_model.h"
 #include "components/offline_pages/proto/offline_pages.pb.h"
+#include "sql/connection.h"

[Scott Hess - ex-Googler, 2016/04/25 19:57:55]

Is this ever used in this CL?

[bburns, 2016/04/26 23:27:02]

good catch, removed.

[Scott Hess - ex-Googler, 2016/04/27 15:21:00]

Unwinding this another step - since the test does not rely directly on //sql,
can BUILD.gn remove the second dependency?  I _think_ that works IWYU-style,
where if you only use it indirectly through other dependencies, you don't need
it.

To be sure, it feels like you are probably going to eventually need this
dependency because you probably will need sql::Connection code to write tests
for certain edge cases.  I'd be super happy with those tests here, but until
then unnecessary dependencies should be avoided.

 #include "testing/gtest/include/gtest/gtest.h"
 
 using leveldb_proto::ProtoDatabaseImpl;
@@ -33,22 +35,44 @@ const base::FilePath::CharType kFilePath[] =
     FILE_PATH_LITERAL("/offline_pages/example_com.mhtml");
 int64_t kFileSize = 234567;
 
-}  // namespace
+class OfflinePageMetadataStoreFactory {
+ public:
+  virtual OfflinePageMetadataStore* BuildStore(const base::FilePath& file) = 0;
+};
 
-class OfflinePageMetadataStoreImplTest : public testing::Test {
+class OfflinePageMetadataStoreImplFactory
+    : public OfflinePageMetadataStoreFactory {
  public:
-  enum CalledCallback { NONE, LOAD, ADD, REMOVE, DESTROY };
-  enum Status { STATUS_NONE, STATUS_TRUE, STATUS_FALSE };
+  OfflinePageMetadataStore* BuildStore(const base::FilePath& file) override {
+    return new OfflinePageMetadataStoreImpl(base::ThreadTaskRunnerHandle::Get(),
+                                            file);
+  }
+};
+
+class OfflinePageMetadataStoreSQLFactory
+    : public OfflinePageMetadataStoreFactory {
+ public:
+  OfflinePageMetadataStore* BuildStore(const base::FilePath& file) override {
+    OfflinePageMetadataStoreSQL* store = new OfflinePageMetadataStoreSQL(
+        base::ThreadTaskRunnerHandle::Get(), file);
+    return store;
+  }
+};
 
-  OfflinePageMetadataStoreImplTest();
-  ~OfflinePageMetadataStoreImplTest() override;
+enum CalledCallback { NONE, LOAD, ADD, REMOVE, DESTROY };
+enum Status { STATUS_NONE, STATUS_TRUE, STATUS_FALSE };
+
+class OfflinePageMetadataStoreTestBase : public testing::Test {
+ public:
+  OfflinePageMetadataStoreTestBase();
+  ~OfflinePageMetadataStoreTestBase() override;
 
   void TearDown() override {
     // Wait for all the pieces of the store to delete itself properly.
     PumpLoop();
   }
 
-  scoped_ptr<OfflinePageMetadataStoreImpl> BuildStore();
+  virtual scoped_ptr<OfflinePageMetadataStore> BuildStore() = 0;
   void PumpLoop();
 
   void LoadCallback(OfflinePageMetadataStore::LoadStatus load_status,
@@ -57,11 +81,6 @@ class OfflinePageMetadataStoreImplTest : public testing::Test {
 
   void ClearResults();
 
-  void UpdateStoreEntries(
-      OfflinePageMetadataStoreImpl* store,
-      scoped_ptr<leveldb_proto::ProtoDatabase<OfflinePageEntry>::KeyEntryVector>
-          entries_to_save);
-
  protected:
   CalledCallback last_called_callback_;
   Status last_status_;
@@ -72,7 +91,7 @@ class OfflinePageMetadataStoreImplTest : public testing::Test {
   base::ThreadTaskRunnerHandle task_runner_handle_;
 };
 
-OfflinePageMetadataStoreImplTest::OfflinePageMetadataStoreImplTest()
+OfflinePageMetadataStoreTestBase::OfflinePageMetadataStoreTestBase()
     : last_called_callback_(NONE),
       last_status_(STATUS_NONE),
       task_runner_(new base::TestSimpleTaskRunner),
@@ -80,25 +99,13 @@ OfflinePageMetadataStoreImplTest::OfflinePageMetadataStoreImplTest()
   EXPECT_TRUE(temp_directory_.CreateUniqueTempDir());
 }
 
-OfflinePageMetadataStoreImplTest::~OfflinePageMetadataStoreImplTest() {
-}
+OfflinePageMetadataStoreTestBase::~OfflinePageMetadataStoreTestBase() {}
 
-void OfflinePageMetadataStoreImplTest::PumpLoop() {
+void OfflinePageMetadataStoreTestBase::PumpLoop() {
   task_runner_->RunUntilIdle();
 }
 
-scoped_ptr<OfflinePageMetadataStoreImpl>
-OfflinePageMetadataStoreImplTest::BuildStore() {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(
-      new OfflinePageMetadataStoreImpl(base::ThreadTaskRunnerHandle::Get(),
-                                       temp_directory_.path()));
-  store->Load(base::Bind(&OfflinePageMetadataStoreImplTest::LoadCallback,
-                         base::Unretained(this)));
-  PumpLoop();
-  return store;
-}
-
-void OfflinePageMetadataStoreImplTest::LoadCallback(
+void OfflinePageMetadataStoreTestBase::LoadCallback(
     OfflinePageMetadataStore::LoadStatus load_status,
     const std::vector<OfflinePageItem>& offline_pages) {
   last_called_callback_ = LOAD;
@@ -107,134 +114,148 @@ void OfflinePageMetadataStoreImplTest::LoadCallback(
   offline_pages_.swap(const_cast<std::vector<OfflinePageItem>&>(offline_pages));
 }
 
-void OfflinePageMetadataStoreImplTest::UpdateCallback(
+void OfflinePageMetadataStoreTestBase::UpdateCallback(
     CalledCallback called_callback,
     bool status) {
   last_called_callback_ = called_callback;
   last_status_ = status ? STATUS_TRUE : STATUS_FALSE;
 }
 
-void OfflinePageMetadataStoreImplTest::ClearResults() {
+void OfflinePageMetadataStoreTestBase::ClearResults() {
   last_called_callback_ = NONE;
   last_status_ = STATUS_NONE;
   offline_pages_.clear();
 }
 
-void OfflinePageMetadataStoreImplTest::UpdateStoreEntries(
-    OfflinePageMetadataStoreImpl* store,
-    scoped_ptr<leveldb_proto::ProtoDatabase<OfflinePageEntry>::KeyEntryVector>
-        entries_to_save) {
-  scoped_ptr<std::vector<std::string>> keys_to_remove(
-      new std::vector<std::string>());
-  store->UpdateEntries(
-      std::move(entries_to_save), std::move(keys_to_remove),
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
-                 base::Unretained(this), ADD));
+template <typename T>
+class OfflinePageMetadataStoreTest : public OfflinePageMetadataStoreTestBase {
+ public:
+  scoped_ptr<OfflinePageMetadataStore> BuildStore();
+
+ protected:
+  T factory_;
+};
+
+template <typename T>
+scoped_ptr<OfflinePageMetadataStore>
+OfflinePageMetadataStoreTest<T>::BuildStore() {
+  scoped_ptr<OfflinePageMetadataStore> store(
+      factory_.BuildStore(temp_directory_.path()));
+  store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback,
+                         base::Unretained(this)));
+  PumpLoop();
+  return store;
 }
 
+typedef testing::Types<OfflinePageMetadataStoreImplFactory,
+                       OfflinePageMetadataStoreSQLFactory>
+    MyTypes;
+TYPED_TEST_CASE(OfflinePageMetadataStoreTest, MyTypes);
+
 // Loads empty store and makes sure that there are no offline pages stored in
 // it.
-TEST_F(OfflinePageMetadataStoreImplTest, LoadEmptyStore) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(0U, offline_pages_.size());
+TYPED_TEST(OfflinePageMetadataStoreTest, LoadEmptyStore) {
+  scoped_ptr<OfflinePageMetadataStore> store(this->BuildStore());
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(0U, this->offline_pages_.size());
 }
 
 // Adds metadata of an offline page into a store and then opens the store
 // again to make sure that stored metadata survives store restarts.
-TEST_F(OfflinePageMetadataStoreImplTest, AddOfflinePage) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
+TYPED_TEST(OfflinePageMetadataStoreTest, AddOfflinePage) {
+  scoped_ptr<OfflinePageMetadataStore> store(this->BuildStore());
 
   OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestBookmarkId,
                                base::FilePath(kFilePath), kFileSize);
   store->AddOrUpdateOfflinePage(
       offline_page,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), ADD));
-  PumpLoop();
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Close the store first to ensure file lock is removed.
   store.reset();
-  store = BuildStore();
-  PumpLoop();
-
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(1U, offline_pages_.size());
-  EXPECT_EQ(offline_page.url, offline_pages_[0].url);
-  EXPECT_EQ(offline_page.offline_id, offline_pages_[0].offline_id);
-  EXPECT_EQ(offline_page.version, offline_pages_[0].version);
-  EXPECT_EQ(offline_page.file_path, offline_pages_[0].file_path);
-  EXPECT_EQ(offline_page.file_size, offline_pages_[0].file_size);
-  EXPECT_EQ(offline_page.creation_time, offline_pages_[0].creation_time);
-  EXPECT_EQ(offline_page.last_access_time, offline_pages_[0].last_access_time);
-  EXPECT_EQ(offline_page.access_count, offline_pages_[0].access_count);
-  EXPECT_EQ(offline_page.client_id, offline_pages_[0].client_id);
+  store = this->BuildStore();
+  this->PumpLoop();
+
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(1U, this->offline_pages_.size());
+  EXPECT_EQ(offline_page.url, this->offline_pages_[0].url);
+  EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id);
+  EXPECT_EQ(offline_page.version, this->offline_pages_[0].version);
+  EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path);
+  EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size);
+  EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time);
+  EXPECT_EQ(offline_page.last_access_time,
+            this->offline_pages_[0].last_access_time);
+  EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count);
+  EXPECT_EQ(offline_page.client_id, this->offline_pages_[0].client_id);
 }
 
 // Tests removing offline page metadata from the store, for which it first adds
 // metadata of an offline page.
-TEST_F(OfflinePageMetadataStoreImplTest, RemoveOfflinePage) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
+TYPED_TEST(OfflinePageMetadataStoreTest, RemoveOfflinePage) {
+  scoped_ptr<OfflinePageMetadataStore> store(this->BuildStore());
 
   // Add an offline page.
   OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestBookmarkId,
                                base::FilePath(kFilePath), kFileSize);
   store->AddOrUpdateOfflinePage(
       offline_page,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), ADD));
-  PumpLoop();
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Load the store.
-  store->Load(base::Bind(&OfflinePageMetadataStoreImplTest::LoadCallback,
+  store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback,
                          base::Unretained(this)));
-  PumpLoop();
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(1U, offline_pages_.size());
+  this->PumpLoop();
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(1U, this->offline_pages_.size());
 
   // Remove the offline page.
   std::vector<int64_t> ids_to_remove;
   ids_to_remove.push_back(offline_page.offline_id);
   store->RemoveOfflinePages(
       ids_to_remove,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), REMOVE));
-  PumpLoop();
-  EXPECT_EQ(REMOVE, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(REMOVE, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Load the store.
-  store->Load(base::Bind(&OfflinePageMetadataStoreImplTest::LoadCallback,
+  store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback,
                          base::Unretained(this)));
-  PumpLoop();
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(0U, offline_pages_.size());
+  this->PumpLoop();
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(0U, this->offline_pages_.size());
 
-  ClearResults();
+  this->ClearResults();
 
   // Close and reload the store.
   store.reset();
-  store = BuildStore();
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(0U, offline_pages_.size());
+  store = this->BuildStore();
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(0U, this->offline_pages_.size());
 }
 
 // Adds metadata of multiple offline pages into a store and removes some.
-TEST_F(OfflinePageMetadataStoreImplTest, AddRemoveMultipleOfflinePages) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
+TYPED_TEST(OfflinePageMetadataStoreTest, AddRemoveMultipleOfflinePages) {
+  scoped_ptr<OfflinePageMetadataStore> store(this->BuildStore());
 
   // Add an offline page.
   OfflinePageItem offline_page_1(GURL(kTestURL), 12345LL, kTestBookmarkId,
@@ -246,148 +267,174 @@ TEST_F(OfflinePageMetadataStoreImplTest, AddRemoveMultipleOfflinePages) {
                                  base::Time::Now());
   store->AddOrUpdateOfflinePage(
       offline_page_1,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), ADD));
-  PumpLoop();
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Add anther offline page.
   store->AddOrUpdateOfflinePage(
       offline_page_2,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), ADD));
-  PumpLoop();
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Load the store.
-  store->Load(base::Bind(&OfflinePageMetadataStoreImplTest::LoadCallback,
+  store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback,
                          base::Unretained(this)));
-  PumpLoop();
+  this->PumpLoop();
 
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(2U, offline_pages_.size());
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(2U, this->offline_pages_.size());
 
   // Remove the offline page.
   std::vector<int64_t> ids_to_remove;
   ids_to_remove.push_back(offline_page_1.offline_id);
   store->RemoveOfflinePages(
       ids_to_remove,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), REMOVE));
-  PumpLoop();
-  EXPECT_EQ(REMOVE, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(REMOVE, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Close and reload the store.
   store.reset();
-  store = BuildStore();
-  store->Load(base::Bind(&OfflinePageMetadataStoreImplTest::LoadCallback,
+  store = this->BuildStore();
+  store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback,
                          base::Unretained(this)));
-  PumpLoop();
-
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(1U, offline_pages_.size());
-  EXPECT_EQ(offline_page_2.url, offline_pages_[0].url);
-  EXPECT_EQ(offline_page_2.offline_id, offline_pages_[0].offline_id);
-  EXPECT_EQ(offline_page_2.version, offline_pages_[0].version);
-  EXPECT_EQ(offline_page_2.file_path, offline_pages_[0].file_path);
-  EXPECT_EQ(offline_page_2.file_size, offline_pages_[0].file_size);
-  EXPECT_EQ(offline_page_2.creation_time, offline_pages_[0].creation_time);
+  this->PumpLoop();
+
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(1U, this->offline_pages_.size());
+  EXPECT_EQ(offline_page_2.url, this->offline_pages_[0].url);
+  EXPECT_EQ(offline_page_2.offline_id, this->offline_pages_[0].offline_id);
+  EXPECT_EQ(offline_page_2.version, this->offline_pages_[0].version);
+  EXPECT_EQ(offline_page_2.file_path, this->offline_pages_[0].file_path);
+  EXPECT_EQ(offline_page_2.file_size, this->offline_pages_[0].file_size);
+  EXPECT_EQ(offline_page_2.creation_time,
+            this->offline_pages_[0].creation_time);
   EXPECT_EQ(offline_page_2.last_access_time,
-            offline_pages_[0].last_access_time);
-  EXPECT_EQ(offline_page_2.access_count, offline_pages_[0].access_count);
-  EXPECT_EQ(offline_page_2.client_id, offline_pages_[0].client_id);
+            this->offline_pages_[0].last_access_time);
+  EXPECT_EQ(offline_page_2.access_count, this->offline_pages_[0].access_count);
+  EXPECT_EQ(offline_page_2.client_id, this->offline_pages_[0].client_id);
 }
 
 // Tests updating offline page metadata from the store.
-TEST_F(OfflinePageMetadataStoreImplTest, UpdateOfflinePage) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
+TYPED_TEST(OfflinePageMetadataStoreTest, UpdateOfflinePage) {
+  scoped_ptr<OfflinePageMetadataStore> store(this->BuildStore());
 
-  // First, adds a fresh page.
+  // First, add a fresh page.
   OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestBookmarkId,
                                base::FilePath(kFilePath), kFileSize);
   store->AddOrUpdateOfflinePage(
       offline_page,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), ADD));
-  PumpLoop();
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
-  store->Load(base::Bind(&OfflinePageMetadataStoreImplTest::LoadCallback,
+  this->ClearResults();
+  store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback,
                          base::Unretained(this)));
-  PumpLoop();
-
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(1U, offline_pages_.size());
-  EXPECT_EQ(offline_page.url, offline_pages_[0].url);
-  EXPECT_EQ(offline_page.offline_id, offline_pages_[0].offline_id);
-  EXPECT_EQ(offline_page.version, offline_pages_[0].version);
-  EXPECT_EQ(offline_page.file_path, offline_pages_[0].file_path);
-  EXPECT_EQ(offline_page.file_size, offline_pages_[0].file_size);
-  EXPECT_EQ(offline_page.creation_time, offline_pages_[0].creation_time);
-  EXPECT_EQ(offline_page.last_access_time, offline_pages_[0].last_access_time);
-  EXPECT_EQ(offline_page.access_count, offline_pages_[0].access_count);
-  EXPECT_EQ(offline_page.client_id, offline_pages_[0].client_id);
-
-  // Then updates some data.
+  this->PumpLoop();
+
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(1U, this->offline_pages_.size());
+  EXPECT_EQ(offline_page.url, this->offline_pages_[0].url);
+  EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id);
+  EXPECT_EQ(offline_page.version, this->offline_pages_[0].version);
+  EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path);
+  EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size);
+  EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time);
+  EXPECT_EQ(offline_page.last_access_time,
+            this->offline_pages_[0].last_access_time);
+  EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count);
+  EXPECT_EQ(offline_page.client_id, this->offline_pages_[0].client_id);
+
+  // Then update some data.
   offline_page.file_size = kFileSize + 1;
   offline_page.access_count++;
   store->AddOrUpdateOfflinePage(
       offline_page,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), ADD));
-  PumpLoop();
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
-  store->Load(base::Bind(&OfflinePageMetadataStoreImplTest::LoadCallback,
+  this->ClearResults();
+  store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback,
                          base::Unretained(this)));
-  PumpLoop();
+  this->PumpLoop();
+
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(1U, this->offline_pages_.size());
+  EXPECT_EQ(offline_page.url, this->offline_pages_[0].url);
+  EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id);
+  EXPECT_EQ(offline_page.version, this->offline_pages_[0].version);
+  EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path);
+  EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size);
+  EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time);
+  EXPECT_EQ(offline_page.last_access_time,
+            this->offline_pages_[0].last_access_time);
+  EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count);
+  EXPECT_EQ(offline_page.client_id, this->offline_pages_[0].client_id);
+}
 
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(1U, offline_pages_.size());
-  EXPECT_EQ(offline_page.url, offline_pages_[0].url);
-  EXPECT_EQ(offline_page.offline_id, offline_pages_[0].offline_id);
-  EXPECT_EQ(offline_page.version, offline_pages_[0].version);
-  EXPECT_EQ(offline_page.file_path, offline_pages_[0].file_path);
-  EXPECT_EQ(offline_page.file_size, offline_pages_[0].file_size);
-  EXPECT_EQ(offline_page.creation_time, offline_pages_[0].creation_time);
-  EXPECT_EQ(offline_page.last_access_time, offline_pages_[0].last_access_time);
-  EXPECT_EQ(offline_page.access_count, offline_pages_[0].access_count);
-  EXPECT_EQ(offline_page.client_id, offline_pages_[0].client_id);
+}  // namespace
+
+class OfflinePageMetadataStoreImplTest
+    : public OfflinePageMetadataStoreTest<OfflinePageMetadataStoreImplFactory> {
+ public:
+  void UpdateStoreEntries(
+      OfflinePageMetadataStoreImpl* store,
+      scoped_ptr<leveldb_proto::ProtoDatabase<OfflinePageEntry>::KeyEntryVector>
+          entries_to_save);
+};
+
+void OfflinePageMetadataStoreImplTest::UpdateStoreEntries(
+    OfflinePageMetadataStoreImpl* store,
+    scoped_ptr<leveldb_proto::ProtoDatabase<OfflinePageEntry>::KeyEntryVector>
+        entries_to_save) {
+  scoped_ptr<std::vector<std::string>> keys_to_remove(
+      new std::vector<std::string>());
+  store->UpdateEntries(
+      std::move(entries_to_save), std::move(keys_to_remove),
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
+                 base::Unretained(this), ADD));
 }
 
 // Test that loading a store with a bad value still loads.
 // Needs to be outside of the anonymous namespace in order for FRIEND_TEST
 // to work.
 TEST_F(OfflinePageMetadataStoreImplTest, LoadCorruptedStore) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
+  scoped_ptr<OfflinePageMetadataStore> store(this->BuildStore());
 
   // Write one ok page.
   OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestBookmarkId,
                                base::FilePath(kFilePath), kFileSize);
   store->AddOrUpdateOfflinePage(
       offline_page,
-      base::Bind(&OfflinePageMetadataStoreImplTest::UpdateCallback,
+      base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback,
                  base::Unretained(this), ADD));
-  PumpLoop();
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  this->PumpLoop();
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
   // Manually write one broken page (no id)
   scoped_ptr<leveldb_proto::ProtoDatabase<OfflinePageEntry>::KeyEntryVector>
@@ -397,38 +444,40 @@ TEST_F(OfflinePageMetadataStoreImplTest, LoadCorruptedStore) {
   OfflinePageEntry offline_page_proto;
   entries_to_save->push_back(std::make_pair("0", offline_page_proto));
 
-  UpdateStoreEntries(store.get(), std::move(entries_to_save));
-  PumpLoop();
+  UpdateStoreEntries((OfflinePageMetadataStoreImpl*)store.get(),

[Scott Hess - ex-Googler, 2016/04/25 19:57:55]

I think Chromium style guide always wants static_cast<...>().

+                     std::move(entries_to_save));
+  this->PumpLoop();
 
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Close the store first to ensure file lock is removed.
   store.reset();
-  store = BuildStore();
-  PumpLoop();
+  store = this->BuildStore();
+  this->PumpLoop();
 
   // One of the pages was busted, so only expect one page.
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
-  EXPECT_EQ(1U, offline_pages_.size());
-  EXPECT_EQ(offline_page.url, offline_pages_[0].url);
-  EXPECT_EQ(offline_page.offline_id, offline_pages_[0].offline_id);
-  EXPECT_EQ(offline_page.version, offline_pages_[0].version);
-  EXPECT_EQ(offline_page.file_path, offline_pages_[0].file_path);
-  EXPECT_EQ(offline_page.file_size, offline_pages_[0].file_size);
-  EXPECT_EQ(offline_page.creation_time, offline_pages_[0].creation_time);
-  EXPECT_EQ(offline_page.last_access_time, offline_pages_[0].last_access_time);
-  EXPECT_EQ(offline_page.access_count, offline_pages_[0].access_count);
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
+  EXPECT_EQ(1U, this->offline_pages_.size());
+  EXPECT_EQ(offline_page.url, this->offline_pages_[0].url);
+  EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id);
+  EXPECT_EQ(offline_page.version, this->offline_pages_[0].version);
+  EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path);
+  EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size);
+  EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time);
+  EXPECT_EQ(offline_page.last_access_time,
+            this->offline_pages_[0].last_access_time);
+  EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count);
 }
 
 // Test that loading a store with nothing but bad values errors.
 // Needs to be outside of the anonymous namespace in order for FRIEND_TEST
 // to work.
 TEST_F(OfflinePageMetadataStoreImplTest, LoadTotallyCorruptedStore) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
+  scoped_ptr<OfflinePageMetadataStore> store(this->BuildStore());
 
   // Manually write two broken pages (no id)
   scoped_ptr<leveldb_proto::ProtoDatabase<OfflinePageEntry>::KeyEntryVector>
@@ -439,26 +488,28 @@ TEST_F(OfflinePageMetadataStoreImplTest, LoadTotallyCorruptedStore) {
   entries_to_save->push_back(std::make_pair("0", offline_page_proto));
   entries_to_save->push_back(std::make_pair("1", offline_page_proto));
 
-  UpdateStoreEntries(store.get(), std::move(entries_to_save));;
-  PumpLoop();
+  UpdateStoreEntries((OfflinePageMetadataStoreImpl*)store.get(),

[Scott Hess - ex-Googler, 2016/04/25 19:57:55]

Also here.

+                     std::move(entries_to_save));
+  ;
+  this->PumpLoop();
 
-  EXPECT_EQ(ADD, last_called_callback_);
-  EXPECT_EQ(STATUS_TRUE, last_status_);
+  EXPECT_EQ(ADD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_TRUE, this->last_status_);
 
-  ClearResults();
+  this->ClearResults();
 
   // Close the store first to ensure file lock is removed.
   store.reset();
-  store = BuildStore();
-  PumpLoop();
+  store = this->BuildStore();
+  this->PumpLoop();
 
   // One of the pages was busted, so only expect one page.
-  EXPECT_EQ(LOAD, last_called_callback_);
-  EXPECT_EQ(STATUS_FALSE, last_status_);
+  EXPECT_EQ(LOAD, this->last_called_callback_);
+  EXPECT_EQ(STATUS_FALSE, this->last_status_);
 }
 
 TEST_F(OfflinePageMetadataStoreImplTest, UpgradeStoreFromBookmarkIdToClientId) {
-  scoped_ptr<OfflinePageMetadataStoreImpl> store(BuildStore());
+  scoped_ptr<OfflinePageMetadataStore> store(BuildStore());
 
   // Manually write a page referring to legacy bookmark id.
   scoped_ptr<leveldb_proto::ProtoDatabase<OfflinePageEntry>::KeyEntryVector>
@@ -472,7 +523,8 @@ TEST_F(OfflinePageMetadataStoreImplTest, UpgradeStoreFromBookmarkIdToClientId) {
   offline_page_proto.set_file_path("/foo/bar");
   entries_to_save->push_back(std::make_pair("1", offline_page_proto));
 
-  UpdateStoreEntries(store.get(), std::move(entries_to_save));
+  UpdateStoreEntries((OfflinePageMetadataStoreImpl*)store.get(),

[Scott Hess - ex-Googler, 2016/04/25 19:57:55]

Also here.

+                     std::move(entries_to_save));
   PumpLoop();
 
   EXPECT_EQ(ADD, last_called_callback_);