Unreviewed, rolling out r145166.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Mar 2013 05:51:31 +0000 (05:51 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Mar 2013 05:51:31 +0000 (05:51 +0000)
http://trac.webkit.org/changeset/145166
https://bugs.webkit.org/show_bug.cgi?id=111819

build break - no symbol
webkit_support::CreateScopedTempDirectory() (Requested by
hayato on #webkit).

Patch by Sheriff Bot <webkit.review.bot@gmail.com> on 2013-03-07

Source/WebCore:

* Modules/indexeddb/IDBBackingStore.cpp:
(WebCore::IDBBackingStore::IDBBackingStore):
(WebCore::IDBBackingStore::~IDBBackingStore):
(WebCore::IDBBackingStore::open):
* Modules/indexeddb/IDBBackingStore.h:
(WebCore):
(IDBBackingStore):
* Modules/indexeddb/IDBFactoryBackendImpl.cpp:
(WebCore::IDBFactoryBackendImpl::addIDBBackingStore):
(WebCore):
(WebCore::IDBFactoryBackendImpl::removeIDBBackingStore):
(WebCore::IDBFactoryBackendImpl::openBackingStore):
* Modules/indexeddb/IDBFactoryBackendImpl.h:
(IDBFactoryBackendImpl):

Source/WebKit/chromium:

* tests/IDBBackingStoreTest.cpp:
(WebCore::IDBBackingStoreTest::SetUp):
(IDBBackingStoreTest):
* tests/IDBCleanupOnIOErrorTest.cpp:
(WebCore::TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@145180 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp
Source/WebCore/Modules/indexeddb/IDBBackingStore.h
Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp
Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/IDBBackingStoreTest.cpp
Source/WebKit/chromium/tests/IDBCleanupOnIOErrorTest.cpp

index 41e5de3..3c8e350 100644 (file)
@@ -1,3 +1,28 @@
+2013-03-07  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r145166.
+        http://trac.webkit.org/changeset/145166
+        https://bugs.webkit.org/show_bug.cgi?id=111819
+
+        build break - no symbol
+        webkit_support::CreateScopedTempDirectory() (Requested by
+        hayato on #webkit).
+
+        * Modules/indexeddb/IDBBackingStore.cpp:
+        (WebCore::IDBBackingStore::IDBBackingStore):
+        (WebCore::IDBBackingStore::~IDBBackingStore):
+        (WebCore::IDBBackingStore::open):
+        * Modules/indexeddb/IDBBackingStore.h:
+        (WebCore):
+        (IDBBackingStore):
+        * Modules/indexeddb/IDBFactoryBackendImpl.cpp:
+        (WebCore::IDBFactoryBackendImpl::addIDBBackingStore):
+        (WebCore):
+        (WebCore::IDBFactoryBackendImpl::removeIDBBackingStore):
+        (WebCore::IDBFactoryBackendImpl::openBackingStore):
+        * Modules/indexeddb/IDBFactoryBackendImpl.h:
+        (IDBFactoryBackendImpl):
+
 2013-03-07  Hajime Morrita  <morrita@google.com>
 
         Custom Elements: CustomElement constructor shouldn't share function instance
index 0abcfed..e37d7a6 100644 (file)
@@ -30,6 +30,7 @@
 
 #include "FileSystem.h"
 #include "HistogramSupport.h"
+#include "IDBFactoryBackendImpl.h"
 #include "IDBKey.h"
 #include "IDBKeyPath.h"
 #include "IDBKeyRange.h"
@@ -43,6 +44,9 @@
 #include "LevelDBTransaction.h"
 #include "SecurityOrigin.h"
 #include "SharedBuffer.h"
+#if PLATFORM(CHROMIUM)
+#include <public/Platform.h>
+#endif
 #include <wtf/Assertions.h>
 
 namespace WebCore {
@@ -335,20 +339,28 @@ public:
     }
 };
 
-IDBBackingStore::IDBBackingStore(const String& identifier, PassOwnPtr<LevelDBDatabase> db)
+IDBBackingStore::IDBBackingStore(const String& identifier, IDBFactoryBackendImpl* factory, PassOwnPtr<LevelDBDatabase> db)
     : m_identifier(identifier)
+    , m_factory(factory)
     , m_db(db)
-    , m_weakFactory(this)
 {
+#if PLATFORM(CHROMIUM)
+    ASSERT(m_factory || WebKit::Platform::current()->unitTestSupport());
+#endif
+    if (m_factory)
+        m_factory->addIDBBackingStore(identifier, this);
 }
 
 IDBBackingStore::IDBBackingStore()
-    : m_weakFactory(this)
 {
 }
 
 IDBBackingStore::~IDBBackingStore()
 {
+    // Only null in tests.
+    if (m_factory)
+        m_factory->removeIDBBackingStore(m_identifier);
+
     // m_db's destructor uses m_comparator. The order of destruction is important.
     m_db.clear();
     m_comparator.clear();
@@ -368,13 +380,13 @@ enum IDBLevelDBBackingStoreOpenResult {
     IDBLevelDBBackingStoreOpenMax,
 };
 
-PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin, const String& pathBaseArg, const String& fileIdentifier)
+PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin, const String& pathBaseArg, const String& fileIdentifier, IDBFactoryBackendImpl* factory)
 {
     DefaultLevelDBFactory levelDBFactory;
-    return IDBBackingStore::open(securityOrigin, pathBaseArg, fileIdentifier, &levelDBFactory);
+    return IDBBackingStore::open(securityOrigin, pathBaseArg, fileIdentifier, factory, &levelDBFactory);
 }
 
-PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin, const String& pathBaseArg, const String& fileIdentifier, LevelDBFactory* levelDBFactory)
+PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin, const String& pathBaseArg, const String& fileIdentifier, IDBFactoryBackendImpl* factory, LevelDBFactory* levelDBFactory)
 {
     IDB_TRACE("IDBBackingStore::open");
     String pathBase = pathBaseArg;
@@ -444,7 +456,7 @@ PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin
 
     // FIXME: Handle comparator name changes.
 
-    RefPtr<IDBBackingStore> backingStore(adoptRef(new IDBBackingStore(fileIdentifier, db.release())));
+    RefPtr<IDBBackingStore> backingStore(adoptRef(new IDBBackingStore(fileIdentifier, factory, db.release())));
     backingStore->m_comparator = comparator.release();
 
     if (!setUpMetadata(backingStore->m_db.get(), fileIdentifier))
index e8ee6b6..fcfe2da 100644 (file)
 #include "LevelDBTransaction.h"
 #include <wtf/OwnPtr.h>
 #include <wtf/RefCounted.h>
-#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
 class LevelDBComparator;
 class LevelDBDatabase;
 class LevelDBTransaction;
+class IDBFactoryBackendImpl;
 class IDBKey;
 class IDBKeyRange;
 class SecurityOrigin;
@@ -57,9 +57,8 @@ public:
     class Transaction;
 
     virtual ~IDBBackingStore();
-    static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier);
-    static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier, LevelDBFactory*);
-    WeakPtr<IDBBackingStore> createWeakPtr() { return m_weakFactory.createWeakPtr(); }
+    static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier, IDBFactoryBackendImpl*);
+    static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier, IDBFactoryBackendImpl*, LevelDBFactory*);
 
     virtual Vector<String> getDatabaseNames();
     virtual bool getIDBDatabaseMetaData(const String& name, IDBDatabaseMetadata*, bool& success) WARN_UNUSED_RETURN;
