Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REGRESSION (UI-side compositing?): Sporadic failures in layout tests due to wrong test images #26214

Conversation

smfr
Copy link
Contributor

@smfr smfr commented Mar 21, 2024

b289d5f

REGRESSION (UI-side compositing?): Sporadic failures in layout tests due to wrong test images
https://bugs.webkit.org/show_bug.cgi?id=271348
rdar://120050275

Reviewed by Tim Horton.

When we take snapshots of layout tests, we wait for a `callAfterNextPresentationUpdate()`, which
waits for a transaction to come from the web process. However, it doesn't wait for the subsequent
Core Animation commit, which actually pushes changes to WindowServer. Yet the snapshot we do is
a WindowServer snapshot (`PlatformWebView::windowSnapshotImage()`), so we can sometimes capture state that is stale.

We don't actually know when our new content gets to WindowServer and will show up in a snapshot,
but we can reduce the raciness by at least waiting for the Core Animation commit to complete.
So add `callAfterNextPresentationUpdateAndLayerCommit()` and use it `WebPageProxy::forceRepaint()`.
Also update `-[WKWebView takeSnapshotWithConfiguration:]` to use it.

* Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):
* Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::WebPageProxy::callAfterNextPresentationUpdateAndLayerCommit):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::forceRepaint):
(WebKit::WebPageProxy::callAfterNextPresentationUpdateAndLayerCommit):
* Source/WebKit/UIProcess/WebPageProxy.h:

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

ff16eed

Misc iOS, tvOS & watchOS macOS Linux Windows
⏳ πŸ§ͺ style ⏳ πŸ›  ios ⏳ πŸ›  mac ⏳ πŸ›  wpe ⏳ πŸ›  wincairo
⏳ πŸ§ͺ bindings ⏳ πŸ›  ios-sim ⏳ πŸ›  mac-AS-debug ⏳ πŸ§ͺ wpe-wk2
⏳ πŸ§ͺ webkitperl ⏳ πŸ§ͺ ios-wk2 ⏳ πŸ§ͺ api-mac ⏳ πŸ§ͺ api-wpe
⏳ πŸ§ͺ webkitpy ⏳ πŸ§ͺ ios-wk2-wpt ⏳ πŸ§ͺ mac-wk1 ⏳ πŸ›  wpe-skia
⏳ πŸ›  πŸ§ͺ jsc ⏳ πŸ§ͺ api-ios ⏳ πŸ§ͺ mac-wk2 ⏳ πŸ›  gtk
⏳ πŸ›  πŸ§ͺ jsc-arm64 ⏳ πŸ›  tv ⏳ πŸ§ͺ mac-AS-debug-wk2 ⏳ πŸ§ͺ gtk-wk2
⏳ πŸ§ͺ services ⏳ πŸ›  tv-sim ⏳ πŸ§ͺ mac-wk2-stress ⏳ πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge ⏳ πŸ›  watch ⏳ πŸ›  jsc-armv7
⏳ πŸ›  watch-sim ⏳ πŸ§ͺ jsc-armv7-tests

@smfr smfr requested a review from cdumez as a code owner March 21, 2024 00:02
@smfr smfr self-assigned this Mar 21, 2024
@smfr smfr added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Mar 21, 2024
@smfr smfr added the merge-queue Applied to send a pull request to merge-queue label Mar 21, 2024
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION-UI-side-compositing-Sporadic-failures-in-layout-tests-due-to-wrong-test-images branch from 7bca848 to ff16eed Compare March 21, 2024 17:50
@smfr smfr added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Mar 21, 2024
…due to wrong test images

https://bugs.webkit.org/show_bug.cgi?id=271348
rdar://120050275

Reviewed by Tim Horton.

When we take snapshots of layout tests, we wait for a `callAfterNextPresentationUpdate()`, which
waits for a transaction to come from the web process. However, it doesn't wait for the subsequent
Core Animation commit, which actually pushes changes to WindowServer. Yet the snapshot we do is
a WindowServer snapshot (`PlatformWebView::windowSnapshotImage()`), so we can sometimes capture state that is stale.

We don't actually know when our new content gets to WindowServer and will show up in a snapshot,
but we can reduce the raciness by at least waiting for the Core Animation commit to complete.
So add `callAfterNextPresentationUpdateAndLayerCommit()` and use it `WebPageProxy::forceRepaint()`.
Also update `-[WKWebView takeSnapshotWithConfiguration:]` to use it.

* Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):
* Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::WebPageProxy::callAfterNextPresentationUpdateAndLayerCommit):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::forceRepaint):
(WebKit::WebPageProxy::callAfterNextPresentationUpdateAndLayerCommit):
* Source/WebKit/UIProcess/WebPageProxy.h:

Canonical link: https://commits.webkit.org/276494@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION-UI-side-compositing-Sporadic-failures-in-layout-tests-due-to-wrong-test-images branch from ff16eed to b289d5f Compare March 21, 2024 21:11
@webkit-commit-queue
Copy link
Collaborator

Committed 276494@main (b289d5f): https://commits.webkit.org/276494@main

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

@webkit-commit-queue webkit-commit-queue merged commit b289d5f into WebKit:main Mar 21, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 21, 2024
@smfr smfr deleted the eng/REGRESSION-UI-side-compositing-Sporadic-failures-in-layout-tests-due-to-wrong-test-images branch May 9, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
4 participants