[webkit-changes] [WebKit/WebKit] bdd0ea: [mac] WebDriver sometimes taking screenshots with ...

[Qianlang Chen]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: bdd0eaf1d28f834bdc3fab3e656486522f3b301f
      https://github.com/WebKit/WebKit/commit/bdd0eaf1d28f834bdc3fab3e656486522f3b301f
  Author: Qianlang Chen <qianlangchen at apple.com>
  Date:   2024-08-06 (Tue, 06 Aug 2024)

  Changed paths:
    M Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
    M Source/WebKit/UIProcess/Automation/WebAutomationSession.h
    M Source/WebKit/UIProcess/PageClient.h
    M Source/WebKit/UIProcess/ViewSnapshotStore.h
    M Source/WebKit/UIProcess/WebPageProxy.cpp
    M Source/WebKit/UIProcess/WebPageProxy.h
    M Source/WebKit/UIProcess/mac/PageClientImplMac.h
    M Source/WebKit/UIProcess/mac/PageClientImplMac.mm
    M Source/WebKit/UIProcess/mac/WebViewImpl.h
    M Source/WebKit/UIProcess/mac/WebViewImpl.mm

  Log Message:
  -----------
  [mac] WebDriver sometimes taking screenshots with transparent grey line at the top and no rounded corners
rdar://116020785
https://bugs.webkit.org/show_bug.cgi?id=262078

Reviewed by Aditya Keerthi, Abrar Rahman Protyasha and BJ Burg.

Our WebDriver currently takes viewport screenshots with the
`static takeWindowSnapshot` function in WebViewImpl. What that function
currently does is:

   1. Try taking a screenshot using the hardware capture path, with the
      CGSHWCaptureWindowList function and the
      kCGSCaptureIgnoreGlobalClipShape option to avoid the rounded
      corners.
   2. The above function may fail (e.g. due to the window being off-
      screen, as noted in the code comments), and if it does, take
      a screenshot using the non-hardware capture path with
      CGWindowListCreateImage and kCGWindowImageBoundsIgnoreFraming.

Due to the kCGSCaptureIgnoreGlobalClipShape option currently bugged
and having no effect, if hardware capturing does succeed, we get a
screenshot with the rounded corners, and if it fails we get no
rounded corners, which contributes toward the tests' flakiness.

   - This inconsistency is also observable from the screenshots taken
     when performing a swipe to go back/forward in the browser, though
     the consequence there is less severe than failing WPT tests.

To address that, we force WebDriver to always fall back to the non-
hardware capture path, taking viewport screenshots with
CGWindowListCreateImage, to rule out any flakiness caused by
CGSHWCaptureWindowList occasionally failing.

   - The reason we began to prefer the hardware capture path with
     CGSHWCaptureWindowList is because CGWindowListCreateImage had a
     performance issue where it might hang. Source: https://github.com/WebKit/WebKit/commit/8f81edf40d069cacf6107254a535a812d5978dea
   - However, the hang has been addressed internally and no longer
     occurs today. My local testing instead shows no noticeable
     difference in speed between CGWindowListCreateImage and
     CGSHWCaptureWindowList.
   - Though, CGWindowListCreateImage has been deprecated in favor of the
     ScreenCaptureKit, but it's still functional for now. So, I believe
     this patch is a reasonable workaround at least until the hardware
     capturing bug gets fixed, in exchange for a more consistent
     performance in WPT.

Despite this patch aims to improve WPT results, it's hard to pinpoint
which specific tests will progress, as all tests that take viewport
screenshots are currently flaky due to the bug. However, I can observe
some tests progressing locally. With this command inside the WPT repo:

    ./wpt run safari ./css/css-contain --webdriver-binary="..."

556 tests were run, where 140-150 tests fail without this patch, and
this patch makes it so that exactly 51 tests were still failing (about
90 tests progressed) and no tests regressed.

* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::takeViewSnapshot):
   - Add the option to always use the non-hardware screen capture path
     when taking a window snapshot.

* Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::takeScreenshot):
* Source/WebKit/UIProcess/Automation/WebAutomationSession.h:
* Source/WebKit/UIProcess/PageClient.h:
* Source/WebKit/UIProcess/ViewSnapshotStore.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::takeViewSnapshot):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/mac/PageClientImplMac.h:
* Source/WebKit/UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::takeViewSnapshot):
* Source/WebKit/UIProcess/mac/WebViewImpl.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::takeWindowSnapshot):
   - Allow WebDriver to make use of this change when taking a viewport
     screenshot.

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



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications