[webkit-changes] [120958] branches/chromium/1180/Source

Thu, 21 Jun 2012 13:06:20 -0700

Title: [120958] branches/chromium/1180/Source
Revision
120958
Author
[email protected]
Date
2012-06-21 12:29:24 -0700 (Thu, 21 Jun 2012)

Log Message

Merge 120828 - [Chromium] IndexedDB: Don't close database if pending connections are in flight
https://bugs.webkit.org/show_bug.cgi?id=89512

Source/WebCore:

Add a counter tracking connections between the two phases, which is used along with
the completed connection count to determine the total number of connections.

Reviewed by Tony Chang.

Test: webkit_unit_tests --gtest_filter='IDBDatabaseBackendTest.ConnectionLifecycle'

* Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
(WebCore::IDBDatabaseBackendImpl::IDBDatabaseBackendImpl):
(WebCore::IDBDatabaseBackendImpl::setVersion):
(WebCore::IDBDatabaseBackendImpl::connectionCount):
(WebCore):
(WebCore::IDBDatabaseBackendImpl::processPendingCalls):
(WebCore::IDBDatabaseBackendImpl::transaction):
(WebCore::IDBDatabaseBackendImpl::registerFrontendCallbacks):
(WebCore::IDBDatabaseBackendImpl::openConnection):
(WebCore::IDBDatabaseBackendImpl::close):
* Modules/indexeddb/IDBDatabaseBackendImpl.h:
(IDBDatabaseBackendImpl):
* Modules/indexeddb/IDBFactoryBackendImpl.cpp:
(WebCore::IDBFactoryBackendImpl::openInternal):
* inspector/InspectorIndexedDBAgent.cpp:
(WebCore):

Source/WebKit/chromium:

Reviewed by Tony Chang.

* tests/IDBDatabaseBackendTest.cpp:
(MockIDBCallbacks):
(WebCore::MockIDBCallbacks::create):
(WebCore::MockIDBCallbacks::MockIDBCallbacks):
(WebCore):
(FakeIDBDatabaseCallbacks):
(WebCore::FakeIDBDatabaseCallbacks::create):
(WebCore::FakeIDBDatabaseCallbacks::FakeIDBDatabaseCallbacks):
(WebCore::TEST):


[email protected]
Review URL: https://chromiumcodereview.appspot.com/10630009

Modified Paths

Diff

Modified: branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp (120957 => 120958)


--- branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp	2012-06-21 19:26:37 UTC (rev 120957)
+++ branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp	2012-06-21 19:29:24 UTC (rev 120958)
@@ -109,6 +109,7 @@
     , m_identifier(uniqueIdentifier)
     , m_factory(factory)
     , m_transactionCoordinator(coordinator)
+    , m_pendingConnectionCount(0)
 {
     ASSERT(!m_name.isNull());
 }
@@ -222,7 +223,7 @@
     // 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 (m_databaseCallbacksSet.size() > 1) {
+    if (connectionCount() > 1) {
         callbacks->onBlocked();
         RefPtr<PendingSetVersionCall> pendingSetVersionCall = PendingSetVersionCall::create(version, callbacks, databaseCallbacks);
         m_pendingSetVersionCalls.append(pendingSetVersionCall);
@@ -278,9 +279,14 @@
     }
 }
 
+int32_t IDBDatabaseBackendImpl::connectionCount()
+{
+    return m_databaseCallbacksSet.size() + m_pendingConnectionCount;
+}
+
 void IDBDatabaseBackendImpl::processPendingCalls()
 {
-    ASSERT(m_databaseCallbacksSet.size() <= 1);
+    ASSERT(connectionCount() <= 1);
 
     // Pending calls may be requeued or aborted
     Deque<RefPtr<PendingSetVersionCall> > pendingSetVersionCalls;
@@ -345,18 +351,24 @@
 
 void IDBDatabaseBackendImpl::registerFrontendCallbacks(PassRefPtr<IDBDatabaseCallbacks> callbacks)
 {
+    ASSERT(m_backingStore.get());
+    ASSERT(m_pendingConnectionCount);
+    --m_pendingConnectionCount;
     m_databaseCallbacksSet.add(RefPtr<IDBDatabaseCallbacks>(callbacks));
 }
 
 void IDBDatabaseBackendImpl::openConnection(PassRefPtr<IDBCallbacks> callbacks)
 {
+    ASSERT(m_backingStore.get());
     if (!m_pendingDeleteCalls.isEmpty() || m_runningVersionChangeTransaction || !m_pendingSetVersionCalls.isEmpty())
         m_pendingOpenCalls.append(PendingOpenCall::create(callbacks));
     else {
         if (m_id == InvalidId && !openInternal())
             callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error."));
-        else
+        else {
+            ++m_pendingConnectionCount;
             callbacks->onSuccess(this);
+        }
     }
 }
 
@@ -395,12 +407,12 @@
     RefPtr<IDBDatabaseCallbacks> callbacks = prpCallbacks;
     ASSERT(m_databaseCallbacksSet.contains(callbacks));
     m_databaseCallbacksSet.remove(callbacks);
-    if (m_databaseCallbacksSet.size() > 1)
+    if (connectionCount() > 1)
         return;
 
     processPendingCalls();
 
-    if (!m_databaseCallbacksSet.size()) {
+    if (!connectionCount()) {
         TransactionSet transactions(m_transactions);
         for (TransactionSet::const_iterator it = transactions.begin(); it != transactions.end(); ++it)
             (*it)->abort();

Modified: branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h (120957 => 120958)


--- branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h	2012-06-21 19:26:37 UTC (rev 120957)
+++ branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h	2012-06-21 19:29:24 UTC (rev 120958)
@@ -78,6 +78,7 @@
 
     bool openInternal();
     void loadObjectStores();
+    int32_t connectionCount();
     void processPendingCalls();
 
     static void createObjectStoreInternal(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendImpl>, PassRefPtr<IDBObjectStoreBackendImpl>, PassRefPtr<IDBTransactionBackendInterface>);
@@ -116,6 +117,10 @@
     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;
 };

Modified: branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp (120957 => 120958)


--- branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp	2012-06-21 19:26:37 UTC (rev 120957)
+++ branches/chromium/1180/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp	2012-06-21 19:29:24 UTC (rev 120958)
@@ -182,8 +182,8 @@
 
     RefPtr<IDBDatabaseBackendImpl> databaseBackend = IDBDatabaseBackendImpl::create(name, backingStore.get(), m_transactionCoordinator.get(), this, uniqueIdentifier);
     if (databaseBackend) {
-        callbacks->onSuccess(RefPtr<IDBDatabaseBackendInterface>(databaseBackend.get()).release());
         m_databaseBackendMap.set(uniqueIdentifier, databaseBackend.get());
+        databaseBackend->openConnection(callbacks);
     } else
         callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error."));
 }

Modified: branches/chromium/1180/Source/WebCore/inspector/InspectorIndexedDBAgent.cpp (120957 => 120958)


--- branches/chromium/1180/Source/WebCore/inspector/InspectorIndexedDBAgent.cpp	2012-06-21 19:26:37 UTC (rev 120957)
+++ branches/chromium/1180/Source/WebCore/inspector/InspectorIndexedDBAgent.cpp	2012-06-21 19:29:24 UTC (rev 120958)
@@ -43,6 +43,7 @@
 #include "IDBCallbacks.h"
 #include "IDBCursor.h"
 #include "IDBDatabaseBackendInterface.h"
+#include "IDBDatabaseCallbacks.h"
 #include "IDBFactoryBackendInterface.h"
 #include "IDBIndexBackendInterface.h"
 #include "IDBKey.h"
@@ -115,6 +116,21 @@
     virtual void onBlocked() { }
 };
 
