Changeset 255329 in webkit

Timestamp:

Jan 28, 2020 5:39:47 PM (4 years ago)

Author:

Chris Dumez

Message:

REGRESSION (r252064): [ Mac iOS ] storage/websql/statement-error-callback.html is timing out flakily
​https://bugs.webkit.org/show_bug.cgi?id=206291
<rdar://problem/58606666>

Reviewed by Maciej Stachowiak.

Source/WebCore:

I found that extending the lifetime of the SQLTransaction by capturing protectedThis in the event loop
task was the source of the flaky timeouts & crashes. Queueing this event loop task in
notifyDatabaseThreadIsShuttingDown() is actually not necessary because calling callErrorCallbackDueToInterruption()
directly would schedule the event loop task for us. Also, in callErrorCallbackDueToInterruption(),
the event loop task only keeps the error callback alive, not just the SQLTransaction object.

No new tests, unskipped existing test.

• Modules/webdatabase/SQLTransaction.cpp:

(WebCore::SQLTransaction::notifyDatabaseThreadIsShuttingDown):

LayoutTests:

Unskip test that is no longer flaky.

• platform/ios/TestExpectations:
• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/webdatabase/SQLTransaction.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-01-28  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r252064): [ Mac iOS ] storage/websql/statement-error-callback.html is timing out flakily
+        https://bugs.webkit.org/show_bug.cgi?id=206291
+        <rdar://problem/58606666>
+
+        Reviewed by Maciej Stachowiak.
+
+        Unskip test that is no longer flaky.
+
+        * platform/ios/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2020-01-28  Jacob Uphoff  <jacob_uphoff@apple.com>
 

trunk/LayoutTests/platform/ios/TestExpectations

@@ -3460 +3460 @@
 webkit.org/b/200043 fast/text/international/system-language/navigator-language [ Pass Failure ]
 
-webkit.org/b/206291 storage/websql/statement-error-callback.html [ Pass Timeout ]
-
 webkit.org/b/203222 svg/wicd/rightsizing-grid.xhtml [ Pass Failure ]
 

trunk/LayoutTests/platform/mac/TestExpectations

@@ -1935 +1935 @@
 webkit.org/b/77568 [ Sierra HighSierra Mojave ] fast/text/locale-shaping-complex.html [ ImageOnlyFailure ]
 
-webkit.org/b/206291 storage/websql/statement-error-callback.html [ Pass Timeout ]
-
 webkit.org/b/203222 svg/wicd/rightsizing-grid.xhtml [ Pass Failure ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-01-28  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r252064): [ Mac iOS ] storage/websql/statement-error-callback.html is timing out flakily
+        https://bugs.webkit.org/show_bug.cgi?id=206291
+        <rdar://problem/58606666>
+
+        Reviewed by Maciej Stachowiak.
+
+        I found that extending the lifetime of the SQLTransaction by capturing protectedThis in the event loop
+        task was the source of the flaky timeouts & crashes. Queueing this event loop task in
+        notifyDatabaseThreadIsShuttingDown() is actually not necessary because calling callErrorCallbackDueToInterruption()
+        directly would schedule the event loop task for us. Also, in callErrorCallbackDueToInterruption(),
+        the event loop task only keeps the error callback alive, not just the SQLTransaction object.
+
+        No new tests, unskipped existing test.
+
+        * Modules/webdatabase/SQLTransaction.cpp:
+        (WebCore::SQLTransaction::notifyDatabaseThreadIsShuttingDown):
+
 2020-01-28  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp

@@ -129 +129 @@
 {
     callOnMainThread([this, protectedThis = makeRef(*this)]() mutable {
-        m_database->document().eventLoop().queueTask(TaskSource::Networking, [this, protectedThis = protectedThis.copyRef()]() mutable {
-            callErrorCallbackDueToInterruption();
-        });
+        callErrorCallbackDueToInterruption();
     });