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

Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -1277,26 +1277,16 @@ - (void)takeSnapshotWithConfiguration:(WKSnapshotConfiguration *)snapshotConfigu
         return;
     }
 
-    _page->callAfterNextPresentationUpdate([callSnapshotRect = WTFMove(callSnapshotRect), handler, page = Ref { *_page }] () mutable {
-
+    _page->callAfterNextPresentationUpdateAndLayerCommit([callSnapshotRect = WTFMove(callSnapshotRect), handler, page = Ref { *_page }] () mutable {
         if (!page->hasRunningProcess()) {
             tracePoint(TakeSnapshotEnd, snapshotFailedTraceValue);
             handler(nil, createNSError(WKErrorUnknown).get());
             return;
         }
 
-        // Create an implicit transaction to ensure a commit will happen next.
-        [CATransaction activate];
-
-        // Wait for the next flush to ensure the latest IOSurfaces are pushed to backboardd before taking the snapshot.
-        [CATransaction addCommitHandler:[callSnapshotRect = WTFMove(callSnapshotRect)]() mutable {
-            // callSnapshotRect() calls the client callback which may call directly or indirectly addCommitHandler.
-            // It is prohibited by CA to add a commit handler while processing a registered commit handler.
-            // So postpone calling callSnapshotRect() till CATransaction processes its commit handlers.
-            dispatch_async(dispatch_get_main_queue(), [callSnapshotRect = WTFMove(callSnapshotRect)] {
-                callSnapshotRect();
-            });
-        } forPhase:kCATransactionPhasePostCommit];
+        dispatch_async(dispatch_get_main_queue(), [callSnapshotRect = WTFMove(callSnapshotRect)] {
+            callSnapshotRect();
+        });
     });
 #endif
 }

Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm

@@ -77,6 +77,7 @@
 #import <pal/spi/ios/BrowserEngineKitSPI.h>
 #import <pal/spi/mac/QuarantineSPI.h>
 #import <wtf/BlockPtr.h>
+#import <wtf/CompletionHandler.h>
 #import <wtf/SoftLinking.h>
 #import <wtf/cf/TypeCastsCF.h>
 #import <wtf/cocoa/SpanCocoa.h>
@@ -167,6 +168,21 @@ static bool exceedsRenderTreeSizeSizeThreshold(uint64_t thresholdSize, uint64_t
     protectedPageClient()->layerTreeCommitComplete();
 }
 
+void WebPageProxy::callAfterNextPresentationUpdateAndLayerCommit(CompletionHandler<void()>&& callback)
+{
+    callAfterNextPresentationUpdate([callback = WTFMove(callback)]() mutable {
+        // Create an implicit transaction to ensure a commit will happen next.
+        [CATransaction activate];
+
+        auto completionBlock = makeBlockPtr([callback = WTFMove(callback)]() mutable {
+            callback();
+        });
+
+        // Wait for the next flush to ensure the latest IOSurfaces are pushed to backboardd before taking the snapshot.
+        [CATransaction addCommitHandler:completionBlock.get() forPhase:kCATransactionPhasePostCommit];
+    });
+}
+
 #if ENABLE(DATA_DETECTION)
 
 void WebPageProxy::setDataDetectionResult(const DataDetectionResult& dataDetectionResult)

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -5698,7 +5698,7 @@ void WebPageProxy::forceRepaint(CompletionHandler<void()>&& callback)
         RefPtr protectedThis = weakThis.get();
         if (!protectedThis)
             return callback();
-        protectedThis->callAfterNextPresentationUpdate(WTFMove(callback));
+        protectedThis->callAfterNextPresentationUpdateAndLayerCommit(WTFMove(callback));
     });
     forEachWebContentProcess([&](auto& webProcess, auto pageID) {
         webProcess.sendWithAsyncReply(Messages::WebPage::ForceRepaint(), [aggregator] { }, pageID);
@@ -11901,6 +11901,13 @@ void WebPageProxy::callAfterNextPresentationUpdate(CompletionHandler<void()>&& c
 }
 #endif
 
+#if !PLATFORM(COCOA)
+void WebPageProxy::callAfterNextPresentationUpdateAndLayerCommit(CompletionHandler<void()>&& callback)
+{
+    return callAfterNextPresentationUpdate(WTFMove(callback));
+}
+#endif
+
 void WebPageProxy::setShouldScaleViewToFitDocument(bool shouldScaleViewToFitDocument)
 {
     if (m_shouldScaleViewToFitDocument == shouldScaleViewToFitDocument)

Source/WebKit/UIProcess/WebPageProxy.h

@@ -1863,6 +1863,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
     // For testing
     void clearWheelEventTestMonitor();
     void callAfterNextPresentationUpdate(CompletionHandler<void()>&&);
+    void callAfterNextPresentationUpdateAndLayerCommit(CompletionHandler<void()>&&);
 
     void didReachLayoutMilestone(OptionSet<WebCore::LayoutMilestone>);