Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 8dcff579b57ecdd441f9e2b54a54299a57c048ed
https://github.com/WebKit/WebKit/commit/8dcff579b57ecdd441f9e2b54a54299a57c048ed
Author: Alex Christensen <[email protected]>
Date: 2023-11-13 (Mon, 13 Nov 2023)
Changed paths:
M
LayoutTests/http/tests/site-isolation/frame-access-after-window-open-expected.txt
M LayoutTests/http/tests/site-isolation/frame-access-after-window-open.html
M LayoutTests/http/tests/site-isolation/resources/opened-iframe-2.html
M LayoutTests/http/tests/site-isolation/resources/opened-iframe.html
M LayoutTests/platform/ios/TestExpectations
M LayoutTests/platform/mac/TestExpectations
M Source/WebCore/bindings/js/JSLocalDOMWindowCustom.cpp
M Source/WebCore/bindings/js/JSRemoteDOMWindowCustom.cpp
M Source/WebCore/page/DOMWindow.idl
M Source/WebCore/page/Frame.cpp
M Source/WebCore/page/Frame.h
M Source/WebCore/page/LocalDOMWindow.cpp
M Source/WebCore/page/LocalDOMWindow.idl
M Source/WebCore/page/LocalFrame.h
M Source/WebCore/page/RemoteDOMWindow.cpp
M Source/WebCore/page/RemoteDOMWindow.h
M Source/WebCore/page/RemoteDOMWindow.idl
M Source/WebCore/page/RemoteFrame.cpp
M Source/WebCore/page/RemoteFrame.h
M Source/WebKit/UIProcess/WebFrameProxy.cpp
M Source/WebKit/UIProcess/WebPageProxy.cpp
M Source/WebKit/UIProcess/WebPageProxy.h
M Source/WebKit/WebProcess/WebPage/WebFrame.cpp
M Source/WebKit/WebProcess/WebPage/WebPage.cpp
Log Message:
-----------
http/tests/site-isolation/frame-access-after-window-open.html should work
with site isolation enabled
https://bugs.webkit.org/show_bug.cgi?id=264653
rdar://118064120
Reviewed by Pascoe.
The test uses window.parent.opener to go from an iframe, through a RemoteFrame
parent,
to a LocalFrame opener to access properties. It also uses openedWindow[0] to
reach the
other direction to access the opened page's iframe. In order to get this to
work, I had to fix
and implement several things:
1. I had to implement JSRemoteDOMWindow::getOwnPropertySlotByIndex, which I did
enough to make
this case work and I filed bugs for future work. It needs to access the nth
iframe of the window.
2. I moved the bindings of window.opener from LocalDOMWindow and
RemoteDOMWindow to the shared
location of DOMWindow.idl. I used the version from LocalDOMWindow because that
is the correct
version that has been used for decades. RemoteDOMWindow is new to site
isolation.
3. I added a new parameter to RemoteFrame::createMainFrame to set the opener of
a RemoteFrame
when transitioning a LocalFrame to a RemoteFrame in
WebFrame::didCommitLoadInAnotherProcess
so that RemoteDOMWindow::opener has something to return. When you call
window.open, it first
creates a LocalFrame that loads about:blank then starts a provisional load in
another process,
and once that load commits by receiving the first bytes after the HTTP header,
that LocalFrame
becomes a RemoteFrame and the contents of the other process are shown, but the
opener is not
changed by this transition.
4. I made all uses of WebPageProxy::forEachWebContentProcess get callbacks with
not only the
WebProcessProxy, but also the PageIdentifier that can be used to communicate
with the WebPage
in that particular process. If you do anything with a WebPageProxy after
window.open, it likely
has a different PageIdentifier in the process where it is a LocalFrame compared
to the process(es)
where it is a RemoteFrame, and we need to use the correct identifier for the
process to which
we are sending a message.
5. Similarly, I added WebPageProxy::webPageIDInProcessForDomain to get the
webPageID to be used
to communicate with a WebPage in the particular process for that domain. Until
now we got
lucky because we were able to just use WebPageProxy::webPageID() for all the
tested behavior,
but with more involved tests after window.open we need this additional
bookkeeping to make
IPC work successfully.
6. I added some assertions in WebPage.cpp that were useful in noticing when I
had sent messages
to the wrong page but with the correct FrameIdentifier. A WebFrame can be
thought of as being
owned by a WebPage, but the way it is currently implemented we only need a
FrameIdentifier to
find a WebFrame in a process, and it will be found even if we mixed up and sent
a message to the
wrong WebPage. These assertions will prevent bookkeeping erros like the ones I
made locally
when developing this PR.
7. There was already a call to Page::setMainFrame in
WebFrame::transitionToLocal when the main
frame switches from being a RemoteFrame to being a LocalFrame, but we were
missing on in
WebFrame::didCommitLoadInAnotherProcess when the main frame switches the other
way. That caused
some issues in this situation because we actually do things with the Page later
in the test
and we needed it to have the correct main frame or strange things were broken.
8. I changed Frame's m_mainFrame from a C++ reference to a safe CheckedRef to
prevent any security
issues in case of future mistakes in this area.
9. In LocalDOMWindow::open we would return early if the main frame wasn't a
LocalFrame because
we need the LocalFrame's Document's DocumentLoader to make WKContentRuleList
work correctly.
I filed a bug to get this working and made it proceed when site isolation has
made the main
frame a RemoteFrame. It should be easy to make WKContentRuleList work in this
case because
we have Page::mainFrameURL, but that will be done in a separate PR. This one's
big enough.
10. The test needed a few tweaks because I also haven't implemented window.open
in named frames
with site isolation yet, so I made the test run successfully and test all the
changes I made
in this PR but still have room for that PR to come in the future and make the
test behave
exactly how it did before site isolation. Also, I took the behavior of the
iframes and put
them in onload handlers in order for the callback sequence to be more
deterministic to help
me debug what is going on during the tests in this and future PRs. This change
did not change
what was being tested or any results of the tests.
And that's all it takes to make this test mostly work with site isolation!
Thanks for reading.
*
LayoutTests/http/tests/site-isolation/frame-access-after-window-open-expected.txt:
* LayoutTests/http/tests/site-isolation/frame-access-after-window-open.html:
* LayoutTests/http/tests/site-isolation/resources/opened-iframe-2.html:
* LayoutTests/http/tests/site-isolation/resources/opened-iframe.html:
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac/TestExpectations:
* Source/WebCore/bindings/js/JSLocalDOMWindowCustom.cpp:
(WebCore::JSLocalDOMWindow::getOwnPropertySlotByIndex):
* Source/WebCore/bindings/js/JSRemoteDOMWindowCustom.cpp:
(WebCore::JSRemoteDOMWindow::getOwnPropertySlotByIndex):
(WebCore::JSRemoteDOMWindow::setOpener):
* Source/WebCore/page/DOMWindow.idl:
* Source/WebCore/page/LocalDOMWindow.idl:
* Source/WebCore/page/RemoteDOMWindow.cpp:
(WebCore::RemoteDOMWindow::setOpener):
* Source/WebCore/page/RemoteDOMWindow.h:
* Source/WebCore/page/RemoteDOMWindow.idl:
* Source/WebCore/page/RemoteFrame.cpp:
(WebCore::RemoteFrame::createMainFrame):
(WebCore::RemoteFrame::RemoteFrame):
* Source/WebCore/page/RemoteFrame.h:
* Source/WebKit/UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::prepareForProvisionalNavigationInProcess):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::dispatchActivityStateChange):
(WebKit::WebPageProxy::continueNavigationInNewProcess):
(WebKit::WebPageProxy::forEachWebContentProcess):
(WebKit::WebPageProxy::createRemoteSubframesInOtherProcesses):
(WebKit::WebPageProxy::broadcastFrameRemovalToOtherProcesses):
(WebKit::WebPageProxy::broadcastMainFrameURLChangeToOtherProcesses):
(WebKit::WebPageProxy::webPageIDInProcessForDomain const):
(WebKit::WebPageProxy::broadcastFocusedFrameToOtherProcesses):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::createRemoteSubframe):
(WebKit::WebFrame::didCommitLoadInAnotherProcess):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didCommitLoadInAnotherProcess):
(WebKit::WebPage::didFinishLoadInAnotherProcess):
(WebKit::WebPage::frameWasRemovedInAnotherProcess):
* Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
(WTR::InjectedBundlePage::didStartProvisionalLoadForFrame):
Canonical link: https://commits.webkit.org/270643@main
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
