[webkit-changes] [WebKit/WebKit] bf1345: [Site Isolation] Make `findString:` correctly cycl...

Charlie Wolfe noreply at github.com
Thu Jan 25 10:29:51 PST 2024


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: bf1345fe689b99d16d63b52702777db6c79e1717
      https://github.com/WebKit/WebKit/commit/bf1345fe689b99d16d63b52702777db6c79e1717
  Author: Charlie Wolfe <charliew at apple.com>
  Date:   2024-01-25 (Thu, 25 Jan 2024)

  Changed paths:
    M LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node.html
    M LayoutTests/platform/ios/fast/scrolling/find-text-in-subframe.html
    M Source/WebCore/editing/Editor.cpp
    M Source/WebCore/editing/FindOptions.h
    M Source/WebCore/page/Page.cpp
    M Source/WebCore/page/Page.h
    M Source/WebKit/Shared/WebFindOptions.cpp
    M Source/WebKit/Shared/WebFindOptions.h
    M Source/WebKit/Shared/WebFindOptions.serialization.in
    M Source/WebKit/Sources.txt
    A Source/WebKit/UIProcess/FindStringCallbackAggregator.cpp
    A Source/WebKit/UIProcess/FindStringCallbackAggregator.h
    M Source/WebKit/UIProcess/WebFrameProxy.cpp
    M Source/WebKit/UIProcess/WebFrameProxy.h
    M Source/WebKit/UIProcess/WebPageProxy.cpp
    M Source/WebKit/UIProcess/WebPageProxy.h
    M Source/WebKit/UIProcess/WebProcessPool.cpp
    M Source/WebKit/UIProcess/WebProcessPool.h
    M Source/WebKit/WebKit.xcodeproj/project.pbxproj
    M Source/WebKit/WebProcess/WebPage/FindController.cpp
    M Source/WebKit/WebProcess/WebPage/FindController.h
    M Source/WebKit/WebProcess/WebPage/WebPage.cpp
    M Source/WebKit/WebProcess/WebPage/WebPage.h
    M Source/WebKit/WebProcess/WebPage/WebPage.messages.in
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm
    M Tools/TestWebKitAPI/cocoa/TestWKWebView.h
    M Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

  Log Message:
  -----------
  [Site Isolation] Make `findString:` correctly cycle through matches and update selection
https://bugs.webkit.org/show_bug.cgi?id=267908
rdar://121411740

Reviewed by Alex Christensen.

This patch adds initial support for the UI process to coordinate `findString:` across multiple web
processes.

To make this work with site isolation, I needed to:
- Search all web processes without setting selection and have them reply with the identifier of the frame
  containing the string and if the last match was cycled.
- Traverse the frame tree in the UI process to determine which frame is closest to the focused frame.
- Consider when all matches have been cycled in a process and when all matches have been cycled in the
  page.
- Change logic in the web process to consider an out-of-process focused or main frame during frame
  traversal.

This assumes that all web processes will respond. There still needs to be work to make this robust
against unresponsive web processes. More details below.

* LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node.html:
* LayoutTests/platform/ios/fast/scrolling/find-text-in-subframe.html:
Wait for a presentation update on some layout tests using findString.

* Source/WebCore/editing/Editor.cpp:
(WebCore::Editor::findString):
* Source/WebCore/editing/EditorCommand.cpp:
(WebCore::executeFindString):
* Source/WebCore/editing/FindOptions.h:
* Source/WebCore/page/LocalDOMWindow.cpp:
(WebCore::LocalDOMWindow::find const):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::findString):
* Source/WebCore/page/Page.h:
`FocusController::focusedOrMainFrame()` will return the root frame if the focused and main frame are both
remote. This means that if there is a local frame containing a match between the root frame and the
out-of-process focused frame, any local frame following the focused remote frame will not be searched. To
fix this, we should always begin the frame traversal at the focused or main frame, even if they are
out-of-process.

We should only update the focused frame if selection is being set.

Also return the identifier of the frame containing the match. std::nullopt indicates no match.

* Source/WebKit/Shared/WebFindOptions.cpp:
(WebKit::core):
* Source/WebKit/Shared/WebFindOptions.h:
* Source/WebKit/Shared/WebFindOptions.serialization.in:
Add a `SetSelection` value to `FindOptions` to indicate when we only want to search the frame.

* Source/WebKit/Sources.txt:
* Source/WebKit/UIProcess/FindStringCallbackAggregator.cpp: Added.
(WebKit::FindStringCallbackAggregator::create):
(WebKit::FindStringCallbackAggregator::foundString):
This function is called in response to each FindString IPC message that does not set selection.

(WebKit::FindStringCallbackAggregator::~FindStringCallbackAggregator):
Traverse the frame tree to find the frame closest to the focused frame containing a match. If findString
has cycled through all matches in the process but not the page the frame should not be considered. If the
frame is in a different process than the frame which previously had focus, send an IPC message to clear
selection from the previously focused frame.

(WebKit::FindStringCallbackAggregator::FindStringCallbackAggregator):

* Source/WebKit/UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::didCreateSubframe):
(WebKit::WebFrameProxy::traverseNext const):
(WebKit::WebFrameProxy::traversePrevious):
(WebKit::WebFrameProxy::isDescendantOf const):
(WebKit::WebFrameProxy::deepLastChild):
(WebKit::WebFrameProxy::firstChild const):
(WebKit::WebFrameProxy::lastChild const):
* Source/WebKit/UIProcess/WebFrameProxy.h:
(WebKit::WebFrameProxy::parentFrame const):
(WebKit::WebFrameProxy::nextSibling const):
(WebKit::WebFrameProxy::previousSibling const):
(WebKit::WebFrameProxy::parentFrame): Deleted.
Added member variables and functions for traversing the frame tree, designed to behave similarly to the
frame traversal functions in `FrameTree`.

* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::findString):
* Source/WebKit/UIProcess/WebPageProxy.h:

* Source/WebKit/UIProcess/WebProcessPool.cpp:
* Source/WebKit/UIProcess/WebProcessPool.h:
There were two `audibleActivityClearDelay`` variables that started conflicting due to the unified sources
shift. I moved this one to fix it.

* Source/WebKit/WebKit.xcodeproj/project.pbxproj:

* Source/WebKit/WebProcess/WebPage/FindController.cpp:
* Source/WebKit/WebProcess/WebPage/FindController.h:
(WebKit::FindController::updateFindUIAfterPageScroll):
Selection should not be cleared on searches where selection cannot be set.

(WebKit::FindController::findString):
Pass the frame identifier and didWrap to the completion handler instead of whether a match was found.

* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::findStringFromInjectedBundle):
(WebKit::WebPage::findString):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::siteIsolatedViewAndDelegate):
(TestWebKitAPI::TEST):
Add tests to verify `findString:` correctly updates selection on pages with various frame orderings.

* Tools/TestWebKitAPI/cocoa/TestWKWebView.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView selectionRangeHasStartOffset:endOffset:inFrame:])
Add a function similar to the existing `selectionRangeHasStartOffset:endOffset:`, used to check selection
within a frame.

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




More information about the webkit-changes mailing list