Skip to content

REGRESSION(291435@main...291445@main): [iOS Debug] TestWebKitAPI.IndexedDB.IndexedDBSuspendImminently is a failure/timeout (flaky in EWS)#42962

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
achristensen07:eng/REGRESSION-291435-main-291445-main-iOS-Debug-TestWebKitAPI-IndexedDB-IndexedDBSuspendImminently-is-a-failure-timeout-flaky-in-EWS
Mar 25, 2025
Merged

REGRESSION(291435@main...291445@main): [iOS Debug] TestWebKitAPI.IndexedDB.IndexedDBSuspendImminently is a failure/timeout (flaky in EWS)#42962
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
achristensen07:eng/REGRESSION-291435-main-291445-main-iOS-Debug-TestWebKitAPI-IndexedDB-IndexedDBSuspendImminently-is-a-failure-timeout-flaky-in-EWS

Conversation

@achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented Mar 24, 2025

a899e6f

REGRESSION(291435@main...291445@main): [iOS Debug] TestWebKitAPI.IndexedDB.IndexedDBSuspendImminently is a failure/timeout (flaky in EWS)
https://bugs.webkit.org/show_bug.cgi?id=290203
rdar://147599411

Reviewed by Sihui Liu.

TestWebKitAPI::Util::run doesn't necessarily stop everything when the condition becomes true.
There is a chance that more work can be done before it returns.
When multiple messages are awaited, sometimes we'll wait for the first one and the first
and second one are received before TestWebKitAPI::Util::run returns, then when we try to wait
for the second one, no more messages come.  I introduce TestScriptMessageHandler with a Deque
to make it more robust.  If it has already received a message, it just returns it immediately.
If it receives too many messages, it'll hang on to them until later.
A similar techinque is already used in TestWKWebView.waitForMessages

* Tools/TestWebKitAPI/SourcesCocoa.txt:
* Tools/TestWebKitAPI/TestScriptMessageHandler.mm: Added.
(-[TestScriptMessageHandler waitForMessage]):
(-[TestScriptMessageHandler userContentController:didReceiveScriptMessage:]):
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBSuspendImminently.mm:
(TEST(IndexedDB, IndexedDBSuspendImminently)):
(TEST(IndexedDB, SuspendImminentlyForThirdPartyDatabases)):
(-[IndexedDBSuspendImminentlyMessageHandler userContentController:didReceiveScriptMessage:]): Deleted.
(runUntilMessageReceived): Deleted.
(keepNetworkProcessActive): Deleted.
(TEST(IndexedDB, DISABLED_IndexedDBSuspendImminently)): Deleted.
* Tools/TestWebKitAPI/cocoa/TestScriptMessageHandler.h: Added.
* Tools/TestWebKitAPI/cocoa/TestScriptMessageHandler.mm: Added.
(-[TestScriptMessageHandler waitForMessage]):
(-[TestScriptMessageHandler userContentController:didReceiveScriptMessage:]):

Canonical link: https://commits.webkit.org/292658@main

2c313a3

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
loading 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@achristensen07 achristensen07 self-assigned this Mar 24, 2025
@achristensen07 achristensen07 added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Mar 24, 2025
Comment on lines -51 to -53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just do here:

receivedScriptMessage = true;
scriptMessages.append(message);

And use [getNextMessage() body] below?
(There is a getNextMessage in DeprecatedGlobalValues.h)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but this is a non-global replacement for DeprecatedGlobalValues. I think it would be better to replace all use of getNextMessage() with TestScriptMessageHandler

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 25, 2025
@achristensen07 achristensen07 added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Mar 25, 2025
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION-291435-main-291445-main-iOS-Debug-TestWebKitAPI-IndexedDB-IndexedDBSuspendImminently-is-a-failure-timeout-flaky-in-EWS branch from 2c313a3 to 10793fb Compare March 25, 2025 18:16
…xedDB.IndexedDBSuspendImminently is a failure/timeout (flaky in EWS)

https://bugs.webkit.org/show_bug.cgi?id=290203
rdar://147599411

Reviewed by Sihui Liu.

TestWebKitAPI::Util::run doesn't necessarily stop everything when the condition becomes true.
There is a chance that more work can be done before it returns.
When multiple messages are awaited, sometimes we'll wait for the first one and the first
and second one are received before TestWebKitAPI::Util::run returns, then when we try to wait
for the second one, no more messages come.  I introduce TestScriptMessageHandler with a Deque
to make it more robust.  If it has already received a message, it just returns it immediately.
If it receives too many messages, it'll hang on to them until later.
A similar techinque is already used in TestWKWebView.waitForMessages

* Tools/TestWebKitAPI/SourcesCocoa.txt:
* Tools/TestWebKitAPI/TestScriptMessageHandler.mm: Added.
(-[TestScriptMessageHandler waitForMessage]):
(-[TestScriptMessageHandler userContentController:didReceiveScriptMessage:]):
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBSuspendImminently.mm:
(TEST(IndexedDB, IndexedDBSuspendImminently)):
(TEST(IndexedDB, SuspendImminentlyForThirdPartyDatabases)):
(-[IndexedDBSuspendImminentlyMessageHandler userContentController:didReceiveScriptMessage:]): Deleted.
(runUntilMessageReceived): Deleted.
(keepNetworkProcessActive): Deleted.
(TEST(IndexedDB, DISABLED_IndexedDBSuspendImminently)): Deleted.
* Tools/TestWebKitAPI/cocoa/TestScriptMessageHandler.h: Added.
* Tools/TestWebKitAPI/cocoa/TestScriptMessageHandler.mm: Added.
(-[TestScriptMessageHandler waitForMessage]):
(-[TestScriptMessageHandler userContentController:didReceiveScriptMessage:]):

Canonical link: https://commits.webkit.org/292658@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION-291435-main-291445-main-iOS-Debug-TestWebKitAPI-IndexedDB-IndexedDBSuspendImminently-is-a-failure-timeout-flaky-in-EWS branch from 10793fb to a899e6f Compare March 25, 2025 18:17
@webkit-commit-queue
Copy link
Collaborator

Committed 292658@main (a899e6f): https://commits.webkit.org/292658@main

Reviewed commits have been landed. Closing PR #42962 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit a899e6f into WebKit:main Mar 25, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments