-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
1 parent
9c56ef4
commit 8dcff57
Showing
24 changed files
with
90 additions
and
50 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 changes: 4 additions & 2 deletions
6
LayoutTests/http/tests/site-isolation/resources/opened-iframe-2.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
<script> | ||
window.customProperty = 43; | ||
window.parent.opener.secondSameOriginIframeOpened() | ||
onload = ()=>{ | ||
window.customProperty = 43; | ||
window.parent.opener.secondSameOriginIframeOpened() | ||
} | ||
</script> |
6 changes: 4 additions & 2 deletions
6
LayoutTests/http/tests/site-isolation/resources/opened-iframe.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
<script> | ||
window.customProperty = 42; | ||
window.parent.opener.firstSameOriginIframeOpened() | ||
onload = ()=>{ | ||
window.customProperty = 42; | ||
window.parent.opener.firstSameOriginIframeOpened() | ||
} | ||
</script> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.