+class InspectorIDBDatabaseCallbacks : public IDBDatabaseCallbacks {
+public:
+    static PassRefPtr<InspectorIDBDatabaseCallbacks> create()
+    {
+        return adoptRef(new InspectorIDBDatabaseCallbacks());
+    }
+
+    virtual ~InspectorIDBDatabaseCallbacks() { }
+
+    virtual void onVersionChange(const String& version) { }
+private:
+    InspectorIDBDatabaseCallbacks() { }
+};
+
+
 class InspectorIDBTransactionCallback : public IDBTransactionCallbacks {
 public:
     static PassRefPtr<InspectorIDBTransactionCallback> create()
@@ -172,6 +188,28 @@
     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);
+    }
+
+private:
+    RefPtr<IDBDatabaseBackendInterface> m_idbDatabase;
+    RefPtr<IDBDatabaseCallbacks> m_idbDatabaseCallbacks;
+};
+
 class OpenDatabaseCallback : public InspectorIDBCallback {
 public:
     static PassRefPtr<OpenDatabaseCallback> create(ExecutableWithDatabase* executableWithDatabase)
@@ -181,8 +219,9 @@
 
     virtual ~OpenDatabaseCallback() { }
 
-    virtual void onSuccess(PassRefPtr<IDBDatabaseBackendInterface> idbDatabase)
+    virtual void onSuccess(PassRefPtr<IDBDatabaseBackendInterface> prpDatabase)
     {
+        RefPtr<IDBDatabaseBackendInterface> idbDatabase = prpDatabase;
         m_executableWithDatabase->execute(idbDatabase);
     }
 
@@ -263,8 +302,10 @@
 
     virtual ~DatabaseLoaderCallback() { }
 
-    virtual void execute(PassRefPtr<IDBDatabaseBackendInterface> idbDatabase)
+    virtual void execute(PassRefPtr<IDBDatabaseBackendInterface> prpDatabase)
     {
+        RefPtr<IDBDatabaseBackendInterface> idbDatabase = prpDatabase;
+        m_connection.connect(idbDatabase);
         if (!m_frontendProvider->frontend())
             return;
 
@@ -315,6 +356,7 @@
         , m_requestId(requestId) { }
     RefPtr<InspectorIndexedDBAgent::FrontendProvider> m_frontendProvider;
     int m_requestId;
+    DatabaseConnection m_connection;
 };
 
 static PassRefPtr<IDBKey> idbKeyFromInspectorObject(InspectorObject* key)
@@ -430,6 +472,8 @@
     return key.release();
 }
 
+class DataLoaderCallback;
+
 class OpenCursorCallback : public InspectorIDBCallback {
 public:
     enum CursorType {
@@ -437,9 +481,9 @@
         IndexDataCursor
     };
 
-    static PassRefPtr<OpenCursorCallback> create(PassRefPtr<InspectorIndexedDBAgent::FrontendProvider> frontendProvider, InjectedScript injectedScript, PassRefPtr<IDBTransactionBackendInterface> idbTransaction, CursorType cursorType, int requestId, int skipCount, unsigned pageSize)
+    static PassRefPtr<OpenCursorCallback> create(PassRefPtr<InspectorIndexedDBAgent::FrontendProvider> frontendProvider, InjectedScript injectedScript, PassRefPtr<DataLoaderCallback> dataLoaderCallback, PassRefPtr<IDBTransactionBackendInterface> idbTransaction, CursorType cursorType, int requestId, int skipCount, unsigned pageSize)
     {
-        return adoptRef(new OpenCursorCallback(frontendProvider, injectedScript, idbTransaction, cursorType, requestId, skipCount, pageSize));
+        return adoptRef(new OpenCursorCallback(frontendProvider, injectedScript, dataLoaderCallback, idbTransaction, cursorType, requestId, skipCount, pageSize));
     }
 
     virtual ~OpenCursorCallback() { }
@@ -491,6 +535,7 @@
 
     void end(bool hasMore)
     {
+        m_dataLoaderCallback.clear();
         if (!m_frontendProvider->frontend())
             return;
 
@@ -509,9 +554,10 @@
     }
 
 private:
-    OpenCursorCallback(PassRefPtr<InspectorIndexedDBAgent::FrontendProvider> frontendProvider, InjectedScript injectedScript, PassRefPtr<IDBTransactionBackendInterface> idbTransaction, CursorType cursorType, int requestId, int skipCount, unsigned pageSize)
+    OpenCursorCallback(PassRefPtr<InspectorIndexedDBAgent::FrontendProvider> frontendProvider, InjectedScript injectedScript, PassRefPtr<DataLoaderCallback> dataLoaderCallback, PassRefPtr<IDBTransactionBackendInterface> idbTransaction, CursorType cursorType, int requestId, int skipCount, unsigned pageSize)
         : m_frontendProvider(frontendProvider)
         , m_injectedScript(injectedScript)
+        , m_dataLoaderCallback(dataLoaderCallback)
         , m_idbTransaction(idbTransaction)
         , m_cursorType(cursorType)
         , m_requestId(requestId)
@@ -523,6 +569,7 @@
     }
     RefPtr<InspectorIndexedDBAgent::FrontendProvider> m_frontendProvider;
     InjectedScript m_injectedScript;
