storage/indexeddb/modern/idbdatabase-deleteobjectstore-failures.html is flaky.

https://bugs.webkit.org/show_bug.cgi?id=150735

Reviewed by Darin Adler.

Source/WebCore:

No new tests (Covered by existing tests).

Transactions were liable to commit too early because IDBRequests could be waiting
to dispatch their error/success events but their operations would no longer be
registered with the transaction.

Having outstanding requests should also keep a transaction from committing, just
like having outstanding operations should.

* Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp:
(WebCore::IDBClient::IDBOpenDBRequest::onUpgradeNeeded):

* Modules/indexeddb/client/IDBRequestImpl.cpp:
(WebCore::IDBClient::IDBRequest::dispatchEvent):

* Modules/indexeddb/client/IDBTransactionImpl.cpp:
(WebCore::IDBClient::IDBTransaction::addRequest):
(WebCore::IDBClient::IDBTransaction::removeRequest):
(WebCore::IDBClient::IDBTransaction::operationTimerFired):
(WebCore::IDBClient::IDBTransaction::requestGetRecord):
(WebCore::IDBClient::IDBTransaction::requestClearObjectStore):
(WebCore::IDBClient::IDBTransaction::requestPutOrAdd):
(WebCore::IDBClient::IDBTransaction::operationDidComplete):
* Modules/indexeddb/client/IDBTransactionImpl.h:

* Modules/indexeddb/client/TransactionOperation.h:
(WebCore::IDBClient::TransactionOperation::completed):

LayoutTests:

* platform/mac-wk1/TestExpectations: Reenable the test.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 278556fe482f7c1348d980be54230377989af850..2b133a65f46a29a3db7549ca37dbef5c9ecdaa55 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2015-10-31  Brady Eidson  <beidson@apple.com>
+
+        storage/indexeddb/modern/idbdatabase-deleteobjectstore-failures.html is flaky.
+        https://bugs.webkit.org/show_bug.cgi?id=150735
+
+        Reviewed by Darin Adler.
+
+        * platform/mac-wk1/TestExpectations: Reenable the test.
+
 2015-10-30  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Test Debugger.scriptParsed events received after opening inspector frontend

diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
index 26a2adcced9b0fa3468c39dcb708b73a65c14087..38eeb87cb5f833d6d8d0277f39387451da1e36ba 100644 (file)
--- a/LayoutTests/platform/mac-wk1/TestExpectations
+++ b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -50,8 +50,6 @@ webkit.org/b/146622 [ Yosemite Debug ] webgl/1.0.2/conformance/more/functions/co
 
 webkit.org/b/150564 svg/repaint/add-background-property-on-root.html [ Pass Timeout ]
 
-webkit.org/b/150735 storage/indexeddb/modern/idbdatabase-deleteobjectstore-failures.html [ Pass Failure ]
-
 ### END OF (1) Failures with bug reports
 ########################################
 

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 69f9afba91c16518d4847ff1f1fbe92854d6635e..26ba3eee95dd7993e476986e5f701e11e4cd2474 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,38 @@
+2015-10-31  Brady Eidson  <beidson@apple.com>
+
+        storage/indexeddb/modern/idbdatabase-deleteobjectstore-failures.html is flaky.
+        https://bugs.webkit.org/show_bug.cgi?id=150735
+
+        Reviewed by Darin Adler.
+
+        No new tests (Covered by existing tests).
+
+        Transactions were liable to commit too early because IDBRequests could be waiting
+        to dispatch their error/success events but their operations would no longer be
+        registered with the transaction.
+        
+        Having outstanding requests should also keep a transaction from committing, just
+        like having outstanding operations should.
+        
+        * Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp:
+        (WebCore::IDBClient::IDBOpenDBRequest::onUpgradeNeeded):
+        
+        * Modules/indexeddb/client/IDBRequestImpl.cpp:
+        (WebCore::IDBClient::IDBRequest::dispatchEvent):
+        
+        * Modules/indexeddb/client/IDBTransactionImpl.cpp:
+        (WebCore::IDBClient::IDBTransaction::addRequest):
+        (WebCore::IDBClient::IDBTransaction::removeRequest):
+        (WebCore::IDBClient::IDBTransaction::operationTimerFired):
+        (WebCore::IDBClient::IDBTransaction::requestGetRecord):
+        (WebCore::IDBClient::IDBTransaction::requestClearObjectStore):
+        (WebCore::IDBClient::IDBTransaction::requestPutOrAdd):
+        (WebCore::IDBClient::IDBTransaction::operationDidComplete):
+        * Modules/indexeddb/client/IDBTransactionImpl.h:
+        
+        * Modules/indexeddb/client/TransactionOperation.h:
+        (WebCore::IDBClient::TransactionOperation::completed):
+
 2015-10-31  Philippe Normand  <pnormand@igalia.com>
 
         [GStreamer][Mac] Fix WebAudio build

