Split IndexedDBFactory into virtual base + impl.

content/browser/indexed_db/indexed_db_factory.h

 // Copyright (c) 2013 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_H_
 #define CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_FACTORY_H_
 
 #include <map>
 #include <set>
 #include <string>
 #include <utility>
 
 #include "base/basictypes.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
 #include "content/browser/indexed_db/indexed_db_callbacks.h"
 #include "content/browser/indexed_db/indexed_db_database.h"
 #include "content/browser/indexed_db/indexed_db_database_callbacks.h"
 #include "content/common/content_export.h"
 #include "url/gurl.h"
 
 namespace net {
 class URLRequestContext;
 }
 
 namespace content {
 
 class IndexedDBBackingStore;
-class IndexedDBContextImpl;
 struct IndexedDBPendingConnection;
 
 class CONTENT_EXPORT IndexedDBFactory
     : NON_EXPORTED_BASE(public base::RefCountedThreadSafe<IndexedDBFactory>) {

[jochen (gone - plz use gerrit), 2014/07/15 11:44:46]

an interface shouldn't inherit from something else

[cmumford, 2014/07/15 16:11:47]

Then maybe I shouldn't use the word "interface" in my commit comment? The
implementation does still need to have a reference counted base class from which
other implementation classes inherit. I'm modeling this after the existing
IndexedDBContext/IndexedDBContextImpl. I'll change the description to say
something like "base implementation".

  public:
   typedef std::multimap<GURL, IndexedDBDatabase*> OriginDBMap;
   typedef OriginDBMap::const_iterator OriginDBMapIterator;
 
-  explicit IndexedDBFactory(IndexedDBContextImpl* context);
+  virtual void ReleaseDatabase(const IndexedDBDatabase::Identifier& identifier,
+                               bool forcedClose) = 0;
 
-  void ReleaseDatabase(const IndexedDBDatabase::Identifier& identifier,
-                       bool forcedClose);
+  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;
 
-  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);
+  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;
 
-  void DeleteDatabase(const base::string16& name,
-                      net::URLRequestContext* request_context,
-                      scoped_refptr<IndexedDBCallbacks> callbacks,
-                      const GURL& origin_url,
-                      const base::FilePath& data_directory);
+  virtual void HandleBackingStoreFailure(const GURL& origin_url) = 0;
+  virtual void HandleBackingStoreCorruption(
+      const GURL& origin_url,
+      const IndexedDBDatabaseError& error) = 0;
 
-  void HandleBackingStoreFailure(const GURL& origin_url);
-  void HandleBackingStoreCorruption(const GURL& origin_url,
-                                    const IndexedDBDatabaseError& error);
+  virtual std::pair<OriginDBMapIterator, OriginDBMapIterator>
+      GetOpenDatabasesForOrigin(const GURL& origin_url) const = 0;
 
-  std::pair<OriginDBMapIterator, OriginDBMapIterator> GetOpenDatabasesForOrigin(
-      const GURL& origin_url) const;
-
-  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() {}

[jochen (gone - plz use gerrit), 2014/07/15 11:44:46]

no need for a ctor on an interface

[cmumford, 2014/07/15 16:11:47]

W/o it I get this error:

error: constructor for 'content::IndexedDBFactoryImpl' must explicitly
initialize the base class 'content::IndexedDBFactory' which does not have a
default constructor

+  virtual ~IndexedDBFactory() {}

[jochen (gone - plz use gerrit), 2014/07/15 11:44:46]

the dtor can be public as well

[cmumford, 2014/07/15 16:11:47]

I think that describing as not an interface satisfies this.

 
   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);
+      bool* disk_full) = 0;
 
   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);
-
-  void ReleaseBackingStore(const GURL& origin_url, bool immediate);
-  void CloseBackingStore(const GURL& origin_url);
-  IndexedDBContextImpl* context() const { return context_; }
+      bool first_time) = 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_;
+  virtual bool IsBackingStoreOpen(const GURL& origin_url) const = 0;
 
   DISALLOW_COPY_AND_ASSIGN(IndexedDBFactory);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_FACTORY_H_