+    RefPtr<DataLoaderCallback> m_dataLoaderCallback;
     RefPtr<IDBTransactionBackendInterface> m_idbTransaction;
     CursorType m_cursorType;
     int m_requestId;
@@ -541,8 +588,10 @@
 
     virtual ~DataLoaderCallback() { }
 
-    virtual void execute(PassRefPtr<IDBDatabaseBackendInterface> idbDatabase)
+    virtual void execute(PassRefPtr<IDBDatabaseBackendInterface> prpDatabase)
     {
+        RefPtr<IDBDatabaseBackendInterface> idbDatabase = prpDatabase;
+        m_connection.connect(idbDatabase);
         if (!m_frontendProvider->frontend())
             return;
 
@@ -558,12 +607,12 @@
             if (!idbIndex)
                 return;
 
-            RefPtr<OpenCursorCallback> openCursorCallback = OpenCursorCallback::create(m_frontendProvider, m_injectedScript, idbTransaction.get(), OpenCursorCallback::IndexDataCursor, m_requestId, m_skipCount, m_pageSize);
+            RefPtr<OpenCursorCallback> openCursorCallback = OpenCursorCallback::create(m_frontendProvider, m_injectedScript, this, idbTransaction.get(), OpenCursorCallback::IndexDataCursor, m_requestId, m_skipCount, m_pageSize);
 
             ExceptionCode ec = 0;
             idbIndex->openCursor(m_idbKeyRange, IDBCursor::NEXT, openCursorCallback, idbTransaction.get(), ec);
         } else {
-            RefPtr<OpenCursorCallback> openCursorCallback = OpenCursorCallback::create(m_frontendProvider, m_injectedScript, idbTransaction.get(), OpenCursorCallback::ObjectStoreDataCursor, m_requestId, m_skipCount, m_pageSize);
+            RefPtr<OpenCursorCallback> openCursorCallback = OpenCursorCallback::create(m_frontendProvider, m_injectedScript, this, idbTransaction.get(), OpenCursorCallback::ObjectStoreDataCursor, m_requestId, m_skipCount, m_pageSize);
 
             ExceptionCode ec = 0;
             idbObjectStore->openCursor(m_idbKeyRange, IDBCursor::NEXT, openCursorCallback, idbTransaction.get(), ec);
@@ -588,6 +637,7 @@
     RefPtr<IDBKeyRange> m_idbKeyRange;
     int m_skipCount;
     unsigned m_pageSize;
+    DatabaseConnection m_connection;
 };
 
 } // namespace

Modified: branches/chromium/1180/Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp (120957 => 120958)


