[webkit-changes] [WebKit/WebKit] 68279f: [ Debug Sequoia ] TestWebKitAPI.WKWebViewCandidate...

[Wenson Hsieh]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 68279ff1ef9c8632e1a07031f592c81900d0a81f
      https://github.com/WebKit/WebKit/commit/68279ff1ef9c8632e1a07031f592c81900d0a81f
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2025-01-02 (Thu, 02 Jan 2025)

  Changed paths:
    M Source/WebKit/UIProcess/PageClient.h
    M Source/WebKit/UIProcess/WebPageProxy.cpp
    M Source/WebKit/UIProcess/WebPageProxy.h
    M Source/WebKit/UIProcess/WebPageProxy.messages.in
    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
    M Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewCandidateTests.mm

  Log Message:
  -----------
  [ Debug Sequoia ] TestWebKitAPI.WKWebViewCandidateTests.SoftSpaceReplacementAfterCandidateInsertionWithoutReplacement is a constant failure
https://bugs.webkit.org/show_bug.cgi?id=280352
rdar://136705852

Reviewed by Abrar Rahman Protyasha.

This API test, which simulates accepting a text candidate that ends with a space and then inserts
another space, is flaky in debug. When this test fails, we get the following sequence of events:

1.  (UI) `WebViewImpl::handleAcceptedCandidate` is invoked, which sets `m_softSpaceRange`.
    `m_isHandlingAcceptedCandidate` is set here.

2.  (WEB) `WebPage::handleAcceptedCandidate` is called, scheduling an editor state update in the
    next layer tree commit in the process due to the selection change.

3.  (UI) `WebPageProxy::didHandleAcceptedCandidate` is called, which unsets
    `m_isHandlingAcceptedCandidate`.

4.  (UI) The next remote layer tree commit (with an editor state) scheduled in step (2) arrives,
    calling `WebViewImpl::selectionDidChange`. Because `m_isHandlingAcceptedCandidate` is `false`,
    `m_softSpaceRange` is set to `(NSNotFound, 0)`.

5.  (UI) The API test calls `WebViewImpl::insertText` with a single space. Since `m_softSpaceRange`
    is already reset, we don't end up replacing the first space with a period.

When this test passes, step (4) happens after step (5), so we replace the soft space. In other
words, this test failure boils down to a race between when the next layer tree commit arrives in
the UI process, and when the `m_isHandlingAcceptedCandidate` flag is unset.

To fix this, we refactor this logic so that `m_isHandlingAcceptedCandidate` is unset after the next
presentation update, to ensure that it remains set until after the next call to `selectionDidChange`
triggered by accepting the text candidate.

* Source/WebKit/UIProcess/PageClient.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didHandleAcceptedCandidate): Deleted.
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/UIProcess/mac/PageClientImplMac.h:
* Source/WebKit/UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::didHandleAcceptedCandidate): Deleted.

Remove this IPC message and plumbing, since it's now unnecessary — this was only used to
(prematurely) reset the value of `m_isHandlingAcceptedCandidate` in the UI process, and also to
facilitate the testing `WKWebView` subclassing hook `-_didHandleAcceptedCandidate`.

* Source/WebKit/UIProcess/mac/WebViewImpl.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::handleAcceptedCandidate):

Use `callAfterNextPresentationUpdate` to unset the flag.

(WebKit::WebViewImpl::didHandleAcceptedCandidate): Deleted.
* Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::handleAcceptedCandidate):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewCandidateTests.mm:
(TEST(WKWebViewCandidateTests, SoftSpaceReplacementAfterCandidateInsertionWithoutReplacement)):

Reenable the test.

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



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