Split IndexedDBFactory into virtual base + impl.

This change makes IndexedDBFactory a virtual base class, and what was
IndexedDBFactory is now IndexedDBFactoryImpl. This change will allow tests
to create proper mock factories and not require them to instantiate the
"real" factory (which often doesn't work) in the test. It will also take us
one step closer to eliminating special cases in IDB's other classes where
having a factory is optional (because tests don't create them).

BUG=393974
R=ericu@chromium.org, jochen@chromium.org, jsbell@chromium.org, michaeln@chromium.org

Review URL: https://codereview.chromium.org/313883003

Patch from cmumford@chromium.org <cmumford@chromium.org>.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284161 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/indexed_db/indexed_db_active_blob_registry_unittest.cc

@@ -7,7 +7,7 @@
 #include "base/test/test_simple_task_runner.h"
 #include "content/browser/indexed_db/indexed_db_active_blob_registry.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
-#include "content/browser/indexed_db/indexed_db_factory.h"
+#include "content/browser/indexed_db/indexed_db_factory_impl.h"
 #include "content/browser/indexed_db/indexed_db_fake_backing_store.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -15,9 +15,9 @@ namespace content {
 
 namespace {
 
-class MockIDBFactory : public IndexedDBFactory {
+class MockIDBFactory : public IndexedDBFactoryImpl {
  public:
-  MockIDBFactory() : IndexedDBFactory(NULL), duplicate_calls_(false) {}
+  MockIDBFactory() : IndexedDBFactoryImpl(NULL), duplicate_calls_(false) {}
 
   virtual void ReportOutstandingBlobs(const GURL& origin_url,
                                       bool blobs_outstanding) OVERRIDE {

content/browser/indexed_db/indexed_db_backing_store_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/task_runner.h"
 #include "base/test/test_simple_task_runner.h"
 #include "content/browser/indexed_db/indexed_db_context_impl.h"
+#include "content/browser/indexed_db/indexed_db_factory_impl.h"
 #include "content/browser/indexed_db/indexed_db_leveldb_coding.h"
 #include "content/browser/indexed_db/indexed_db_value.h"
 #include "content/browser/indexed_db/leveldb/leveldb_factory.h"
@@ -173,10 +174,10 @@ class TestableIndexedDBBackingStore : public IndexedDBBackingStore {
   DISALLOW_COPY_AND_ASSIGN(TestableIndexedDBBackingStore);
 };
 
-class TestIDBFactory : public IndexedDBFactory {
+class TestIDBFactory : public IndexedDBFactoryImpl {
  public:
   explicit TestIDBFactory(IndexedDBContextImpl* idb_context)
-      : IndexedDBFactory(idb_context) {}
+      : IndexedDBFactoryImpl(idb_context) {}
 
   scoped_refptr<TestableIndexedDBBackingStore> OpenBackingStoreForTest(
       const GURL& origin,

content/browser/indexed_db/indexed_db_context_impl.cc

@@ -23,7 +23,7 @@
 #include "content/browser/indexed_db/indexed_db_connection.h"
 #include "content/browser/indexed_db/indexed_db_database.h"
 #include "content/browser/indexed_db/indexed_db_dispatcher_host.h"
-#include "content/browser/indexed_db/indexed_db_factory.h"
+#include "content/browser/indexed_db/indexed_db_factory_impl.h"
 #include "content/browser/indexed_db/indexed_db_quota_client.h"
 #include "content/browser/indexed_db/indexed_db_tracing.h"
 #include "content/browser/indexed_db/indexed_db_transaction.h"
@@ -123,7 +123,7 @@ IndexedDBFactory* IndexedDBContextImpl::GetIDBFactory() {
     // Prime our cache of origins with existing databases so we can
     // detect when dbs are newly created.
     GetOriginSet();
-    factory_ = new IndexedDBFactory(this);
+    factory_ = new IndexedDBFactoryImpl(this);
   }
   return factory_;
 }

content/browser/indexed_db/indexed_db_factory.h

@@ -27,7 +27,6 @@ class URLRequestContext;
 namespace content {
 
 class IndexedDBBackingStore;
-class IndexedDBContextImpl;
 struct IndexedDBPendingConnection;
 
 class CONTENT_EXPORT IndexedDBFactory
@@ -36,52 +35,53 @@ class CONTENT_EXPORT IndexedDBFactory
   typedef std::multimap<GURL, IndexedDBDatabase*> OriginDBMap;
   typedef OriginDBMap::const_iterator OriginDBMapIterator;
 
-  explicit IndexedDBFactory(IndexedDBContextImpl* context);
-
-  void ReleaseDatabase(const IndexedDBDatabase::Identifier& identifier,
-                       bool forcedClose);
-
-  void GetDatabaseNames(scoped_refptr<IndexedDBCallbacks> callbacks,
-                        const GURL& origin_url,
-                        const base::FilePath& data_directory,
-                        net::URLRequestContext* request_context);
-  void Open(const base::string16& name,
-            const IndexedDBPendingConnection& connection,
-            net::URLRequestContext* request_context,
-            const GURL& origin_url,
-            const base::FilePath& data_directory);
-
-  void DeleteDatabase(const base::string16& name,
-                      net::URLRequestContext* request_context,
-                      scoped_refptr<IndexedDBCallbacks> callbacks,
-                      const GURL& origin_url,
-                      const base::FilePath& data_directory);
-
-  void HandleBackingStoreFailure(const GURL& origin_url);
-  void HandleBackingStoreCorruption(const GURL& origin_url,
-                                    const IndexedDBDatabaseError& error);
+  virtual void ReleaseDatabase(const IndexedDBDatabase::Identifier& identifier,
+                               bool forcedClose) = 0;
+
+  virtual void GetDatabaseNames(scoped_refptr<IndexedDBCallbacks> callbacks,
+                                const GURL& origin_url,
+                                const base::FilePath& data_directory,
+                                net::URLRequestContext* request_context) = 0;
+  virtual void Open(const base::string16& name,
+                    const IndexedDBPendingConnection& connection,
+                    net::URLRequestContext* request_context,
+                    const GURL& origin_url,
+                    const base::FilePath& data_directory) = 0;
+
+  virtual void DeleteDatabase(const base::string16& name,
+                              net::URLRequestContext* request_context,
+                              scoped_refptr<IndexedDBCallbacks> callbacks,
+                              const GURL& origin_url,
+                              const base::FilePath& data_directory) = 0;
+
+  virtual void HandleBackingStoreFailure(const GURL& origin_url) = 0;
+  virtual void HandleBackingStoreCorruption(
+      const GURL& origin_url,
+      const IndexedDBDatabaseError& error) = 0;
 
-  std::pair<OriginDBMapIterator, OriginDBMapIterator> GetOpenDatabasesForOrigin(
-      const GURL& origin_url) const;
+  virtual std::pair<OriginDBMapIterator, OriginDBMapIterator>
+      GetOpenDatabasesForOrigin(const GURL& origin_url) const = 0;
 
-  void ForceClose(const GURL& origin_url);
+  virtual void ForceClose(const GURL& origin_url) = 0;
 
   // Called by the IndexedDBContext destructor so the factory can do cleanup.
-  void ContextDestroyed();
+  virtual void ContextDestroyed() = 0;
 
   // Called by the IndexedDBActiveBlobRegistry.
   virtual void ReportOutstandingBlobs(const GURL& origin_url,
-                                      bool blobs_outstanding);
+                                      bool blobs_outstanding) = 0;
 
   // Called by an IndexedDBDatabase when it is actually deleted.
-  void DatabaseDeleted(const IndexedDBDatabase::Identifier& identifier);
+  virtual void DatabaseDeleted(
+      const IndexedDBDatabase::Identifier& identifier) = 0;
 
-  size_t GetConnectionCount(const GURL& origin_url) const;
+  virtual size_t GetConnectionCount(const GURL& origin_url) const = 0;
 
  protected:
   friend class base::RefCountedThreadSafe<IndexedDBFactory>;
 
-  virtual ~IndexedDBFactory();
+  IndexedDBFactory() {}
+  virtual ~IndexedDBFactory() {}
 
   virtual scoped_refptr<IndexedDBBackingStore> OpenBackingStore(
       const GURL& origin_url,
@@ -90,7 +90,7 @@ class CONTENT_EXPORT IndexedDBFactory
       blink::WebIDBDataLoss* data_loss,
       std::string* data_loss_reason,
       bool* disk_full,
-      leveldb::Status* status);
+      leveldb::Status* status) = 0;
 
   virtual scoped_refptr<IndexedDBBackingStore> OpenBackingStoreHelper(
       const GURL& origin_url,
@@ -100,54 +100,9 @@ class CONTENT_EXPORT IndexedDBFactory
       std::string* data_loss_message,
       bool* disk_full,
       bool first_time,
-      leveldb::Status* status);
-
-  void ReleaseBackingStore(const GURL& origin_url, bool immediate);
-  void CloseBackingStore(const GURL& origin_url);
-  IndexedDBContextImpl* context() const { return context_; }
+      leveldb::Status* status) = 0;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
-                           BackingStoreReleasedOnForcedClose);
-  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
-                           BackingStoreReleaseDelayedOnClose);
-  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest, DatabaseFailedOpen);
-  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
-                           DeleteDatabaseClosesBackingStore);
-  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
-                           ForceCloseReleasesBackingStore);
-  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
-                           GetDatabaseNamesClosesBackingStore);
-  FRIEND_TEST_ALL_PREFIXES(IndexedDBTest,
-                           ForceCloseOpenDatabasesOnCommitFailure);
-
-  // Called internally after a database is closed, with some delay. If this
-  // factory has the last reference, it will be released.
-  void MaybeCloseBackingStore(const GURL& origin_url);
-  bool HasLastBackingStoreReference(const GURL& origin_url) const;
-
-  // Testing helpers, so unit tests don't need to grovel through internal state.
-  bool IsDatabaseOpen(const GURL& origin_url,
-                      const base::string16& name) const;
-  bool IsBackingStoreOpen(const GURL& origin_url) const;
-  bool IsBackingStorePendingClose(const GURL& origin_url) const;
-  void RemoveDatabaseFromMaps(const IndexedDBDatabase::Identifier& identifier);
-
-  IndexedDBContextImpl* context_;
-
-  typedef std::map<IndexedDBDatabase::Identifier,
-                   IndexedDBDatabase*> IndexedDBDatabaseMap;
-  IndexedDBDatabaseMap database_map_;
-  OriginDBMap origin_dbs_;
-
-  typedef std::map<GURL, scoped_refptr<IndexedDBBackingStore> >
-      IndexedDBBackingStoreMap;
-  IndexedDBBackingStoreMap backing_store_map_;
-
-  std::set<scoped_refptr<IndexedDBBackingStore> > session_only_backing_stores_;
-  IndexedDBBackingStoreMap backing_stores_with_active_blobs_;
-  std::set<GURL> backends_opened_since_boot_;
-
   DISALLOW_COPY_AND_ASSIGN(IndexedDBFactory);
 };
 