--- branches/chromium/1180/Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp	2012-06-21 19:26:37 UTC (rev 120957)
+++ branches/chromium/1180/Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp	2012-06-21 19:29:24 UTC (rev 120958)
@@ -67,6 +67,70 @@
     EXPECT_TRUE(backingStore->hasOneRef());
 }
 
+class MockIDBCallbacks : public IDBCallbacks {
+public:
+    static PassRefPtr<MockIDBCallbacks> create() { return adoptRef(new MockIDBCallbacks()); }
+    virtual ~MockIDBCallbacks() OVERRIDE
+    {
+        EXPECT_TRUE(m_wasSuccessDBCalled);
+    }
+    virtual void onError(PassRefPtr<IDBDatabaseError>) OVERRIDE { }
+    virtual void onSuccess(PassRefPtr<DOMStringList>) OVERRIDE { }
+    virtual void onSuccess(PassRefPtr<IDBCursorBackendInterface>) OVERRIDE { }
+    virtual void onSuccess(PassRefPtr<IDBDatabaseBackendInterface>) OVERRIDE
+    {
+        m_wasSuccessDBCalled = true;
+    }
+    virtual void onSuccess(PassRefPtr<IDBKey>) OVERRIDE { }
+    virtual void onSuccess(PassRefPtr<IDBTransactionBackendInterface>) OVERRIDE { }
+    virtual void onSuccess(PassRefPtr<SerializedScriptValue>) OVERRIDE { }
+    virtual void onSuccessWithContinuation() OVERRIDE { }
+    virtual void onSuccessWithPrefetch(const Vector<RefPtr<IDBKey> >&, const Vector<RefPtr<IDBKey> >&, const Vector<RefPtr<SerializedScriptValue> >&) OVERRIDE { }
+    virtual void onBlocked() OVERRIDE { }
+private:
+    MockIDBCallbacks()
+        : m_wasSuccessDBCalled(false) { }
+    bool m_wasSuccessDBCalled;
+};
+
+class FakeIDBDatabaseCallbacks : public IDBDatabaseCallbacks {
+public:
+    static PassRefPtr<FakeIDBDatabaseCallbacks> create() { return adoptRef(new FakeIDBDatabaseCallbacks()); }
+    virtual ~FakeIDBDatabaseCallbacks() OVERRIDE { }
+    virtual void onVersionChange(const String& version) OVERRIDE { }
+private:
+    FakeIDBDatabaseCallbacks() { }
+};
+
+TEST(IDBDatabaseBackendTest, ConnectionLifecycle)
+{
+    RefPtr<IDBFakeBackingStore> backingStore = adoptRef(new IDBFakeBackingStore());
+    EXPECT_TRUE(backingStore->hasOneRef());
+
+    IDBTransactionCoordinator* coordinator = 0;
+    IDBFactoryBackendImpl* factory = 0;
+    RefPtr<IDBDatabaseBackendImpl> db = IDBDatabaseBackendImpl::create("db", backingStore.get(), coordinator, factory, "uniqueid");
+    EXPECT_GT(backingStore->refCount(), 1);
+
+    RefPtr<MockIDBCallbacks> request1 = MockIDBCallbacks::create();
+    db->openConnection(request1);
+
+    RefPtr<FakeIDBDatabaseCallbacks> connection1 = FakeIDBDatabaseCallbacks::create();
+    db->registerFrontendCallbacks(connection1);
+
+    RefPtr<MockIDBCallbacks> request2 = MockIDBCallbacks::create();
+    db->openConnection(request2);
+
+    db->close(connection1);
+    EXPECT_GT(backingStore->refCount(), 1);
+
+    RefPtr<FakeIDBDatabaseCallbacks> connection2 = FakeIDBDatabaseCallbacks::create();
+    db->registerFrontendCallbacks(connection2);
+
+    db->close(connection2);
+    EXPECT_TRUE(backingStore->hasOneRef());
+}
+
 } // namespace
 
 #endif // ENABLE(INDEXED_DATABASE)

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to