Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 68279ff1ef9c8632e1a07031f592c81900d0a81f
https://github.com/WebKit/WebKit/commit/68279ff1ef9c8632e1a07031f592c81900d0a81f
Author: Wenson Hsieh <[email protected]>
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
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