content/browser/indexed_db/indexed_db_factory_impl.h

+// Copyright (c) 2014 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 CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_FACTORY_IMPL_H_
+#define CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_FACTORY_IMPL_H_
+
+#include "content/browser/indexed_db/indexed_db_factory.h"
+
+namespace content {
+
+class IndexedDBContextImpl;
+
+class CONTENT_EXPORT IndexedDBFactoryImpl : public IndexedDBFactory {
+ public:
+  explicit IndexedDBFactoryImpl(IndexedDBContextImpl* context);
+
+  // content::IndexedDBFactory overrides:
+  virtual void ReleaseDatabase(const IndexedDBDatabase::Identifier& identifier,
+                               bool forcedClose) OVERRIDE;
+
+  virtual void GetDatabaseNames(
+      scoped_refptr<IndexedDBCallbacks> callbacks,
+      const GURL& origin_url,
+      const base::FilePath& data_directory,
+      net::URLRequestContext* request_context) OVERRIDE;
+  virtual void Open(const base::string16& name,
+                    const IndexedDBPendingConnection& connection,
+                    net::URLRequestContext* request_context,
+                    const GURL& origin_url,
+                    const base::FilePath& data_directory) OVERRIDE;
+
+  virtual void DeleteDatabase(const base::string16& name,
+                              net::URLRequestContext* request_context,
+                              scoped_refptr<IndexedDBCallbacks> callbacks,
+                              const GURL& origin_url,
+                              const base::FilePath& data_directory) OVERRIDE;
+
+  virtual void HandleBackingStoreFailure(const GURL& origin_url) OVERRIDE;
+  virtual void HandleBackingStoreCorruption(
+      const GURL& origin_url,
+      const IndexedDBDatabaseError& error) OVERRIDE;
+
+  virtual std::pair<OriginDBMapIterator, OriginDBMapIterator>
+      GetOpenDatabasesForOrigin(const GURL& origin_url) const OVERRIDE;
+
+  virtual void ForceClose(const GURL& origin_url) OVERRIDE;
+
+  // Called by the IndexedDBContext destructor so the factory can do cleanup.
+  virtual void ContextDestroyed() OVERRIDE;
+
+  // Called by the IndexedDBActiveBlobRegistry.
+  virtual void ReportOutstandingBlobs(const GURL& origin_url,
+                                      bool blobs_outstanding) OVERRIDE;
+
+  // Called by an IndexedDBDatabase when it is actually deleted.
+  virtual void DatabaseDeleted(
+      const IndexedDBDatabase::Identifier& identifier) OVERRIDE;
+
+  virtual size_t GetConnectionCount(const GURL& origin_url) const OVERRIDE;
+
+ protected:
+  virtual ~IndexedDBFactoryImpl();
+
+  virtual scoped_refptr<IndexedDBBackingStore> OpenBackingStore(
+      const GURL& origin_url,
+      const base::FilePath& data_directory,
+      net::URLRequestContext* request_context,
+      blink::WebIDBDataLoss* data_loss,
+      std::string* data_loss_reason,
+      bool* disk_full,
+      leveldb::Status* s) OVERRIDE;
+
+  virtual scoped_refptr<IndexedDBBackingStore> OpenBackingStoreHelper(
+      const GURL& origin_url,
+      const base::FilePath& data_directory,
+      net::URLRequestContext* request_context,
+      blink::WebIDBDataLoss* data_loss,
+      std::string* data_loss_message,
+      bool* disk_full,
+      bool first_time,
+      leveldb::Status* s) OVERRIDE;
+
+  void ReleaseBackingStore(const GURL& origin_url, bool immediate);
+  void CloseBackingStore(const GURL& origin_url);
+  IndexedDBContextImpl* context() const { return context_; }
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
+                           BackingStoreReleasedOnForcedClose);
+  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
+                           BackingStoreReleaseDelayedOnClose);
+  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest, DatabaseFailedOpen);
+  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
+                           DeleteDatabaseClosesBackingStore);
+  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
+                           ForceCloseReleasesBackingStore);
+  FRIEND_TEST_ALL_PREFIXES(IndexedDBFactoryTest,
+                           GetDatabaseNamesClosesBackingStore);
+  FRIEND_TEST_ALL_PREFIXES(IndexedDBTest,
+                           ForceCloseOpenDatabasesOnCommitFailure);
+
+  // Called internally after a database is closed, with some delay. If this
+  // factory has the last reference, it will be released.
+  void MaybeCloseBackingStore(const GURL& origin_url);
+  bool HasLastBackingStoreReference(const GURL& origin_url) const;
+
+  // Testing helpers, so unit tests don't need to grovel through internal state.
+  bool IsDatabaseOpen(const GURL& origin_url, const base::string16& name) const;
+  bool IsBackingStoreOpen(const GURL& origin_url) const;
+  bool IsBackingStorePendingClose(const GURL& origin_url) const;
+  void RemoveDatabaseFromMaps(const IndexedDBDatabase::Identifier& identifier);
+
+  IndexedDBContextImpl* context_;
+
+  typedef std::map<IndexedDBDatabase::Identifier, IndexedDBDatabase*>
+      IndexedDBDatabaseMap;
+  IndexedDBDatabaseMap database_map_;
+  OriginDBMap origin_dbs_;
+
+  typedef std::map<GURL, scoped_refptr<IndexedDBBackingStore> >
+      IndexedDBBackingStoreMap;
+  IndexedDBBackingStoreMap backing_store_map_;
+
+  std::set<scoped_refptr<IndexedDBBackingStore> > session_only_backing_stores_;
+  IndexedDBBackingStoreMap backing_stores_with_active_blobs_;
+  std::set<GURL> backends_opened_since_boot_;
+
+  DISALLOW_COPY_AND_ASSIGN(IndexedDBFactoryImpl);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_FACTORY_IMPL_H_