diff --git a/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp b/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp
index b6dbd3b7b10d9c2f5b5a570b5b323cc341efc165..80ffce13856d661d1127061dcc09b944e20beddf 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp
+++ b/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp
@@ -96,6 +96,7 @@ void IDBOpenDBRequest::onUpgradeNeeded(const IDBResultData& resultData)
     m_result = IDBAny::create(WTF::move(database));
     m_readyState = IDBRequestReadyState::Done;
     m_transaction = adoptRef(&transaction.leakRef());
+    m_transaction->addRequest(*this);
 
     enqueueEvent(IDBVersionChangeEvent::create(oldVersion, newVersion, eventNames().upgradeneededEvent));
 }

diff --git a/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp b/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp
index cb294015ba204a0c24b9ceb9062ed67231db9fa0..addf3fae641b36c1eeefca3f25f66cd412c5b80d 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp
+++ b/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp
@@ -166,6 +166,10 @@ bool IDBRequest::dispatchEvent(PassRefPtr<Event> prpEvent)
 
     m_hasPendingActivity = false;
 
+    // FIXME: When we implement reusable requests (for cursors) it will be incorrect to always remove the request from the transaction.
+    if (m_transaction)
+        m_transaction->removeRequest(*this);
+
     return dontPreventDefault;
 }
 

diff --git a/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp b/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp
index 5fa46079646cafe0f3087cf3065d8c0ca74d6d75..5b1a93a499eedbf2df315ee26fe090a16d888d47 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp
+++ b/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp
@@ -207,6 +207,18 @@ bool IDBTransaction::isFinishedOrFinishing() const
         || m_state == IndexedDB::TransactionState::Finished;
 }
 
+void IDBTransaction::addRequest(IDBRequest& request)
+{
+    ASSERT(!m_openRequests.contains(&request));
+    m_openRequests.add(&request);
+}
+
+void IDBTransaction::removeRequest(IDBRequest& request)
+{
+    ASSERT(m_openRequests.contains(&request));
+    m_openRequests.remove(&request);
+}
+
 void IDBTransaction::scheduleOperation(RefPtr<TransactionOperation>&& operation)
 {
     ASSERT(!m_transactionOperationMap.contains(operation->identifier()));
@@ -237,6 +249,9 @@ void IDBTransaction::operationTimerFired()
         return;
     }
 
+    if (!m_transactionOperationMap.isEmpty() || !m_openRequests.isEmpty())
+        return;
+
     if (!isFinishedOrFinishing())
         commit();
 }
