Skip to content

Commit

Permalink
[Site Isolation] Begin implementing window.focus()
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264714
rdar://118307699

Reviewed by Alex Christensen.

Begin implementing window.focus() such that it works as expected when called on a cross-origin opened
window. I filed bugs for future work, like adding the correct security checks.

* Source/WebCore/page/DOMWindow.idl:
* Source/WebCore/page/LocalDOMWindow.idl:
* Source/WebCore/page/RemoteDOMWindow.cpp:
(WebCore::RemoteDOMWindow::focus):
* Source/WebCore/page/RemoteDOMWindow.idl:
* Source/WebCore/page/RemoteFrameClient.h:
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::focusRemoteFrame):
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.messages.in:
* Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp:
(WebKit::WebRemoteFrameClient::focus):
* Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/270673@main
  • Loading branch information
charliewolfe committed Nov 13, 2023
1 parent 923ed51 commit 32658f6
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 5 deletions.
1 change: 1 addition & 0 deletions Source/WebCore/page/DOMWindow.idl
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ interface mixin DOMWindow {
[CallWith=CurrentGlobalObject&IncumbentWindow, DoNotCheckSecurity] undefined postMessage(any message, optional WindowPostMessageOptions options);
[DoNotCheckSecurity, PutForwards=href, LegacyUnforgeable] readonly attribute Location location;
[DoNotCheckSecurity, CallWith=IncumbentDocument] undefined close();
[DoNotCheckSecurity, CallWith=IncumbentWindow] undefined focus();
[DoNotCheckSecurityOnGetter, CustomSetter] attribute WindowProxy? opener;
};
1 change: 0 additions & 1 deletion Source/WebCore/page/LocalDOMWindow.idl
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
attribute DOMString status;
[DoNotCheckSecurity] readonly attribute boolean closed;
undefined stop();
[DoNotCheckSecurity, CallWith=IncumbentWindow] undefined focus();
[DoNotCheckSecurity] undefined blur();

// other browsing contexts
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/page/RemoteDOMWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ bool RemoteDOMWindow::closed() const

void RemoteDOMWindow::focus(LocalDOMWindow&)
{
// FIXME: Implemented this. <rdar://116203970>
// FIXME(264713): Add security checks here equivalent to LocalDOMWindow::focus().
if (m_frame && m_frame->isMainFrame())
m_frame->client().focus();
}

void RemoteDOMWindow::blur()
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/page/RemoteDOMWindow.idl
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
[LegacyUnforgeable, ImplementedAs=self] readonly attribute WindowProxy window;
[Replaceable] readonly attribute WindowProxy self;
readonly attribute boolean closed;
[CallWith=IncumbentWindow] undefined focus();
undefined blur();
[Replaceable, ImplementedAs=self] readonly attribute WindowProxy frames;
[Replaceable] readonly attribute unsigned long length;
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/page/RemoteFrameClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class RemoteFrameClient {
virtual String renderTreeAsText(size_t baseIndent, OptionSet<RenderAsTextFlag>) = 0;
virtual void broadcastFrameRemovalToOtherProcesses() = 0;
virtual void close() = 0;
virtual void focus() = 0;
virtual ~RemoteFrameClient() { }
};

Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/WebPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -2292,6 +2292,8 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
bool shouldAllowAutoFillForCellularIdentifiers() const;
#endif

void broadcastFocusedFrameToOtherProcesses(IPC::Connection&, const WebCore::FrameIdentifier&);

private:
WebPageProxy(PageClient&, WebProcessProxy&, Ref<API::PageConfiguration>&&);
void platformInitialize();
Expand Down Expand Up @@ -2847,8 +2849,6 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ

void dispatchLoadEventToFrameOwnerElement(WebCore::FrameIdentifier);

void broadcastFocusedFrameToOtherProcesses(IPC::Connection&, const WebCore::FrameIdentifier&);

struct Internals;
Internals& internals() { return m_internals; }
const Internals& internals() const { return m_internals; }
Expand Down
16 changes: 16 additions & 0 deletions Source/WebKit/UIProcess/WebProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,22 @@ void WebProcessProxy::closeRemoteFrame(WebCore::FrameIdentifier frameID)
page->closePage();
}