@@ -171,7 +170,7 @@ public:
     };
 
 protected:
-    IDBBackingStore(const String& identifier, PassOwnPtr<LevelDBDatabase>);
+    IDBBackingStore(const String& identifier, IDBFactoryBackendImpl*, PassOwnPtr<LevelDBDatabase>);
 
     // Should only used for mocking.
     IDBBackingStore();
@@ -181,10 +180,10 @@ private:
     void getIndexes(int64_t databaseId, int64_t objectStoreId, IDBObjectStoreMetadata::IndexMap*);
 
     String m_identifier;
-
+    RefPtr<IDBFactoryBackendImpl> m_factory;
     OwnPtr<LevelDBDatabase> m_db;
     OwnPtr<LevelDBComparator> m_comparator;
-    WeakPtrFactory<IDBBackingStore> m_weakFactory;
+
 };
 
 } // namespace WebCore
index 4191750..23c616e 100644 (file)
 
 namespace WebCore {
 
-template<typename K, typename M>
-static void cleanWeakMap(HashMap<K, WeakPtr<M> >& map)
-{
-    HashMap<K, WeakPtr<M> > other;
-    other.swap(map);
-
-    typename HashMap<K, WeakPtr<M> >::const_iterator iter = other.begin();
-    while (iter != other.end()) {
-        if (iter->value.get())
-            map.set(iter->key, iter->value);
-        ++iter;
-    }
-}
-
 static String computeFileIdentifier(SecurityOrigin* securityOrigin)
 {
     static const char levelDBFileSuffix[] = "@1";
@@ -80,6 +66,18 @@ void IDBFactoryBackendImpl::removeIDBDatabaseBackend(const String& uniqueIdentif
     m_databaseBackendMap.remove(uniqueIdentifier);
 }
 
+void IDBFactoryBackendImpl::addIDBBackingStore(const String& fileIdentifier, IDBBackingStore* backingStore)
+{
+    ASSERT(!m_backingStoreMap.contains(fileIdentifier));
+    m_backingStoreMap.set(fileIdentifier, backingStore);
+}
+
+void IDBFactoryBackendImpl::removeIDBBackingStore(const String& fileIdentifier)
+{
+    ASSERT(m_backingStoreMap.contains(fileIdentifier));
+    m_backingStoreMap.remove(fileIdentifier);
+}
+
 void IDBFactoryBackendImpl::getDatabaseNames(PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<SecurityOrigin> securityOrigin, ScriptExecutionContext*, const String& dataDirectory)
 {
     RefPtr<IDBBackingStore> backingStore = openBackingStore(securityOrigin, dataDirectory);
@@ -129,18 +127,16 @@ PassRefPtr<IDBBackingStore> IDBFactoryBackendImpl::openBackingStore(PassRefPtr<S
 {
     const String fileIdentifier = computeFileIdentifier(securityOrigin.get());
 
+    RefPtr<IDBBackingStore> backingStore;
     IDBBackingStoreMap::iterator it2 = m_backingStoreMap.find(fileIdentifier);
-    if (it2 != m_backingStoreMap.end()) {
-        if (it2->value.get())
-            return it2->value.get();
+    if (it2 != m_backingStoreMap.end())
+        backingStore = it2->value;
+    else {
+        backingStore = IDBBackingStore::open(securityOrigin.get(), dataDirectory, fileIdentifier, this);
     }
 
-    RefPtr<IDBBackingStore> backingStore = IDBBackingStore::open(securityOrigin.get(), dataDirectory, fileIdentifier);
-    if (backingStore) {
-        cleanWeakMap(m_backingStoreMap);
-        m_backingStoreMap.set(fileIdentifier, backingStore->createWeakPtr());
+    if (backingStore)
         return backingStore.release();
-    }
 
     return 0;
 }
index 8c8a86a..24f73ae 100644 (file)
@@ -33,8 +33,6 @@
 #include "IDBFactoryBackendInterface.h"
 #include "SecurityOrigin.h"
 #include <wtf/HashMap.h>
-#include <wtf/RefCounted.h>
-#include <wtf/WeakPtr.h>
 #include <wtf/text/StringHash.h>
 
 #if ENABLE(INDEXED_DATABASE)
@@ -56,6 +54,8 @@ public:
 
     // Notifications from weak pointers.
     virtual void removeIDBDatabaseBackend(const String& uniqueIdentifier);
+    void addIDBBackingStore(const String& fileIdentifier, IDBBackingStore*);
+    virtual void removeIDBBackingStore(const String& fileIdentifier);
 
     virtual void getDatabaseNames(PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir);
     virtual void open(const String& name, int64_t version, int64_t transactionId, PassRefPtr<IDBCallbacks>, PassRefPtr<IDBDatabaseCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir);
@@ -70,7 +70,7 @@ private:
     typedef HashMap<String, RefPtr<IDBDatabaseBackendImpl> > IDBDatabaseBackendMap;
     IDBDatabaseBackendMap m_databaseBackendMap;
 
-    typedef HashMap<String, WeakPtr<IDBBackingStore> > IDBBackingStoreMap;
+    typedef HashMap<String, IDBBackingStore*> IDBBackingStoreMap;
     IDBBackingStoreMap m_backingStoreMap;
 
     // Only one instance of the factory should exist at any given time.
index 6e16b7f..c7fc075 100644 (file)
@@ -1,3 +1,19 @@
+2013-03-07  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r145166.
+        http://trac.webkit.org/changeset/145166
+        https://bugs.webkit.org/show_bug.cgi?id=111819
+
+        build break - no symbol
+        webkit_support::CreateScopedTempDirectory() (Requested by
+        hayato on #webkit).
+
+        * tests/IDBBackingStoreTest.cpp:
+        (WebCore::IDBBackingStoreTest::SetUp):
+        (IDBBackingStoreTest):
+        * tests/IDBCleanupOnIOErrorTest.cpp:
+        (WebCore::TEST):
+
 2013-03-07  Joshua Bell  <jsbell@chromium.org>
 
         IndexedDB: Use WeakPtr for Factory-to-BackingStore reference
index 5302e12..aa4cdc9 100644 (file)
 
 #include "IDBBackingStore.h"
 
-#include "IDBFactoryBackendImpl.h"
 #include "SharedBuffer.h"
 
 #include <gtest/gtest.h>
-#include <webkit/support/webkit_support.h>
 
 #if ENABLE(INDEXED_DATABASE)
 
@@ -45,10 +43,11 @@ public:
     void SetUp()
     {
         SecurityOrigin* securityOrigin = 0;
+        m_factory = 0;
         // Empty pathBase means an in-memory database.
         String pathBase;
         String fileIdentifier;
-        m_backingStore = IDBBackingStore::open(securityOrigin, pathBase, fileIdentifier);
+        m_backingStore = IDBBackingStore::open(securityOrigin, pathBase, fileIdentifier, m_factory);
 
         // useful keys and values during tests
         const char rawValue1[] = "value1";
@@ -60,6 +59,7 @@ public:
     }
 
 protected:
+    IDBFactoryBackendImpl* m_factory;
     RefPtr<IDBBackingStore> m_backingStore;
 
     // Sample keys and values that are consistent.
@@ -183,42 +183,6 @@ TEST_F(IDBBackingStoreTest, CreateDatabase)
     }
 }
 
-class MockIDBFactoryBackend : public IDBFactoryBackendImpl {
-public:
-    static PassRefPtr<MockIDBFactoryBackend> create()
-    {
-        return adoptRef(new MockIDBFactoryBackend());
-    }
-
-    PassRefPtr<IDBBackingStore> testOpenBackingStore(PassRefPtr<SecurityOrigin> origin, const String& dataDirectory)
-    {
-        return openBackingStore(origin, dataDirectory);
-    }
-};
-
-TEST(IDBFactoryBackendTest, BackingStoreLifetime)
-{
-    RefPtr<SecurityOrigin> origin1 = SecurityOrigin::create("http", "localhost", 81);
-    RefPtr<SecurityOrigin> origin2 = SecurityOrigin::create("http", "localhost", 82);
-
-    RefPtr<MockIDBFactoryBackend> factory = MockIDBFactoryBackend::create();
-
-    OwnPtr<webkit_support::ScopedTempDirectory> tempDirectory = adoptPtr(webkit_support::CreateScopedTempDirectory());
-    tempDirectory->CreateUniqueTempDir();
-    const String path = String::fromUTF8(tempDirectory->path().c_str());
-
-    RefPtr<IDBBackingStore> diskStore1 = factory->testOpenBackingStore(origin1, path);
-    EXPECT_TRUE(diskStore1->hasOneRef());
-
-    RefPtr<IDBBackingStore> diskStore2 = factory->testOpenBackingStore(origin1, path);
-    EXPECT_EQ(diskStore2.get(), diskStore1.get());
-    EXPECT_EQ(diskStore2->refCount(), 2);
-
-    RefPtr<IDBBackingStore> diskStore3 = factory->testOpenBackingStore(origin2, path);
-    EXPECT_TRUE(diskStore3->hasOneRef());
-    EXPECT_EQ(diskStore1->refCount(), 2);
-}
-
 } // namespace
 
 #endif // ENABLE(INDEXED_DATABASE)
index 743e5c8..90dccd6 100644 (file)
@@ -77,8 +77,9 @@ TEST(IDBIOErrorTest, CleanUpTest)
     tempDirectory->CreateUniqueTempDir();
     const String path = String::fromUTF8(tempDirectory->path().c_str());
     String dummyFileIdentifier;
+    IDBFactoryBackendImpl* dummyIDBFactory = 0;
     MockLevelDBFactory mockLevelDBFactory;
-    RefPtr<IDBBackingStore> backingStore = IDBBackingStore::open(origin.get(), path, dummyFileIdentifier, &mockLevelDBFactory);
+    RefPtr<IDBBackingStore> backingStore = IDBBackingStore::open(origin.get(), path, dummyFileIdentifier, dummyIDBFactory, &mockLevelDBFactory);
 }
 
 } // namespace