IndexedDB: Consolidate two-phase connection to avoid race conditions
authorjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Sep 2012 00:28:38 +0000 (00:28 +0000)
committerjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Sep 2012 00:28:38 +0000 (00:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90411

Reviewed by Tony Chang.

Source/WebCore:

Previously, IDB connections were opened by having the front-end (1) call through to
a back-end open() method, eventually receive a success message with a back-end object
handle, and (2) call into the back-end object to register front-end callbacks. This left
the back-end's notion of an open connection in a limbo state between these two calls.
In multi-process ports, a crash of the front-end process could leave the back-end wedged
waiting for this second call (e.g. can't delete until all connections are closed).

Simplify this by having the front-end pass through the callbacks into the back-end
during the initial open() call, which eliminates the limbo state.

No new tests - no functional changes. Chromium port's webkit_unit_tests updated.

* Modules/indexeddb/IDBDatabase.cpp:
(WebCore::IDBDatabase::create):
(WebCore::IDBDatabase::IDBDatabase): Db-callbacks is available at creation time.
(WebCore::IDBDatabase::~IDBDatabase):
* Modules/indexeddb/IDBDatabase.h:
(IDBDatabase):
* Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
(WebCore::IDBDatabaseBackendImpl::PendingOpenCall::create): Need to track db-callbacks as well.
(WebCore::IDBDatabaseBackendImpl::PendingOpenCall::databaseCallbacks):
(WebCore::IDBDatabaseBackendImpl::PendingOpenCall::PendingOpenCall):
(IDBDatabaseBackendImpl::PendingOpenCall):
(WebCore::IDBDatabaseBackendImpl::PendingOpenWithVersionCall::create): Ditto.
(WebCore::IDBDatabaseBackendImpl::PendingOpenWithVersionCall::databaseCallbacks):
(WebCore::IDBDatabaseBackendImpl::PendingOpenWithVersionCall::PendingOpenWithVersionCall):
(IDBDatabaseBackendImpl::PendingOpenWithVersionCall):
(WebCore::IDBDatabaseBackendImpl::IDBDatabaseBackendImpl):
(WebCore::IDBDatabaseBackendImpl::setVersion):
(WebCore::IDBDatabaseBackendImpl::connectionCount): Don't need to count limbo connections any more.
(WebCore::IDBDatabaseBackendImpl::processPendingCalls): Pass through db-callbacks.
(WebCore::IDBDatabaseBackendImpl::openConnection): No more limbo connections (yay!).
(WebCore::IDBDatabaseBackendImpl::runIntVersionChangeTransaction): Pass through db-callbacks.
(WebCore::IDBDatabaseBackendImpl::openConnectionWithVersion): Ditto.
(WebCore::IDBDatabaseBackendImpl::deleteDatabase): Style.
(WebCore::IDBDatabaseBackendImpl::close): Resolve FIXME about connectionCount.
* Modules/indexeddb/IDBDatabaseBackendImpl.h:
(IDBDatabaseBackendImpl):
* Modules/indexeddb/IDBDatabaseBackendInterface.h:
(IDBDatabaseBackendInterface):
* Modules/indexeddb/IDBDatabaseCallbacksImpl.cpp:
(WebCore::IDBDatabaseCallbacksImpl::create):
(WebCore::IDBDatabaseCallbacksImpl::IDBDatabaseCallbacksImpl):
(WebCore::IDBDatabaseCallbacksImpl::connect):
* Modules/indexeddb/IDBDatabaseCallbacksImpl.h:
(IDBDatabaseCallbacksImpl):
* Modules/indexeddb/IDBFactory.cpp:
(WebCore::IDBFactory::open): Mint the db-callbacks here...
* Modules/indexeddb/IDBFactoryBackendImpl.cpp:
(WebCore::IDBFactoryBackendImpl::open): ...passed through to here...
* Modules/indexeddb/IDBFactoryBackendImpl.h:
(IDBFactoryBackendImpl):
* Modules/indexeddb/IDBFactoryBackendInterface.h:
(IDBFactoryBackendInterface):
* Modules/indexeddb/IDBOpenDBRequest.cpp:
(WebCore::IDBOpenDBRequest::create): ...all the way to here...
(WebCore::IDBOpenDBRequest::IDBOpenDBRequest):
(WebCore::IDBOpenDBRequest::onUpgradeNeeded): ...and finally hooked up here.
(WebCore::IDBOpenDBRequest::onSuccess): (or here, if no upgrade needed).
* Modules/indexeddb/IDBOpenDBRequest.h:
(WebCore):
(IDBOpenDBRequest):
* Modules/indexeddb/IDBTransaction.cpp:
(WebCore::IDBTransaction::onAbort): Tweak event/notification ordering; the
notifying the database that the transaction is finished may unblock closing,
which fires more events, and the delivery matters. Previously the close would
be blocked by the transaction which gave the desired order.
(WebCore::IDBTransaction::onComplete): Ditto.
* inspector/InspectorIndexedDBAgent.cpp: New hookup logic.
(WebCore):

Source/WebKit/chromium:

API plumbing for simplified single-phase connection opening, and tests updated
to exercise the new APIs.

* public/WebIDBDatabase.h:
(WebIDBDatabase): Just a FIXME to remove the old second-phase hookup API.
* public/WebIDBFactory.h:
(WebKit):
(WebIDBFactory):
(WebKit::WebIDBFactory::open): New overload that takes db-callbacks.
* src/IDBCallbacksProxy.cpp: The db-callbacks plumbing is needed earlier.
(WebKit::IDBCallbacksProxy::onSuccess):
(WebKit::IDBCallbacksProxy::onUpgradeNeeded):
(WebKit):
(WebKit::IDBCallbacksProxy::setDatabaseCallbacks): Needs to hold on to
the db-callbacks and hook it up when the onSuccess callback comes through.
* src/IDBCallbacksProxy.h:
(WebKit):
(IDBCallbacksProxy):
* src/IDBDatabaseBackendProxy.cpp:
* src/IDBDatabaseBackendProxy.h:
(IDBDatabaseBackendProxy):
* src/IDBFactoryBackendProxy.cpp:
(WebKit::IDBFactoryBackendProxy::open):
* src/IDBFactoryBackendProxy.h:
(IDBFactoryBackendProxy):
* src/WebIDBDatabaseImpl.cpp:
(WebKit::WebIDBDatabaseImpl::WebIDBDatabaseImpl):
(WebKit::WebIDBDatabaseImpl::close):
* src/WebIDBDatabaseImpl.h:
(WebIDBDatabaseImpl):
* src/WebIDBFactoryImpl.cpp:
(WebKit::WebIDBFactoryImpl::open):
* src/WebIDBFactoryImpl.h:
(WebIDBFactoryImpl):
* tests/IDBAbortOnCorruptTest.cpp:
(FakeIDBDatabaseCallbacks):
(WebCore::FakeIDBDatabaseCallbacks::create):
(WebCore::FakeIDBDatabaseCallbacks::~FakeIDBDatabaseCallbacks):
(WebCore::FakeIDBDatabaseCallbacks::FakeIDBDatabaseCallbacks):
(WebCore):
(WebCore::TEST): Updated connection sequence.
* tests/IDBDatabaseBackendTest.cpp: Ditto.

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

31 files changed:
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBDatabase.cpp
Source/WebCore/Modules/indexeddb/IDBDatabase.h
Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp
Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h
Source/WebCore/Modules/indexeddb/IDBDatabaseBackendInterface.h
Source/WebCore/Modules/indexeddb/IDBDatabaseCallbacksImpl.cpp
Source/WebCore/Modules/indexeddb/IDBDatabaseCallbacksImpl.h
Source/WebCore/Modules/indexeddb/IDBFactory.cpp
Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp
Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.h
Source/WebCore/Modules/indexeddb/IDBFactoryBackendInterface.h
Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp
Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.h
Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
Source/WebCore/inspector/InspectorIndexedDBAgent.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/public/WebIDBDatabase.h
Source/WebKit/chromium/public/WebIDBFactory.h
Source/WebKit/chromium/src/IDBCallbacksProxy.cpp
Source/WebKit/chromium/src/IDBCallbacksProxy.h
Source/WebKit/chromium/src/IDBDatabaseBackendProxy.cpp
Source/WebKit/chromium/src/IDBDatabaseBackendProxy.h
Source/WebKit/chromium/src/IDBFactoryBackendProxy.cpp
Source/WebKit/chromium/src/IDBFactoryBackendProxy.h
Source/WebKit/chromium/src/WebIDBDatabaseImpl.cpp
Source/WebKit/chromium/src/WebIDBDatabaseImpl.h
Source/WebKit/chromium/src/WebIDBFactoryImpl.cpp
Source/WebKit/chromium/src/WebIDBFactoryImpl.h
Source/WebKit/chromium/tests/IDBAbortOnCorruptTest.cpp
Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp

index 7c285bb..26ac376 100644 (file)
@@ -1,3 +1,81 @@
+2012-09-13  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: Consolidate two-phase connection to avoid race conditions
+        https://bugs.webkit.org/show_bug.cgi?id=90411
+
+        Reviewed by Tony Chang.
+
+        Previously, IDB connections were opened by having the front-end (1) call through to
+        a back-end open() method, eventually receive a success message with a back-end object
+        handle, and (2) call into the back-end object to register front-end callbacks. This left
+        the back-end's notion of an open connection in a limbo state between these two calls.
+        In multi-process ports, a crash of the front-end process could leave the back-end wedged
+        waiting for this second call (e.g. can't delete until all connections are closed).
+
+        Simplify this by having the front-end pass through the callbacks into the back-end
+        during the initial open() call, which eliminates the limbo state.
+
+        No new tests - no functional changes. Chromium port's webkit_unit_tests updated.
+
+        * Modules/indexeddb/IDBDatabase.cpp:
+        (WebCore::IDBDatabase::create):
+        (WebCore::IDBDatabase::IDBDatabase): Db-callbacks is available at creation time.
+        (WebCore::IDBDatabase::~IDBDatabase):
+        * Modules/indexeddb/IDBDatabase.h:
+        (IDBDatabase):
+        * Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
+        (WebCore::IDBDatabaseBackendImpl::PendingOpenCall::create): Need to track db-callbacks as well.
+        (WebCore::IDBDatabaseBackendImpl::PendingOpenCall::databaseCallbacks):
+        (WebCore::IDBDatabaseBackendImpl::PendingOpenCall::PendingOpenCall):
+        (IDBDatabaseBackendImpl::PendingOpenCall):
+        (WebCore::IDBDatabaseBackendImpl::PendingOpenWithVersionCall::create): Ditto.
+        (WebCore::IDBDatabaseBackendImpl::PendingOpenWithVersionCall::databaseCallbacks):
+        (WebCore::IDBDatabaseBackendImpl::PendingOpenWithVersionCall::PendingOpenWithVersionCall):
+        (IDBDatabaseBackendImpl::PendingOpenWithVersionCall):
+        (WebCore::IDBDatabaseBackendImpl::IDBDatabaseBackendImpl):
+        (WebCore::IDBDatabaseBackendImpl::setVersion):
+        (WebCore::IDBDatabaseBackendImpl::connectionCount): Don't need to count limbo connections any more.
+        (WebCore::IDBDatabaseBackendImpl::processPendingCalls): Pass through db-callbacks.
+        (WebCore::IDBDatabaseBackendImpl::openConnection): No more limbo connections (yay!).
+        (WebCore::IDBDatabaseBackendImpl::runIntVersionChangeTransaction): Pass through db-callbacks.
+        (WebCore::IDBDatabaseBackendImpl::openConnectionWithVersion): Ditto.
+        (WebCore::IDBDatabaseBackendImpl::deleteDatabase): Style.
+        (WebCore::IDBDatabaseBackendImpl::close): Resolve FIXME about connectionCount.
+        * Modules/indexeddb/IDBDatabaseBackendImpl.h:
+        (IDBDatabaseBackendImpl):
+        * Modules/indexeddb/IDBDatabaseBackendInterface.h:
+        (IDBDatabaseBackendInterface):
+        * Modules/indexeddb/IDBDatabaseCallbacksImpl.cpp:
+        (WebCore::IDBDatabaseCallbacksImpl::create):
+        (WebCore::IDBDatabaseCallbacksImpl::IDBDatabaseCallbacksImpl):
+        (WebCore::IDBDatabaseCallbacksImpl::connect):
+        * Modules/indexeddb/IDBDatabaseCallbacksImpl.h:
+        (IDBDatabaseCallbacksImpl):
+        * Modules/indexeddb/IDBFactory.cpp:
+        (WebCore::IDBFactory::open): Mint the db-callbacks here...
+        * Modules/indexeddb/IDBFactoryBackendImpl.cpp:
+        (WebCore::IDBFactoryBackendImpl::open): ...passed through to here...
+        * Modules/indexeddb/IDBFactoryBackendImpl.h:
+        (IDBFactoryBackendImpl):
+        * Modules/indexeddb/IDBFactoryBackendInterface.h:
+        (IDBFactoryBackendInterface):
+        * Modules/indexeddb/IDBOpenDBRequest.cpp:
+        (WebCore::IDBOpenDBRequest::create): ...all the way to here...
+        (WebCore::IDBOpenDBRequest::IDBOpenDBRequest):
+        (WebCore::IDBOpenDBRequest::onUpgradeNeeded): ...and finally hooked up here.
+        (WebCore::IDBOpenDBRequest::onSuccess): (or here, if no upgrade needed).
+        * Modules/indexeddb/IDBOpenDBRequest.h:
+        (WebCore):
+        (IDBOpenDBRequest):
+        * Modules/indexeddb/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::onAbort): Tweak event/notification ordering; the
+        notifying the database that the transaction is finished may unblock closing,
+        which fires more events, and the delivery matters. Previously the close would
+        be blocked by the transaction which gave the desired order.
+        (WebCore::IDBTransaction::onComplete): Ditto.
+        * inspector/InspectorIndexedDBAgent.cpp: New hookup logic.
+        (WebCore):
+
 2012-09-13  Ryosuke Niwa  <rniwa@webkit.org>
 
         suspend/resumeWidgetHierarchyUpdates should be a RAII object
index b3f70ad..d641d04 100644 (file)
@@ -31,7 +31,7 @@
 #include "EventQueue.h"
 #include "ExceptionCode.h"
 #include "IDBAny.h"
-#include "IDBDatabaseCallbacksImpl.h"
+#include "IDBDatabaseCallbacks.h"
 #include "IDBDatabaseError.h"
 #include "IDBDatabaseException.h"
 #include "IDBEventDispatcher.h"
 
 namespace WebCore {
 
-PassRefPtr<IDBDatabase> IDBDatabase::create(ScriptExecutionContext* context, PassRefPtr<IDBDatabaseBackendInterface> database)
+PassRefPtr<IDBDatabase> IDBDatabase::create(ScriptExecutionContext* context, PassRefPtr<IDBDatabaseBackendInterface> database, PassRefPtr<IDBDatabaseCallbacks> callbacks)
 {
-    RefPtr<IDBDatabase> idbDatabase(adoptRef(new IDBDatabase(context, database)));
+    RefPtr<IDBDatabase> idbDatabase(adoptRef(new IDBDatabase(context, database, callbacks)));
     idbDatabase->suspendIfNeeded();
     return idbDatabase.release();
 }
 
-IDBDatabase::IDBDatabase(ScriptExecutionContext* context, PassRefPtr<IDBDatabaseBackendInterface> backend)
+IDBDatabase::IDBDatabase(ScriptExecutionContext* context, PassRefPtr<IDBDatabaseBackendInterface> backend, PassRefPtr<IDBDatabaseCallbacks> callbacks)
     : ActiveDOMObject(context, this)
     , m_backend(backend)
     , m_closePending(false)
     , m_contextStopped(false)
+    , m_databaseCallbacks(callbacks)
 {
     // We pass a reference of this object before it can be adopted.
     relaxAdoptionRequirement();
-    m_databaseCallbacks = IDBDatabaseCallbacksImpl::create(this);
     m_metadata = m_backend->metadata();
 }
 
 IDBDatabase::~IDBDatabase()
 {
     close();
-    m_databaseCallbacks->unregisterDatabase(this);
 }
 
 void IDBDatabase::transactionCreated(IDBTransaction* transaction)
@@ -335,12 +334,6 @@ void IDBDatabase::onVersionChange(const String& version)
     enqueueEvent(IDBVersionChangeEvent::create(version, eventNames().versionchangeEvent));
 }
 
-void IDBDatabase::registerFrontendCallbacks()
-{
-    ASSERT(m_backend);
-    m_backend->registerFrontendCallbacks(m_databaseCallbacks);
-}
-
 void IDBDatabase::enqueueEvent(PassRefPtr<Event> event)
 {
     ASSERT(!m_contextStopped);
index 27f03b6..4deb27a 100644 (file)
@@ -32,7 +32,7 @@
 #include "Event.h"
 #include "EventTarget.h"
 #include "IDBDatabaseBackendInterface.h"
-#include "IDBDatabaseCallbacksImpl.h"
+#include "IDBDatabaseCallbacks.h"
 #include "IDBMetadata.h"
 #include "IDBObjectStore.h"
 #include "IDBTransaction.h"
@@ -51,7 +51,7 @@ typedef int ExceptionCode;
 
 class IDBDatabase : public RefCounted<IDBDatabase>, public EventTarget, public ActiveDOMObject {
 public:
-    static PassRefPtr<IDBDatabase> create(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendInterface>);
+    static PassRefPtr<IDBDatabase> create(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendInterface>, PassRefPtr<IDBDatabaseCallbacks>);
     ~IDBDatabase();
 
     void transactionCreated(IDBTransaction*);
@@ -89,7 +89,6 @@ public:
     virtual ScriptExecutionContext* scriptExecutionContext() const;
 
     void forceClose();
-    void registerFrontendCallbacks();
     const IDBDatabaseMetadata metadata() const { return m_metadata; }
     void enqueueEvent(PassRefPtr<Event>);
     bool dispatchEvent(PassRefPtr<Event> event, ExceptionCode& ec) { return EventTarget::dispatchEvent(event, ec); }
@@ -99,7 +98,7 @@ public:
     using RefCounted<IDBDatabase>::deref;
 
 private:
-    IDBDatabase(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendInterface>);
+    IDBDatabase(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendInterface>, PassRefPtr<IDBDatabaseCallbacks>);
 
     // EventTarget
     virtual void refEventTarget() { ref(); }
@@ -123,7 +122,7 @@ private:
     // database so that we can cancel them if the database closes.
     Vector<RefPtr<Event> > m_enqueuedEvents;
 
-    RefPtr<IDBDatabaseCallbacksImpl> m_databaseCallbacks;
+    RefPtr<IDBDatabaseCallbacks> m_databaseCallbacks;
 };
 
 } // namespace WebCore
index 8110c24..bfb2ae2 100644 (file)
@@ -41,37 +41,43 @@ namespace WebCore {
 
 class IDBDatabaseBackendImpl::PendingOpenCall : public RefCounted<PendingOpenCall> {
 public:
-    static PassRefPtr<PendingOpenCall> create(PassRefPtr<IDBCallbacks> callbacks)
+    static PassRefPtr<PendingOpenCall> create(PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks)
     {
-        return adoptRef(new PendingOpenCall(callbacks));
+        return adoptRef(new PendingOpenCall(callbacks, databaseCallbacks));
     }
     PassRefPtr<IDBCallbacks> callbacks() { return m_callbacks; }
+    PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks() { return m_databaseCallbacks; }
 
 private:
-    PendingOpenCall(PassRefPtr<IDBCallbacks> callbacks)
+    PendingOpenCall(PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks)
         : m_callbacks(callbacks)
+        , m_databaseCallbacks(databaseCallbacks)
     {
     }
 
     RefPtr<IDBCallbacks> m_callbacks;
+    RefPtr<IDBDatabaseCallbacks> m_databaseCallbacks;
 };
 
 class IDBDatabaseBackendImpl::PendingOpenWithVersionCall : public RefCounted<PendingOpenWithVersionCall> {
 public:
-    static PassRefPtr<PendingOpenWithVersionCall> create(PassRefPtr<IDBCallbacks> callbacks, int64_t version)
+    static PassRefPtr<PendingOpenWithVersionCall> create(PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks, int64_t version)
     {
-        return adoptRef(new PendingOpenWithVersionCall(callbacks, version));
+        return adoptRef(new PendingOpenWithVersionCall(callbacks, databaseCallbacks, version));
     }
     PassRefPtr<IDBCallbacks> callbacks() { return m_callbacks; }
+    PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks() { return m_databaseCallbacks; }
     int64_t version() { return m_version; }
 
 private:
-    PendingOpenWithVersionCall(PassRefPtr<IDBCallbacks> callbacks, int64_t version)
+    PendingOpenWithVersionCall(PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks, int64_t version)
         : m_callbacks(callbacks)
+        , m_databaseCallbacks(databaseCallbacks)
         , m_version(version)
     {
     }
     RefPtr<IDBCallbacks> m_callbacks;
+    RefPtr<IDBDatabaseCallbacks> m_databaseCallbacks;
     int64_t m_version;
 };
 
@@ -130,7 +136,6 @@ IDBDatabaseBackendImpl::IDBDatabaseBackendImpl(const String& name, IDBBackingSto
     , m_identifier(uniqueIdentifier)
     , m_factory(factory)
     , m_transactionCoordinator(coordinator)
-    , m_pendingConnectionCount(0)
 {
     ASSERT(!m_name.isNull());
 }
@@ -240,6 +245,7 @@ void IDBDatabaseBackendImpl::setVersion(const String& version, PassRefPtr<IDBCal
         return;
     }
     for (DatabaseCallbacksSet::const_iterator it = m_databaseCallbacksSet.begin(); it != m_databaseCallbacksSet.end(); ++it) {
+        // Front end ensures the event is not fired at connections that have closePending set.
         if (*it != databaseCallbacks)
             (*it)->onVersionChange(version);
     }
@@ -342,9 +348,10 @@ void IDBDatabaseBackendImpl::transactionFinishedAndCompleteFired(PassRefPtr<IDBT
         processPendingCalls();
 }
 
-int32_t IDBDatabaseBackendImpl::connectionCount()
+size_t IDBDatabaseBackendImpl::connectionCount()
 {
-    return m_databaseCallbacksSet.size() + m_pendingConnectionCount;
+    // This does not include pending open calls, as those should not block version changes and deletes.
+    return m_databaseCallbacksSet.size();
 }
 
 void IDBDatabaseBackendImpl::processPendingCalls()
@@ -355,9 +362,8 @@ void IDBDatabaseBackendImpl::processPendingCalls()
         RefPtr<PendingOpenWithVersionCall> pendingOpenWithVersionCall = m_pendingSecondHalfOpenWithVersionCalls.takeFirst();
         ASSERT(pendingOpenWithVersionCall->version() == m_intVersion);
         ASSERT(m_id != InvalidId);
-        ++m_pendingConnectionCount;
         pendingOpenWithVersionCall->callbacks()->onSuccess(this);
-        return;
+        // Fall through when complete, as pending deletes may be (partially) unblocked.
     }
 
     // Pending calls may be requeued or aborted
@@ -400,7 +406,7 @@ void IDBDatabaseBackendImpl::processPendingCalls()
     m_pendingOpenWithVersionCalls.swap(pendingOpenWithVersionCalls);
     while (!pendingOpenWithVersionCalls.isEmpty()) {
         RefPtr<PendingOpenWithVersionCall> pendingOpenWithVersionCall = pendingOpenWithVersionCalls.takeFirst();
-        openConnectionWithVersion(pendingOpenWithVersionCall->callbacks(), pendingOpenWithVersionCall->version());
+        openConnectionWithVersion(pendingOpenWithVersionCall->callbacks(), pendingOpenWithVersionCall->databaseCallbacks(), pendingOpenWithVersionCall->version());
     }
 
     // Given the check above, it appears that calls cannot be requeued by
@@ -409,7 +415,7 @@ void IDBDatabaseBackendImpl::processPendingCalls()
     m_pendingOpenCalls.swap(pendingOpenCalls);
     while (!pendingOpenCalls.isEmpty()) {
         RefPtr<PendingOpenCall> pendingOpenCall = pendingOpenCalls.takeFirst();
-        openConnection(pendingOpenCall->callbacks());
+        openConnection(pendingOpenCall->callbacks(), pendingOpenCall->databaseCallbacks());
     }
     ASSERT(m_pendingOpenCalls.isEmpty());
 }
@@ -428,44 +434,30 @@ PassRefPtr<IDBTransactionBackendInterface> IDBDatabaseBackendImpl::transaction(D
     return transaction.release();
 }
 
-void IDBDatabaseBackendImpl::registerFrontendCallbacks(PassRefPtr<IDBDatabaseCallbacks> callbacks)
-{
-    ASSERT(m_backingStore.get());
-    ASSERT(m_pendingConnectionCount);
-    --m_pendingConnectionCount;
-    m_databaseCallbacksSet.add(RefPtr<IDBDatabaseCallbacks>(callbacks));
-    // We give max priority to open calls that follow upgradeneeded
-    // events; trigger the rest of the queues to be serviced when those open
-    // calls are finished.
-    processPendingCalls();
-}
-
-void IDBDatabaseBackendImpl::openConnection(PassRefPtr<IDBCallbacks> callbacks)
+void IDBDatabaseBackendImpl::openConnection(PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks)
 {
     ASSERT(m_backingStore.get());
     if (!m_pendingDeleteCalls.isEmpty() || m_runningVersionChangeTransaction || !m_pendingSetVersionCalls.isEmpty())
-        m_pendingOpenCalls.append(PendingOpenCall::create(callbacks));
+        m_pendingOpenCalls.append(PendingOpenCall::create(callbacks, databaseCallbacks));
     else {
         if (m_id == InvalidId && !openInternal())
             callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error."));
         else {
-            ++m_pendingConnectionCount;
+            m_databaseCallbacksSet.add(RefPtr<IDBDatabaseCallbacks>(databaseCallbacks));
             callbacks->onSuccess(this);
         }
     }
 }
 
-void IDBDatabaseBackendImpl::runIntVersionChangeTransaction(int64_t requestedVersion, PassRefPtr<IDBCallbacks> prpCallbacks)
+void IDBDatabaseBackendImpl::runIntVersionChangeTransaction(int64_t requestedVersion, PassRefPtr<IDBCallbacks> prpCallbacks, PassRefPtr<IDBDatabaseCallbacks> prpDatabaseCallbacks)
 {
     RefPtr<IDBCallbacks> callbacks = prpCallbacks;
+    RefPtr<IDBDatabaseCallbacks> databaseCallbacks = prpDatabaseCallbacks;
     ASSERT(callbacks);
     for (DatabaseCallbacksSet::const_iterator it = m_databaseCallbacksSet.begin(); it != m_databaseCallbacksSet.end(); ++it) {
-        // Note that some connections might close in the versionchange event
-        // handler for some other connection, after which its own versionchange
-        // event should not be fired. The backend doesn't worry about this, we
-        // just queue up a version change event for every connection. The
-        // frontend takes care to only dispatch to open connections.
-        (*it)->onVersionChange(m_intVersion, requestedVersion);
+        // Front end ensures the event is not fired at connections that have closePending set.
+        if (*it != databaseCallbacks)
+            (*it)->onVersionChange(m_intVersion, requestedVersion);
     }
     // The spec dictates we wait until all the version change events are
     // delivered and then check m_databaseCallbacks.empty() before proceeding
@@ -474,11 +466,11 @@ void IDBDatabaseBackendImpl::runIntVersionChangeTransaction(int64_t requestedVer
     // FIXME: Remove the call to onBlocked and instead wait until the frontend
     // tells us that all the blocked events have been delivered. See
     // https://bugs.webkit.org/show_bug.cgi?id=71130
-    if (connectionCount() > 0)
+    if (connectionCount())
         callbacks->onBlocked(m_intVersion);
     // FIXME: Add test for m_runningVersionChangeTransaction.
-    if (m_runningVersionChangeTransaction || connectionCount() > 0) {
-        m_pendingOpenWithVersionCalls.append(PendingOpenWithVersionCall::create(callbacks, requestedVersion));
+    if (m_runningVersionChangeTransaction || connectionCount()) {
+        m_pendingOpenWithVersionCalls.append(PendingOpenWithVersionCall::create(callbacks, databaseCallbacks, requestedVersion));
         return;
     }
 
@@ -497,14 +489,16 @@ void IDBDatabaseBackendImpl::runIntVersionChangeTransaction(int64_t requestedVer
         ec = IDBDatabaseException::TRANSACTION_INACTIVE_ERR;
     }
     ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersionCalls.size(), "m_pendingSecondHalfOpenWithVersionCalls.size = %zu", m_pendingSecondHalfOpenWithVersionCalls.size());
-    m_pendingSecondHalfOpenWithVersionCalls.append(PendingOpenWithVersionCall::create(callbacks, requestedVersion));
+    m_pendingSecondHalfOpenWithVersionCalls.append(PendingOpenWithVersionCall::create(callbacks, databaseCallbacks, requestedVersion));
+    m_databaseCallbacksSet.add(databaseCallbacks);
 }
 
-void IDBDatabaseBackendImpl::openConnectionWithVersion(PassRefPtr<IDBCallbacks> prpCallbacks, int64_t version)
+void IDBDatabaseBackendImpl::openConnectionWithVersion(PassRefPtr<IDBCallbacks> prpCallbacks, PassRefPtr<IDBDatabaseCallbacks> prpDatabaseCallbacks, int64_t version)
 {
     RefPtr<IDBCallbacks> callbacks = prpCallbacks;
+    RefPtr<IDBDatabaseCallbacks> databaseCallbacks = prpDatabaseCallbacks;
     if (!m_pendingDeleteCalls.isEmpty() || m_runningVersionChangeTransaction || !m_pendingSetVersionCalls.isEmpty()) {
-        m_pendingOpenWithVersionCalls.append(PendingOpenWithVersionCall::create(callbacks, version));
+        m_pendingOpenWithVersionCalls.append(PendingOpenWithVersionCall::create(callbacks, databaseCallbacks, version));
         return;
     }
     if (m_id == InvalidId) {
@@ -516,7 +510,7 @@ void IDBDatabaseBackendImpl::openConnectionWithVersion(PassRefPtr<IDBCallbacks>
         }
     }
     if (version > m_intVersion) {
-        runIntVersionChangeTransaction(version, callbacks);
+        runIntVersionChangeTransaction(version, callbacks, databaseCallbacks);
         return;
     }
     if (version < m_intVersion) {
@@ -524,7 +518,7 @@ void IDBDatabaseBackendImpl::openConnectionWithVersion(PassRefPtr<IDBCallbacks>
         return;
     }
     ASSERT(version == m_intVersion);
-    ++m_pendingConnectionCount;
+    m_databaseCallbacksSet.add(databaseCallbacks);
     callbacks->onSuccess(this);
 }
 
@@ -535,15 +529,14 @@ void IDBDatabaseBackendImpl::deleteDatabase(PassRefPtr<IDBCallbacks> prpCallback
         return;
     }
     RefPtr<IDBCallbacks> callbacks = prpCallbacks;
-    // FIXME: Only fire onVersionChange if there the connection isn't in
-    // the process of closing.
-    // https://bugs.webkit.org/show_bug.cgi?id=71129
-    for (DatabaseCallbacksSet::const_iterator it = m_databaseCallbacksSet.begin(); it != m_databaseCallbacksSet.end(); ++it)
+    for (DatabaseCallbacksSet::const_iterator it = m_databaseCallbacksSet.begin(); it != m_databaseCallbacksSet.end(); ++it) {
+        // Front end ensures the event is not fired at connections that have closePending set.
         (*it)->onVersionChange("");
+    }
     // FIXME: Only fire onBlocked if there are open connections after the
     // VersionChangeEvents are received, not just set up to fire.
     // https://bugs.webkit.org/show_bug.cgi?id=71130
-    if (connectionCount() >= 1) {
+    if (connectionCount()) {
         m_pendingDeleteCalls.append(PendingDeleteCall::create(callbacks));
         callbacks->onBlocked();
         return;
@@ -564,18 +557,19 @@ void IDBDatabaseBackendImpl::close(PassRefPtr<IDBDatabaseCallbacks> prpCallbacks
 {
     RefPtr<IDBDatabaseCallbacks> callbacks = prpCallbacks;
     ASSERT(m_databaseCallbacksSet.contains(callbacks));
+
     m_databaseCallbacksSet.remove(callbacks);
+    // FIXME: If callbacks is also held in m_pendingSecondHalfOpenWithVersionCalls
+    // it should be removed and onError fired against it.
+
     if (connectionCount() > 1)
         return;
 
-    TransactionSet transactions(m_transactions);
     processPendingCalls();
 
-    ASSERT(m_transactions.size() - transactions.size() <= 1);
-    // FIXME: Instead of relying on transactions.size(), make connectionCount
-    // aware of in-flight upgradeneeded events as well as in-flight success
-    // events.
-    if (!connectionCount() && !m_pendingDeleteCalls.size() && m_transactions.size() == transactions.size()) {
+    // FIXME: Add a test for the m_pendingOpenCalls¬∑and m_pendingOpenWithVersionCalls cases below.
+    if (!connectionCount() && !m_pendingOpenCalls.size() && !m_pendingOpenWithVersionCalls.size() && !m_pendingDeleteCalls.size()) {
+        TransactionSet transactions(m_transactions);
         for (TransactionSet::const_iterator it = transactions.begin(); it != transactions.end(); ++it)
             (*it)->abort();
 
index 7f31cb0..22ccc43 100644 (file)
@@ -55,9 +55,8 @@ public:
     static const int64_t InvalidId = 0;
     int64_t id() const { return m_id; }
 
-    void registerFrontendCallbacks(PassRefPtr<IDBDatabaseCallbacks>);
-    void openConnection(PassRefPtr<IDBCallbacks>);
-    void openConnectionWithVersion(PassRefPtr<IDBCallbacks>, int64_t version);
+    void openConnection(PassRefPtr<IDBCallbacks>, PassRefPtr<IDBDatabaseCallbacks>);
+    void openConnectionWithVersion(PassRefPtr<IDBCallbacks>, PassRefPtr<IDBDatabaseCallbacks>, int64_t version);
     void deleteDatabase(PassRefPtr<IDBCallbacks>);
 
     // IDBDatabaseBackendInterface
@@ -79,9 +78,9 @@ private:
     IDBDatabaseBackendImpl(const String& name, IDBBackingStore* database, IDBTransactionCoordinator*, IDBFactoryBackendImpl*, const String& uniqueIdentifier);
 
     bool openInternal();
-    void runIntVersionChangeTransaction(int64_t requestedVersion, PassRefPtr<IDBCallbacks>);
+    void runIntVersionChangeTransaction(int64_t requestedVersion, PassRefPtr<IDBCallbacks>, PassRefPtr<IDBDatabaseCallbacks>);
     void loadObjectStores();
-    int32_t connectionCount();
+    size_t connectionCount();
     void processPendingCalls();
 
     static void createObjectStoreInternal(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendImpl>, PassRefPtr<IDBObjectStoreBackendImpl>, PassRefPtr<IDBTransactionBackendImpl>);
@@ -127,10 +126,6 @@ private:
     class PendingDeleteCall;
     Deque<RefPtr<PendingDeleteCall> > m_pendingDeleteCalls;
 
-    // FIXME: Eliminate the limbo state between openConnection() and registerFrontendCallbacks()
-    // that this counter tracks.
-    int32_t m_pendingConnectionCount;
-
     typedef ListHashSet<RefPtr<IDBDatabaseCallbacks> > DatabaseCallbacksSet;
     DatabaseCallbacksSet m_databaseCallbacksSet;
 };
index c7d1d60..cbcb9a5 100644 (file)
@@ -35,7 +35,6 @@
 namespace WebCore {
 
 class DOMStringList;
-class Frame;
 class IDBCallbacks;
 class IDBDatabaseCallbacks;
 class IDBKeyPath;
@@ -60,8 +59,6 @@ public:
     virtual void setVersion(const String& version, PassRefPtr<IDBCallbacks>, PassRefPtr<IDBDatabaseCallbacks>, ExceptionCode&) = 0;
     virtual PassRefPtr<IDBTransactionBackendInterface> transaction(DOMStringList* storeNames, unsigned short mode, ExceptionCode&) = 0;
     virtual void close(PassRefPtr<IDBDatabaseCallbacks>) = 0;
-
-    virtual void registerFrontendCallbacks(PassRefPtr<IDBDatabaseCallbacks>) = 0;
 };
 
 } // namespace WebCore
index f6565dc..970d3a1 100644 (file)
 
 namespace WebCore {
 
-PassRefPtr<IDBDatabaseCallbacksImpl> IDBDatabaseCallbacksImpl::create(IDBDatabase* database)
+PassRefPtr<IDBDatabaseCallbacksImpl> IDBDatabaseCallbacksImpl::create()
 {
-    return adoptRef(new IDBDatabaseCallbacksImpl(database));
+    return adoptRef(new IDBDatabaseCallbacksImpl());
 }
 
-IDBDatabaseCallbacksImpl::IDBDatabaseCallbacksImpl(IDBDatabase* database)
-    : m_database(database)
+IDBDatabaseCallbacksImpl::IDBDatabaseCallbacksImpl()
+    : m_database(0)
 {
 }
 
@@ -64,10 +64,11 @@ void IDBDatabaseCallbacksImpl::onVersionChange(int64_t oldVersion, int64_t newVe
         m_database->onVersionChange(oldVersion, newVersion);
 }
 
-void IDBDatabaseCallbacksImpl::unregisterDatabase(IDBDatabase* database)
+void IDBDatabaseCallbacksImpl::connect(IDBDatabase* database)
 {
-    ASSERT_UNUSED(database, database == m_database);
-    m_database = 0;
+    ASSERT(!m_database);
+    ASSERT(database);
+    m_database = database;
 }
 
 } // namespace WebCore
index 8781dbb..bb4a1ae 100644 (file)
@@ -38,18 +38,20 @@ class IDBDatabase;
 
 class IDBDatabaseCallbacksImpl : public IDBDatabaseCallbacks {
 public:
-    static PassRefPtr<IDBDatabaseCallbacksImpl> create(IDBDatabase*);
+    static PassRefPtr<IDBDatabaseCallbacksImpl> create();
     virtual ~IDBDatabaseCallbacksImpl();
 
+    // IDBDatabaseCallbacks
     virtual void onForcedClose();
     virtual void onVersionChange(const String& version);
     virtual void onVersionChange(int64_t oldVersion, int64_t newVersion);
-    void unregisterDatabase(IDBDatabase*);
+
+    void connect(IDBDatabase*);
 
 private:
-    IDBDatabaseCallbacksImpl(IDBDatabase*);
+    IDBDatabaseCallbacksImpl();
 
-    // m_database has a RefPtr to this, so use a weak pointer to avoid a cycle.
+    // The initial IDBOpenDBRequest or final IDBDatabase maintains a RefPtr to this
     IDBDatabase* m_database;
 };
 
index 4998f6a..f5fccb4 100644 (file)
@@ -37,6 +37,7 @@
 #include "Frame.h"
 #include "GroupSettings.h"
 #include "IDBDatabase.h"
+#include "IDBDatabaseCallbacksImpl.h"
 #include "IDBDatabaseException.h"
 #include "IDBFactoryBackendInterface.h"
 #include "IDBKey.h"
@@ -120,8 +121,9 @@ PassRefPtr<IDBOpenDBRequest> IDBFactory::open(ScriptExecutionContext* context, c
     if (!isContextValid(context))
         return 0;
 
-    RefPtr<IDBOpenDBRequest> request = IDBOpenDBRequest::create(context, IDBAny::createNull(), version);
-    m_backend->open(name, version, request, context->securityOrigin(), context, getIndexedDBDatabasePath(context));
+    RefPtr<IDBDatabaseCallbacksImpl> databaseCallbacks = IDBDatabaseCallbacksImpl::create();
+    RefPtr<IDBOpenDBRequest> request = IDBOpenDBRequest::create(context, IDBAny::createNull(), databaseCallbacks, version);
+    m_backend->open(name, version, request, databaseCallbacks, context->securityOrigin(), context, getIndexedDBDatabasePath(context));
     return request;
 }
 
index 8da5c02..c7ef860 100644 (file)
@@ -150,7 +150,7 @@ PassRefPtr<IDBBackingStore> IDBFactoryBackendImpl::openBackingStore(PassRefPtr<S
     return 0;
 }
 
-void IDBFactoryBackendImpl::open(const String& name, int64_t version, PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<SecurityOrigin> prpSecurityOrigin, ScriptExecutionContext*, const String& dataDirectory)
+void IDBFactoryBackendImpl::open(const String& name, int64_t version, PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks, PassRefPtr<SecurityOrigin> prpSecurityOrigin, ScriptExecutionContext*, const String& dataDirectory)
 {
     RefPtr<SecurityOrigin> securityOrigin = prpSecurityOrigin;
     const String uniqueIdentifier = computeUniqueIdentifier(name, securityOrigin.get());
@@ -175,9 +175,9 @@ void IDBFactoryBackendImpl::open(const String& name, int64_t version, PassRefPtr
         databaseBackend = it->second;
 
     if (version == IDBDatabaseMetadata::NoIntVersion)
-        databaseBackend->openConnection(callbacks);
+        databaseBackend->openConnection(callbacks, databaseCallbacks);
     else
-        databaseBackend->openConnectionWithVersion(callbacks, version);
+        databaseBackend->openConnectionWithVersion(callbacks, databaseCallbacks, version);
 }
 
 } // namespace WebCore
index cad3b5a..3ae0703 100644 (file)
@@ -57,7 +57,7 @@ public:
     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, PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir);
+    virtual void open(const String& name, int64_t version, PassRefPtr<IDBCallbacks>, PassRefPtr<IDBDatabaseCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir);
     virtual void deleteDatabase(const String& name, PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir);
 
 protected:
index f56fc06..bab219f 100644 (file)
 #ifndef IDBFactoryBackendInterface_h
 #define IDBFactoryBackendInterface_h
 
-#include "IDBCallbacks.h"
+#include <wtf/PassRefPtr.h>
 #include <wtf/Threading.h>
-#include <wtf/Vector.h>
 #include <wtf/text/WTFString.h>
 
 #if ENABLE(INDEXED_DATABASE)
 
 namespace WebCore {
 
-class Frame;
+class IDBCallbacks;
 class IDBDatabase;
+class IDBDatabaseCallbacks;
 class SecurityOrigin;
-class WorkerContext;
+class ScriptExecutionContext;
 
 typedef int ExceptionCode;
 
@@ -54,7 +54,7 @@ public:
     virtual ~IDBFactoryBackendInterface() { }
 
     virtual void getDatabaseNames(PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir) = 0;
-    virtual void open(const String& name, int64_t version, PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir) = 0;
+    virtual void open(const String& name, int64_t version, PassRefPtr<IDBCallbacks>, PassRefPtr<IDBDatabaseCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir) = 0;
     virtual void deleteDatabase(const String& name, PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>, ScriptExecutionContext*, const String& dataDir) = 0;
 };
 
index e89a74c..6ef16cf 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(INDEXED_DATABASE)
 
 #include "IDBDatabase.h"
+#include "IDBDatabaseCallbacksImpl.h"
 #include "IDBPendingTransactionMonitor.h"
 #include "IDBTracing.h"
 #include "IDBUpgradeNeededEvent.h"
 
 namespace WebCore {
 
-PassRefPtr<IDBOpenDBRequest> IDBOpenDBRequest::create(ScriptExecutionContext* context, PassRefPtr<IDBAny> source, int64_t version)
+PassRefPtr<IDBOpenDBRequest> IDBOpenDBRequest::create(ScriptExecutionContext* context, PassRefPtr<IDBAny> source, PassRefPtr<IDBDatabaseCallbacksImpl> callbacks, int64_t version)
 {
-    RefPtr<IDBOpenDBRequest> request(adoptRef(new IDBOpenDBRequest(context, source, version)));
+    RefPtr<IDBOpenDBRequest> request(adoptRef(new IDBOpenDBRequest(context, source, callbacks, version)));
     request->suspendIfNeeded();
     return request.release();
 }
 
-IDBOpenDBRequest::IDBOpenDBRequest(ScriptExecutionContext* context, PassRefPtr<IDBAny> source, int64_t version)
+IDBOpenDBRequest::IDBOpenDBRequest(ScriptExecutionContext* context, PassRefPtr<IDBAny> source, PassRefPtr<IDBDatabaseCallbacksImpl> callbacks, int64_t version)
     : IDBRequest(context, source, IDBTransactionBackendInterface::NormalTask, 0)
+    , m_databaseCallbacks(callbacks)
     , m_version(version)
 {
     ASSERT(!m_result);
@@ -72,9 +74,13 @@ void IDBOpenDBRequest::onUpgradeNeeded(int64_t oldVersion, PassRefPtr<IDBTransac
     if (!shouldEnqueueEvent())
         return;
 
+    ASSERT(m_databaseCallbacks);
+
     RefPtr<IDBDatabaseBackendInterface> databaseBackend = prpDatabaseBackend;
     RefPtr<IDBTransactionBackendInterface> transactionBackend = prpTransactionBackend;
-    RefPtr<IDBDatabase> idbDatabase = IDBDatabase::create(scriptExecutionContext(), databaseBackend);
+    RefPtr<IDBDatabase> idbDatabase = IDBDatabase::create(scriptExecutionContext(), databaseBackend, m_databaseCallbacks);
+    m_databaseCallbacks->connect(idbDatabase.get());
+    m_databaseCallbacks = 0;
 
     RefPtr<IDBTransaction> frontend = IDBTransaction::create(scriptExecutionContext(), transactionBackend, IDBTransaction::VERSION_CHANGE, idbDatabase.get(), this);
     transactionBackend->setCallbacks(frontend.get());
@@ -98,11 +104,14 @@ void IDBOpenDBRequest::onSuccess(PassRefPtr<IDBDatabaseBackendInterface> backend
     if (m_result) {
         idbDatabase = m_result->idbDatabase();
         ASSERT(idbDatabase);
+        ASSERT(!m_databaseCallbacks);
     } else {
-        idbDatabase = IDBDatabase::create(scriptExecutionContext(), backend);
+        ASSERT(m_databaseCallbacks);
+        idbDatabase = IDBDatabase::create(scriptExecutionContext(), backend, m_databaseCallbacks);
+        m_databaseCallbacks->connect(idbDatabase.get());
+        m_databaseCallbacks = 0;
         m_result = IDBAny::create(idbDatabase.get());
     }
-    idbDatabase->registerFrontendCallbacks();
     enqueueEvent(Event::create(eventNames().successEvent, false, false));
 }
 
index c87db7e..1e84b54 100644 (file)
 
 namespace WebCore {
 
+class IDBDatabaseCallbacksImpl;
+
 class IDBOpenDBRequest : public IDBRequest {
 public:
-    static PassRefPtr<IDBOpenDBRequest> create(ScriptExecutionContext*, PassRefPtr<IDBAny> source, int64_t version);
+    static PassRefPtr<IDBOpenDBRequest> create(ScriptExecutionContext*, PassRefPtr<IDBAny> source, PassRefPtr<IDBDatabaseCallbacksImpl>, int64_t version);
     virtual ~IDBOpenDBRequest();
 
     using IDBRequest::onSuccess;
@@ -53,8 +55,9 @@ protected:
     virtual bool shouldEnqueueEvent() const OVERRIDE;
 
 private:
-    IDBOpenDBRequest(ScriptExecutionContext*, PassRefPtr<IDBAny> source, int64_t version);
+    IDBOpenDBRequest(ScriptExecutionContext*, PassRefPtr<IDBAny> source, PassRefPtr<IDBDatabaseCallbacksImpl>, int64_t version);
 
+    RefPtr<IDBDatabaseCallbacksImpl> m_databaseCallbacks;
     int64_t m_version;
 };
 
index 21e5f32..e1e05f1 100644 (file)
@@ -294,12 +294,10 @@ void IDBTransaction::onAbort()
     }
     m_objectStoreCleanupMap.clear();
     closeOpenCursors();
-    m_database->transactionFinished(this);
-
-    if (m_contextStopped || !scriptExecutionContext())
-        return;
 
+    // Enqueue events before notifying database, as database may close which enqueues more events and order matters.
     enqueueEvent(Event::create(eventNames().abortEvent, true, false));
+    m_database->transactionFinished(this);
 }
 
 void IDBTransaction::onComplete()
@@ -309,12 +307,10 @@ void IDBTransaction::onComplete()
     m_state = Finishing;
     m_objectStoreCleanupMap.clear();
     closeOpenCursors();
-    m_database->transactionFinished(this);
-
-    if (m_contextStopped || !scriptExecutionContext())
-        return;
 
+    // Enqueue events before notifying database, as database may close which enqueues more events and order matters.
     enqueueEvent(Event::create(eventNames().completeEvent, false, false));
+    m_database->transactionFinished(this);
 }
 
 bool IDBTransaction::hasPendingActivity() const
index 5dfd995..f12bd0d 100644 (file)
@@ -172,35 +172,35 @@ private:
     String m_securityOrigin;
 };
 
-class ExecutableWithDatabase : public RefCounted<ExecutableWithDatabase> {
-public:
-    virtual ~ExecutableWithDatabase() { };
-    void start(IDBFactoryBackendInterface*, SecurityOrigin*, ScriptExecutionContext*, const String& databaseName);
-    virtual void execute(PassRefPtr<IDBDatabaseBackendInterface>) = 0;
-};
-
 class DatabaseConnection {
 public:
     DatabaseConnection()
         : m_idbDatabaseCallbacks(InspectorIDBDatabaseCallbacks::create()) { }
 
-    void connect(PassRefPtr<IDBDatabaseBackendInterface> idbDatabase)
-    {
-        m_idbDatabase = idbDatabase;
-        m_idbDatabase->registerFrontendCallbacks(m_idbDatabaseCallbacks);
-    }
-
     ~DatabaseConnection()
     {
         if (m_idbDatabase)
             m_idbDatabase->close(m_idbDatabaseCallbacks);
     }
 
+    void connect(PassRefPtr<IDBDatabaseBackendInterface> database) { m_idbDatabase = database; }
+    PassRefPtr<IDBDatabaseCallbacks> callbacks() { return m_idbDatabaseCallbacks; }
+
 private:
     RefPtr<IDBDatabaseBackendInterface> m_idbDatabase;
     RefPtr<IDBDatabaseCallbacks> m_idbDatabaseCallbacks;
 };
 
+class ExecutableWithDatabase : public RefCounted<ExecutableWithDatabase> {
+public:
+    virtual ~ExecutableWithDatabase() { };
+    void start(IDBFactoryBackendInterface*, SecurityOrigin*, ScriptExecutionContext*, const String& databaseName);
+    void connect(PassRefPtr<IDBDatabaseBackendInterface> database) { m_connection.connect(database); }
+    virtual void execute(PassRefPtr<IDBDatabaseBackendInterface>) = 0;
+private:
+    DatabaseConnection m_connection;
+};
+
 class OpenDatabaseCallback : public InspectorIDBCallback {
 public:
     static PassRefPtr<OpenDatabaseCallback> create(ExecutableWithDatabase* executableWithDatabase)
@@ -213,6 +213,7 @@ public:
     virtual void onSuccess(PassRefPtr<IDBDatabaseBackendInterface> prpDatabase)
     {
         RefPtr<IDBDatabaseBackendInterface> idbDatabase = prpDatabase;
+        m_executableWithDatabase->connect(idbDatabase);
         m_executableWithDatabase->execute(idbDatabase);
     }
 
@@ -225,7 +226,7 @@ private:
 void ExecutableWithDatabase::start(IDBFactoryBackendInterface* idbFactory, SecurityOrigin* securityOrigin, ScriptExecutionContext* context, const String& databaseName)
 {
     RefPtr<OpenDatabaseCallback> callback = OpenDatabaseCallback::create(this);
-    idbFactory->open(databaseName, IDBDatabaseMetadata::NoIntVersion, callback.get(), securityOrigin, context, String());
+    idbFactory->open(databaseName, IDBDatabaseMetadata::NoIntVersion, callback, m_connection.callbacks(), securityOrigin, context, String());
 }
 
 static PassRefPtr<IDBTransactionBackendInterface> transactionForDatabase(IDBDatabaseBackendInterface* idbDatabase, const String& objectStoreName)
@@ -296,7 +297,6 @@ public:
     virtual void execute(PassRefPtr<IDBDatabaseBackendInterface> prpDatabase)
     {
         RefPtr<IDBDatabaseBackendInterface> idbDatabase = prpDatabase;
-        m_connection.connect(idbDatabase);
         if (!m_requestCallback->isActive())
             return;
 
@@ -339,7 +339,6 @@ private:
     DatabaseLoaderCallback(PassRefPtr<RequestDatabaseCallback> requestCallback)
         : m_requestCallback(requestCallback) { }
     RefPtr<RequestDatabaseCallback> m_requestCallback;
-    DatabaseConnection m_connection;
 };
 
 static PassRefPtr<IDBKey> idbKeyFromInspectorObject(InspectorObject* key)
@@ -555,7 +554,6 @@ public:
     virtual void execute(PassRefPtr<IDBDatabaseBackendInterface> prpDatabase)
     {
         RefPtr<IDBDatabaseBackendInterface> idbDatabase = prpDatabase;
-        m_connection.connect(idbDatabase);
         if (!m_requestCallback->isActive())
             return;
 
@@ -597,7 +595,6 @@ private:
     RefPtr<IDBKeyRange> m_idbKeyRange;
     int m_skipCount;
     unsigned m_pageSize;
-    DatabaseConnection m_connection;
 };
 
 } // namespace
index 909e417..830ce53 100644 (file)
@@ -1,3 +1,53 @@
+2012-09-13  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: Consolidate two-phase connection to avoid race conditions
+        https://bugs.webkit.org/show_bug.cgi?id=90411
+
+        Reviewed by Tony Chang.
+
+        API plumbing for simplified single-phase connection opening, and tests updated
+        to exercise the new APIs.
+
+        * public/WebIDBDatabase.h:
+        (WebIDBDatabase): Just a FIXME to remove the old second-phase hookup API.
+        * public/WebIDBFactory.h:
+        (WebKit):
+        (WebIDBFactory):
+        (WebKit::WebIDBFactory::open): New overload that takes db-callbacks.
+        * src/IDBCallbacksProxy.cpp: The db-callbacks plumbing is needed earlier.
+        (WebKit::IDBCallbacksProxy::onSuccess):
+        (WebKit::IDBCallbacksProxy::onUpgradeNeeded):
+        (WebKit):
+        (WebKit::IDBCallbacksProxy::setDatabaseCallbacks): Needs to hold on to
+        the db-callbacks and hook it up when the onSuccess callback comes through.
+        * src/IDBCallbacksProxy.h:
+        (WebKit):
+        (IDBCallbacksProxy):
+        * src/IDBDatabaseBackendProxy.cpp:
+        * src/IDBDatabaseBackendProxy.h:
+        (IDBDatabaseBackendProxy):
+        * src/IDBFactoryBackendProxy.cpp:
+        (WebKit::IDBFactoryBackendProxy::open):
+        * src/IDBFactoryBackendProxy.h:
+        (IDBFactoryBackendProxy):
+        * src/WebIDBDatabaseImpl.cpp:
+        (WebKit::WebIDBDatabaseImpl::WebIDBDatabaseImpl):
+        (WebKit::WebIDBDatabaseImpl::close):
+        * src/WebIDBDatabaseImpl.h:
+        (WebIDBDatabaseImpl):
+        * src/WebIDBFactoryImpl.cpp:
+        (WebKit::WebIDBFactoryImpl::open):
+        * src/WebIDBFactoryImpl.h:
+        (WebIDBFactoryImpl):
+        * tests/IDBAbortOnCorruptTest.cpp:
+        (FakeIDBDatabaseCallbacks):
+        (WebCore::FakeIDBDatabaseCallbacks::create):
+        (WebCore::FakeIDBDatabaseCallbacks::~FakeIDBDatabaseCallbacks):
+        (WebCore::FakeIDBDatabaseCallbacks::FakeIDBDatabaseCallbacks):
+        (WebCore):
+        (WebCore::TEST): Updated connection sequence.
+        * tests/IDBDatabaseBackendTest.cpp: Ditto.
+
 2012-09-13  James Robinson  <jamesr@chromium.org>
 
         PlatformGestureCurveTest compile fix pt 2
index a8b035c..592a10e 100644 (file)
@@ -66,6 +66,7 @@ public:
     virtual void close() { WEBKIT_ASSERT_NOT_REACHED(); }
     virtual void forceClose() { WEBKIT_ASSERT_NOT_REACHED(); }
 
+    // FIXME: Remove this method after WK90411 cleanup is complete on the Chromium side.
     virtual void open(WebIDBDatabaseCallbacks*) { WEBKIT_ASSERT_NOT_REACHED(); }
 
 protected:
index 70c8c84..69a453a 100644 (file)
@@ -55,7 +55,7 @@ public:
 
     virtual void getDatabaseNames(WebIDBCallbacks* callbacks, const WebSecurityOrigin& origin, WebFrame* frame, const WebString& dataDir) { WEBKIT_ASSERT_NOT_REACHED(); }
 
-    // FIXME: This overload should be removed when WK90411 lands.
+    // FIXME: Remove this overload after WK90411 cleanup is complete on the Chromium side.
     // The WebKit implementation of open ignores the WebFrame* parameter.
     virtual void open(const WebString& name, long long version, WebIDBCallbacks* callbacks, const WebSecurityOrigin& origin, WebFrame* frame, const WebString& dataDir) { WEBKIT_ASSERT_NOT_REACHED(); }
 
index 193bf2b..af4984a 100644 (file)
 #include "IDBCursorBackendInterface.h"
 #include "IDBDatabaseBackendInterface.h"
 #include "IDBDatabaseBackendProxy.h"
+#include "IDBDatabaseCallbacksProxy.h"
 #include "IDBDatabaseError.h"
 #include "IDBObjectStoreBackendInterface.h"
 #include "IDBTransactionBackendInterface.h"
 #include "WebIDBCallbacks.h"
 #include "WebIDBCursorImpl.h"
-#include "WebIDBDatabaseImpl.h"
+#include "WebIDBDatabaseCallbacks.h"
 #include "WebIDBDatabaseError.h"
+#include "WebIDBDatabaseImpl.h"
 #include "WebIDBKey.h"
 #include "WebIDBTransactionImpl.h"
 #include "platform/WebSerializedScriptValue.h"
@@ -75,7 +77,8 @@ void IDBCallbacksProxy::onSuccess(PassRefPtr<IDBCursorBackendInterface> idbCurso
 
 void IDBCallbacksProxy::onSuccess(PassRefPtr<IDBDatabaseBackendInterface> backend)
 {
-    m_callbacks->onSuccess(new WebIDBDatabaseImpl(backend));
+    ASSERT(m_databaseCallbacks.get());
+    m_callbacks->onSuccess(new WebIDBDatabaseImpl(backend, m_databaseCallbacks.release()));
 }
 
 void IDBCallbacksProxy::onSuccess(PassRefPtr<IDBKey> idbKey)
@@ -137,9 +140,17 @@ void IDBCallbacksProxy::onBlocked(int64_t existingVersion)
 
 void IDBCallbacksProxy::onUpgradeNeeded(int64_t oldVersion, PassRefPtr<IDBTransactionBackendInterface> transaction, PassRefPtr<IDBDatabaseBackendInterface> database)
 {
-    m_callbacks->onUpgradeNeeded(oldVersion, new WebIDBTransactionImpl(transaction), new WebIDBDatabaseImpl(database));
+    ASSERT(m_databaseCallbacks);
+    m_callbacks->onUpgradeNeeded(oldVersion, new WebIDBTransactionImpl(transaction), new WebIDBDatabaseImpl(database, m_databaseCallbacks));
+}
+
+void IDBCallbacksProxy::setDatabaseCallbacks(PassRefPtr<IDBDatabaseCallbacksProxy> databaseCallbacks)
+{
+    ASSERT(!m_databaseCallbacks);
+    m_databaseCallbacks = databaseCallbacks;
 }
 
 } // namespace WebKit
 
+
 #endif // ENABLE(INDEXED_DATABASE)
index 825758a..f613a85 100644 (file)
@@ -39,6 +39,7 @@
 namespace WebKit {
 
 class WebIDBCallbacks;
+class IDBDatabaseCallbacksProxy;
 
 class IDBCallbacksProxy : public WebCore::IDBCallbacks {
 public:
@@ -59,10 +60,13 @@ public:
     virtual void onBlocked(int64_t existingVersion);
     virtual void onUpgradeNeeded(int64_t oldVersion, PassRefPtr<WebCore::IDBTransactionBackendInterface>, PassRefPtr<WebCore::IDBDatabaseBackendInterface>);
 
+    void setDatabaseCallbacks(PassRefPtr<IDBDatabaseCallbacksProxy>);
+
 private:
     IDBCallbacksProxy(PassOwnPtr<WebIDBCallbacks>);
 
     OwnPtr<WebIDBCallbacks> m_callbacks;
+    RefPtr<IDBDatabaseCallbacksProxy> m_databaseCallbacks;
 };
 
 } // namespace WebKit
index 56afe5f..501ce61 100644 (file)
@@ -107,11 +107,6 @@ void IDBDatabaseBackendProxy::close(PassRefPtr<IDBDatabaseCallbacks>)
     m_webIDBDatabase->close();
 }
 
-void IDBDatabaseBackendProxy::registerFrontendCallbacks(PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks)
-{
-    m_webIDBDatabase->open(new WebIDBDatabaseCallbacksImpl(databaseCallbacks));
-}
-
 } // namespace WebKit
 
 #endif // ENABLE(INDEXED_DATABASE)
index 2a93600..795610a 100644 (file)
@@ -50,8 +50,6 @@ public:
     virtual PassRefPtr<WebCore::IDBTransactionBackendInterface> transaction(WebCore::DOMStringList* storeNames, unsigned short mode, WebCore::ExceptionCode&);
     virtual void close(PassRefPtr<WebCore::IDBDatabaseCallbacks>);
 
-    virtual void registerFrontendCallbacks(PassRefPtr<WebCore::IDBDatabaseCallbacks>);
-
 private:
     IDBDatabaseBackendProxy(PassOwnPtr<WebIDBDatabase>);
 
index babcbae..28995ec 100755 (executable)
 #include "CrossThreadTask.h"
 #include "DOMStringList.h"
 #include "IDBDatabaseBackendProxy.h"
+#include "IDBDatabaseCallbacks.h"
 #include "IDBDatabaseError.h"
 #include "ScriptExecutionContext.h"
 #include "SecurityOrigin.h"
 #include "WebFrameImpl.h"
 #include "WebIDBCallbacksImpl.h"
 #include "WebIDBDatabase.h"
+#include "WebIDBDatabaseCallbacksImpl.h"
 #include "WebIDBDatabaseError.h"
 #include "WebIDBFactory.h"
 #include "WebKit.h"
@@ -203,15 +205,16 @@ void IDBFactoryBackendProxy::getDatabaseNames(PassRefPtr<IDBCallbacks> prpCallba
 }
 
 
-void IDBFactoryBackendProxy::open(const String& name, int64_t version, PassRefPtr<IDBCallbacks> prpCallbacks, PassRefPtr<SecurityOrigin> securityOrigin, ScriptExecutionContext* context, const String& dataDir)
+void IDBFactoryBackendProxy::open(const String& name, int64_t version, PassRefPtr<IDBCallbacks> prpCallbacks, PassRefPtr<IDBDatabaseCallbacks> prpDatabaseCallbacks, PassRefPtr<SecurityOrigin> securityOrigin, ScriptExecutionContext* context, const String& dataDir)
 {
     RefPtr<IDBCallbacks> callbacks(prpCallbacks);
+    RefPtr<IDBDatabaseCallbacks> databaseCallbacks(prpDatabaseCallbacks);
     WebSecurityOrigin origin(securityOrigin);
     if (!allowIndexedDB(context, name, origin, callbacks))
         return;
 
     WebFrameImpl* webFrame = getWebFrame(context);
-    m_webIDBFactory->open(name, version, new WebIDBCallbacksImpl(callbacks), origin, webFrame, dataDir);
+    m_webIDBFactory->open(name, version, new WebIDBCallbacksImpl(callbacks), new WebIDBDatabaseCallbacksImpl(databaseCallbacks), origin, webFrame, dataDir);
 }
 
 void IDBFactoryBackendProxy::deleteDatabase(const String& name, PassRefPtr<IDBCallbacks> prpCallbacks, PassRefPtr<SecurityOrigin> securityOrigin, ScriptExecutionContext* context, const String& dataDir)
index c0de910..88380d7 100644 (file)
@@ -49,7 +49,7 @@ public:
     virtual ~IDBFactoryBackendProxy();
 
     virtual void getDatabaseNames(PassRefPtr<WebCore::IDBCallbacks>, PassRefPtr<WebCore::SecurityOrigin>, WebCore::ScriptExecutionContext*, const String& dataDir);
-    virtual void open(const String& name, int64_t version, PassRefPtr<WebCore::IDBCallbacks>, PassRefPtr<WebCore::SecurityOrigin>, WebCore::ScriptExecutionContext*, const String& dataDir);
+    virtual void open(const String& name, int64_t version, PassRefPtr<WebCore::IDBCallbacks>, PassRefPtr<WebCore::IDBDatabaseCallbacks>, PassRefPtr<WebCore::SecurityOrigin>, WebCore::ScriptExecutionContext*, const String& dataDir);
     virtual void deleteDatabase(const String& name, PassRefPtr<WebCore::IDBCallbacks>, PassRefPtr<WebCore::SecurityOrigin>, WebCore::ScriptExecutionContext*, const String& dataDir);
 
 private:
index 0d142bc..52879a9 100644 (file)
@@ -45,8 +45,9 @@ using namespace WebCore;
 
 namespace WebKit {
 
-WebIDBDatabaseImpl::WebIDBDatabaseImpl(PassRefPtr<IDBDatabaseBackendInterface> databaseBackend)
+WebIDBDatabaseImpl::WebIDBDatabaseImpl(PassRefPtr<IDBDatabaseBackendInterface> databaseBackend, WTF::PassRefPtr<IDBDatabaseCallbacksProxy> databaseCallbacks)
     : m_databaseBackend(databaseBackend)
+    , m_databaseCallbacks(databaseCallbacks)
     , m_closePending(false)
 {
 }
@@ -93,7 +94,7 @@ WebIDBTransaction* WebIDBDatabaseImpl::transaction(const WebDOMStringList& names
 
 void WebIDBDatabaseImpl::close()
 {
-    // Use the callbacks that ::open gave us so that the backend in
+    // Use the callbacks passed in to the constructor so that the backend in
     // multi-process chromium knows which database connection is closing.
     if (!m_databaseCallbacks) {
         m_closePending = true;
@@ -113,15 +114,6 @@ void WebIDBDatabaseImpl::forceClose()
     callbacks->onForcedClose();
 }
 
-void WebIDBDatabaseImpl::open(WebIDBDatabaseCallbacks* callbacks)
-{
-    ASSERT(!m_databaseCallbacks);
-    m_databaseCallbacks = IDBDatabaseCallbacksProxy::create(adoptPtr(callbacks));
-    m_databaseBackend->registerFrontendCallbacks(m_databaseCallbacks);
-    if (m_closePending)
-        close();
-}
-
 } // namespace WebKit
 
 #endif // ENABLE(INDEXED_DATABASE)
index 4a8284c..9cd1caf 100644 (file)
@@ -47,7 +47,7 @@ class WebIDBTransaction;
 // See comment in WebIDBFactory for a high level overview these classes.
 class WebIDBDatabaseImpl : public WebIDBDatabase {
 public:
-    WebIDBDatabaseImpl(WTF::PassRefPtr<WebCore::IDBDatabaseBackendInterface>);
+    WebIDBDatabaseImpl(WTF::PassRefPtr<WebCore::IDBDatabaseBackendInterface>, WTF::PassRefPtr<IDBDatabaseCallbacksProxy>);
     virtual ~WebIDBDatabaseImpl();
 
     virtual WebIDBMetadata metadata() const;
@@ -59,9 +59,6 @@ public:
     virtual void forceClose();
     virtual void close();
 
-    // FIXME: Rename "open" to registerFrontendCallbacks.
-    virtual void open(WebIDBDatabaseCallbacks*);
-
 private:
     WTF::RefPtr<WebCore::IDBDatabaseBackendInterface> m_databaseBackend;
     WTF::RefPtr<IDBDatabaseCallbacksProxy> m_databaseCallbacks;
index ad71476..94a8d46 100755 (executable)
 
 #include "DOMStringList.h"
 #include "IDBCallbacksProxy.h"
+#include "IDBDatabaseCallbacksProxy.h"
 #include "IDBFactoryBackendImpl.h"
 #include "SecurityOrigin.h"
+#include "WebIDBDatabaseCallbacks.h"
 #include "WebIDBDatabaseError.h"
 #include <wtf/OwnPtr.h>
 
@@ -63,9 +65,12 @@ void WebIDBFactoryImpl::getDatabaseNames(WebIDBCallbacks* callbacks, const WebSe
     m_idbFactoryBackend->getDatabaseNames(IDBCallbacksProxy::create(adoptPtr(callbacks)), origin, 0, dataDir);
 }
 
-void WebIDBFactoryImpl::open(const WebString& name, long long version, WebIDBCallbacks* callbacks, const WebSecurityOrigin& origin, WebFrame*, const WebString& dataDir)
+void WebIDBFactoryImpl::open(const WebString& name, long long version, WebIDBCallbacks* callbacks, WebIDBDatabaseCallbacks* databaseCallbacks, const WebSecurityOrigin& origin, WebFrame*, const WebString& dataDir)
 {
-    m_idbFactoryBackend->open(name, version, IDBCallbacksProxy::create(adoptPtr(callbacks)).get(), origin, 0, dataDir);
+    RefPtr<IDBCallbacksProxy> callbacksProxy = IDBCallbacksProxy::create(adoptPtr(callbacks));
+    RefPtr<IDBDatabaseCallbacksProxy> databaseCallbacksProxy = IDBDatabaseCallbacksProxy::create(adoptPtr(databaseCallbacks));
+    callbacksProxy->setDatabaseCallbacks(databaseCallbacksProxy);
+    m_idbFactoryBackend->open(name, version, callbacksProxy.get(), databaseCallbacksProxy.get(), origin, 0, dataDir);
 }
 
 void WebIDBFactoryImpl::deleteDatabase(const WebString& name, WebIDBCallbacks* callbacks, const WebSecurityOrigin& origin, WebFrame*, const WebString& dataDir)
index 833eeb5..33f5a21 100644 (file)
@@ -45,7 +45,7 @@ public:
     virtual ~WebIDBFactoryImpl();
 
     virtual void getDatabaseNames(WebIDBCallbacks*, const WebSecurityOrigin&, WebFrame*, const WebString& dataDir);
-    virtual void open(const WebString& name, long long version, WebIDBCallbacks*, const WebSecurityOrigin&, WebFrame*, const WebString& dataDir);
+    virtual void open(const WebString& name, long long version, WebIDBCallbacks*, WebIDBDatabaseCallbacks*, const WebSecurityOrigin&, WebFrame*, const WebString& dataDir);
     virtual void deleteDatabase(const WebString& name, WebIDBCallbacks*, const WebSecurityOrigin&, WebFrame*, const WebString& dataDir);
 
 private:
index 1d39509..b080bfc 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "IDBCursorBackendInterface.h"
 #include "IDBDatabaseBackendInterface.h"
+#include "IDBDatabaseCallbacks.h"
 #include "IDBFactoryBackendImpl.h"
 #include "IDBFakeBackingStore.h"
 #include "SecurityOrigin.h"
@@ -96,14 +97,26 @@ protected:
     }
 };
 
+class FakeIDBDatabaseCallbacks : public IDBDatabaseCallbacks {
+public:
+    static PassRefPtr<FakeIDBDatabaseCallbacks> create() { return adoptRef(new FakeIDBDatabaseCallbacks()); }
+    virtual ~FakeIDBDatabaseCallbacks() { }
+    virtual void onVersionChange(const String& version) OVERRIDE { }
+    virtual void onVersionChange(int64_t oldVersion, int64_t newVersion) OVERRIDE { }
+    virtual void onForcedClose() OVERRIDE { }
+private:
+    FakeIDBDatabaseCallbacks() { }
+};
+
 TEST(IDBAbortTest, TheTest)
 {
     RefPtr<IDBFactoryBackendImpl> factory = FailingIDBFactoryBackendImpl::create();
     const String& name = "db name";
     MockIDBCallbacks callbacks;
+    RefPtr<FakeIDBDatabaseCallbacks> databaseCallbacks = FakeIDBDatabaseCallbacks::create();
     RefPtr<SecurityOrigin> origin = SecurityOrigin::create("http", "localhost", 81);
     const int64_t DummyVersion = 2;
-    factory->open(name, DummyVersion, &callbacks, origin, 0 /*Frame*/, String() /*path*/);
+    factory->open(name, DummyVersion, &callbacks, databaseCallbacks, origin, 0 /*Frame*/, String() /*path*/);
 }
 
 } // namespace
index 1fa2c26..cb94243 100644 (file)
@@ -27,6 +27,7 @@
 #include "IDBBackingStore.h"
 #include "IDBCursorBackendInterface.h"
 #include "IDBDatabaseBackendImpl.h"
+#include "IDBDatabaseCallbacksProxy.h"
 #include "IDBFactoryBackendImpl.h"
 #include "IDBFakeBackingStore.h"
 #include "IDBIndexBackendImpl.h"
 #if ENABLE(INDEXED_DATABASE)
 
 using namespace WebCore;
+using WebKit::IDBDatabaseCallbacksProxy;
 using WebKit::WebIDBDatabase;
-using WebKit::WebIDBDatabaseCallbacksImpl;
 using WebKit::WebIDBDatabaseImpl;
+using WebKit::WebIDBDatabaseCallbacksImpl;
 
 namespace {
 
@@ -122,20 +124,16 @@ TEST(IDBDatabaseBackendTest, ConnectionLifecycle)
     EXPECT_GT(backingStore->refCount(), 1);
 
     RefPtr<MockIDBCallbacks> request1 = MockIDBCallbacks::create();
-    db->openConnection(request1);
-
     RefPtr<FakeIDBDatabaseCallbacks> connection1 = FakeIDBDatabaseCallbacks::create();
-    db->registerFrontendCallbacks(connection1);
+    db->openConnection(request1, connection1);
 
     RefPtr<MockIDBCallbacks> request2 = MockIDBCallbacks::create();
-    db->openConnection(request2);
+    RefPtr<FakeIDBDatabaseCallbacks> connection2 = FakeIDBDatabaseCallbacks::create();
+    db->openConnection(request2, connection2);
 
     db->close(connection1);
     EXPECT_GT(backingStore->refCount(), 1);
 
-    RefPtr<FakeIDBDatabaseCallbacks> connection2 = FakeIDBDatabaseCallbacks::create();
-    db->registerFrontendCallbacks(connection2);
-
     db->close(connection2);
     EXPECT_TRUE(backingStore->hasOneRef());
 }
@@ -149,7 +147,7 @@ public:
 
     ~MockIDBDatabaseBackendProxy()
     {
-        EXPECT_TRUE(m_wasRegisterFrontendCallbacksCalled);
+        EXPECT_TRUE(m_wasCloseCalled);
     }
 
     virtual IDBDatabaseMetadata metadata() const { return IDBDatabaseMetadata(); }
@@ -163,19 +161,12 @@ public:
         m_wasCloseCalled = true;
         m_webDatabase.close();
     }
-    virtual void registerFrontendCallbacks(PassRefPtr<IDBDatabaseCallbacks> connection)
-    {
-        m_wasRegisterFrontendCallbacksCalled = true;
-        m_webDatabase.open(new WebIDBDatabaseCallbacksImpl(connection));
-    }
 
 private:
     MockIDBDatabaseBackendProxy(WebIDBDatabaseImpl& webDatabase)
-        : m_wasRegisterFrontendCallbacksCalled(false)
-        , m_wasCloseCalled(false)
+        : m_wasCloseCalled(false)
         , m_webDatabase(webDatabase) { }
 
-    bool m_wasRegisterFrontendCallbacksCalled;
     bool m_wasCloseCalled;
 
     WebIDBDatabaseImpl& m_webDatabase;
@@ -191,18 +182,19 @@ TEST(IDBDatabaseBackendTest, ForcedClose)
     RefPtr<IDBDatabaseBackendImpl> backend = IDBDatabaseBackendImpl::create("db", backingStore.get(), coordinator, factory, "uniqueid");
     EXPECT_GT(backingStore->refCount(), 1);
 
-    WebIDBDatabaseImpl webDatabase(backend);
-
-    RefPtr<MockIDBCallbacks> request1 = MockIDBCallbacks::create();
-    backend->openConnection(request1);
+    RefPtr<FakeIDBDatabaseCallbacks> connection = FakeIDBDatabaseCallbacks::create();
+    RefPtr<IDBDatabaseCallbacksProxy> connectionProxy = IDBDatabaseCallbacksProxy::create(adoptPtr(new WebIDBDatabaseCallbacksImpl(connection)));
+    WebIDBDatabaseImpl webDatabase(backend, connectionProxy);
 
     RefPtr<MockIDBDatabaseBackendProxy> proxy = MockIDBDatabaseBackendProxy::create(webDatabase);
+    RefPtr<MockIDBCallbacks> request = MockIDBCallbacks::create();
+    backend->openConnection(request, connectionProxy);
 
     ScriptExecutionContext* context = 0;
-    RefPtr<IDBDatabase> idbDatabase = IDBDatabase::create(context, proxy);
-    idbDatabase->registerFrontendCallbacks();
+    RefPtr<IDBDatabase> idbDatabase = IDBDatabase::create(context, proxy, connection);
 
     webDatabase.forceClose();
+
     EXPECT_TRUE(backingStore->hasOneRef());
 }