void WebProcessProxy::focusRemoteFrame(WebCore::FrameIdentifier frameID)
{
// FIXME: <rdar://117383252> This, postMessageToRemote, renderTreeAsText, etc. should be messages to the WebPageProxy instead of the process.
// They are more the page doing things than the process.
RefPtr destinationFrame = WebFrameProxy::webFrame(frameID);
if (!destinationFrame || !destinationFrame->isMainFrame())
return;

RefPtr page = destinationFrame->page();
if (!page)
return;

page->broadcastFocusedFrameToOtherProcesses(*connection(), frameID);
page->setFocus(true);
}

void WebProcessProxy::renderTreeAsText(WebCore::FrameIdentifier frameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag> behavior, CompletionHandler<void(String&&)>&& completionHandler)
{
RefPtr frame = WebFrameProxy::webFrame(frameIdentifier);
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/WebProcessProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy {
void didDestroyUserGestureToken(uint64_t);
void postMessageToRemote(WebCore::FrameIdentifier, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&);
void closeRemoteFrame(WebCore::FrameIdentifier);
void focusRemoteFrame(WebCore::FrameIdentifier);
void renderTreeAsText(WebCore::FrameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag>, CompletionHandler<void(String&&)>&&);

bool canBeAddedToWebProcessCache() const;
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/WebProcessProxy.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ messages -> WebProcessProxy LegacyReceiver {

PostMessageToRemote(WebCore::FrameIdentifier identifier, std::optional<WebCore::SecurityOriginData> target, struct WebCore::MessageWithMessagePorts message)
CloseRemoteFrame(WebCore::FrameIdentifier identifier)
FocusRemoteFrame(WebCore::FrameIdentifier identifier)

RenderTreeAsText(WebCore::FrameIdentifier identifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag> behavior) -> (String renderTreeDump) Synchronous
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,9 @@ void WebRemoteFrameClient::close()
WebProcess::singleton().send(Messages::WebProcessProxy::CloseRemoteFrame(m_frame->frameID()), 0);
}

void WebRemoteFrameClient::focus()
{
WebProcess::singleton().send(Messages::WebProcessProxy::FocusRemoteFrame(m_frame->frameID()), 0);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class WebRemoteFrameClient final : public WebCore::RemoteFrameClient, public Web
String renderTreeAsText(size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag>) final;
void broadcastFrameRemovalToOtherProcesses() final;
void close() final;
void focus() final;

ScopeExit<Function<void()>> m_frameInvalidator;
};
Expand Down
41 changes: 41 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1570,4 +1570,45 @@ HTTPServer server({
EXPECT_FALSE(canLoadURLInIFrame(@"/always_blocked"));
}

TEST(SiteIsolation, FocusOpenedWindow)
{
auto openerHTML = "<script>"
" let w = window.open('https://domain2.com/opened');"
"</script>"_s;
HTTPServer server({
{ "/example"_s, { openerHTML } },
{ "/opened"_s, { ""_s } }
}, HTTPServer::Protocol::HttpsProxy);
auto [opener, opened] = openerAndOpenedViews(server);

RetainPtr<WKFrameInfo> openerInfo;
RetainPtr<WKFrameInfo> openedInfo;
auto getUpdatedFrameInfo = [&] (WKWebView *openerWebView, WKWebView *openedWebView) {
__block bool done = false;
[openerWebView _frames:^(_WKFrameTreeNode *mainFrame) {
openerInfo = mainFrame.info;
done = true;
}];
Util::run(&done);

done = false;
[openedWebView _frames:^(_WKFrameTreeNode *mainFrame) {
openedInfo = mainFrame.info;
done = true;
}];
Util::run(&done);
};

getUpdatedFrameInfo(opener.webView.get(), opened.webView.get());
EXPECT_FALSE([openerInfo _isFocused]);
EXPECT_FALSE([openedInfo _isFocused]);

[opener.webView.get() evaluateJavaScript:@"w.focus()" completionHandler:nil];

do {
getUpdatedFrameInfo(opener.webView.get(), opened.webView.get());
} while (![openedInfo _isFocused]);
EXPECT_FALSE([openerInfo _isFocused]);
}

}

0 comments on commit 32658f6

Please sign in to comment.