content/browser/indexed_db/indexed_db_factory_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/browser/indexed_db/indexed_db_factory.h"
+#include "content/browser/indexed_db/indexed_db_factory_impl.h"
 
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -26,10 +26,10 @@ namespace content {
 
 namespace {
 
-class MockIDBFactory : public IndexedDBFactory {
+class MockIDBFactory : public IndexedDBFactoryImpl {
  public:
   explicit MockIDBFactory(IndexedDBContextImpl* context)
-      : IndexedDBFactory(context) {}
+      : IndexedDBFactoryImpl(context) {}
   scoped_refptr<IndexedDBBackingStore> TestOpenBackingStore(
       const GURL& origin,
       const base::FilePath& data_directory) {
@@ -199,10 +199,10 @@ TEST_F(IndexedDBFactoryTest, RejectLongOrigins) {
   EXPECT_TRUE(diskStore2);
 }
 
-class DiskFullFactory : public IndexedDBFactory {
+class DiskFullFactory : public IndexedDBFactoryImpl {
  public:
   explicit DiskFullFactory(IndexedDBContextImpl* context)
-      : IndexedDBFactory(context) {}
+      : IndexedDBFactoryImpl(context) {}
 
  private:
   virtual ~DiskFullFactory() {}

content/browser/indexed_db/indexed_db_unittest.cc

@@ -9,6 +9,7 @@
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/indexed_db/indexed_db_connection.h"
 #include "content/browser/indexed_db/indexed_db_context_impl.h"
+#include "content/browser/indexed_db/indexed_db_factory_impl.h"
 #include "content/browser/indexed_db/mock_indexed_db_callbacks.h"
 #include "content/browser/indexed_db/mock_indexed_db_database_callbacks.h"
 #include "content/public/browser/storage_partition.h"
@@ -247,7 +248,8 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnCommitFailure) {
   scoped_refptr<IndexedDBContextImpl> context = new IndexedDBContextImpl(
       temp_dir.path(), special_storage_policy_, NULL, task_runner_);
 
-  scoped_refptr<IndexedDBFactory> factory = context->GetIDBFactory();
+  scoped_refptr<IndexedDBFactoryImpl> factory =
+      static_cast<IndexedDBFactoryImpl*>(context->GetIDBFactory());
 
   scoped_refptr<MockIndexedDBCallbacks> callbacks(new MockIndexedDBCallbacks());
   scoped_refptr<MockIndexedDBDatabaseCallbacks> db_callbacks(

content/content_browser.gypi

@@ -725,8 +725,9 @@
       'browser/indexed_db/indexed_db_database_error.h',
       'browser/indexed_db/indexed_db_dispatcher_host.cc',
       'browser/indexed_db/indexed_db_dispatcher_host.h',
-      'browser/indexed_db/indexed_db_factory.cc',
       'browser/indexed_db/indexed_db_factory.h',
+      'browser/indexed_db/indexed_db_factory_impl.cc',
+      'browser/indexed_db/indexed_db_factory_impl.h',
       'browser/indexed_db/indexed_db_index_writer.cc',
       'browser/indexed_db/indexed_db_index_writer.h',
       'browser/indexed_db/indexed_db_internals_ui.cc',