[webkit-changes] [WebKit/WebKit] 8dcff5: http/tests/site-isolation/frame-access-after-windo...

[Alex Christensen]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 8dcff579b57ecdd441f9e2b54a54299a57c048ed
      https://github.com/WebKit/WebKit/commit/8dcff579b57ecdd441f9e2b54a54299a57c048ed
  Author: Alex Christensen <achristensen at apple.com>
  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