@@ -393,6 +408,7 @@ Ref<IDBRequest> IDBTransaction::requestGetRecord(ScriptExecutionContext& context
     ASSERT(!keyRangeData.isNull);
 
     Ref<IDBRequest> request = IDBRequest::create(context, objectStore, *this);
+    addRequest(request.get());
 
     auto operation = createTransactionOperation(*this, request.get(), &IDBTransaction::didGetRecordOnServer, &IDBTransaction::getRecordOnServer, keyRangeData);
     scheduleOperation(WTF::move(operation));
@@ -421,6 +437,7 @@ Ref<IDBRequest> IDBTransaction::requestClearObjectStore(ScriptExecutionContext&
     ASSERT(isActive());
 
     Ref<IDBRequest> request = IDBRequest::create(context, objectStore, *this);
+    addRequest(request.get());
 
     uint64_t objectStoreIdentifier = objectStore.info().identifier();
     auto operation = createTransactionOperation(*this, request.get(), &IDBTransaction::didClearObjectStoreOnServer, &IDBTransaction::clearObjectStoreOnServer, objectStoreIdentifier);
@@ -452,6 +469,7 @@ Ref<IDBRequest> IDBTransaction::requestPutOrAdd(ScriptExecutionContext& context,
     ASSERT(objectStore.info().autoIncrement() || key);
 
     Ref<IDBRequest> request = IDBRequest::create(context, objectStore, *this);
+    addRequest(request.get());
 
     auto operation = createTransactionOperation(*this, request.get(), &IDBTransaction::didPutOrAddOnServer, &IDBTransaction::putOrAddOnServer, key, &value, overwriteMode);
     scheduleOperation(WTF::move(operation));
@@ -503,6 +521,14 @@ void IDBTransaction::didDeleteObjectStoreOnServer(const IDBResultData& resultDat
     ASSERT_UNUSED(resultData, resultData.type() == IDBResultType::DeleteObjectStoreSuccess);
 }
 
+void IDBTransaction::operationDidComplete(TransactionOperation& operation)
+{
+    ASSERT(m_transactionOperationMap.get(operation.identifier()) == &operation);
+    m_transactionOperationMap.remove(operation.identifier());
+
+    scheduleOperationTimer();
+}
+
 void IDBTransaction::establishOnServer()
 {
     LOG(IndexedDB, "IDBTransaction::establishOnServer");

diff --git a/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.h b/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.h
index 721b4308ee431d3a871e23b1a5f1922776521e22..a19066530237853f59a19e5a0b64284f3301fa26 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.h
+++ b/Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.h
@@ -95,12 +95,15 @@ public:
 
     void deleteObjectStore(const String& objectStoreName);
 
+    void addRequest(IDBRequest&);
+    void removeRequest(IDBRequest&);
+
     IDBConnectionToServer& serverConnection();
 
     void activate();
     void deactivate();
 
-    void scheduleOperationTimer();
+    void operationDidComplete(TransactionOperation&);
 
 private:
     IDBTransaction(IDBDatabase&, const IDBTransactionInfo&);
@@ -138,6 +141,8 @@ private:
 
     void establishOnServer();
 
+    void scheduleOperationTimer();
+
     Ref<IDBDatabase> m_database;
     IDBTransactionInfo m_info;
     std::unique_ptr<IDBDatabaseInfo> m_originalDatabaseInfo;
@@ -154,6 +159,8 @@ private:
     HashMap<IDBResourceIdentifier, RefPtr<TransactionOperation>> m_transactionOperationMap;
 
     HashMap<String, RefPtr<IDBObjectStore>> m_referencedObjectStores;
+
+    HashSet<RefPtr<IDBRequest>> m_openRequests;
 };
 
 class TransactionActivator {

diff --git a/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h b/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h
index 87d055c1ceae35bb0897803df4c89000deb7661d..ac606c926a78f09314f4f5ec6e455c622b57114c 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h
+++ b/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h
@@ -48,7 +48,7 @@ public:
     void completed(const IDBResultData& data)
     {
         m_completeFunction(data);
-        m_transaction->scheduleOperationTimer();
+        m_transaction->operationDidComplete(*this);
     }
 
     const IDBResourceIdentifier& identifier() const { return m_identifier; }