[webkit-changes] [WebKit/WebKit] 0c677e: Load site-isolated cross-origin iframes in their o...

EWS noreply at github.com
Tue Dec 6 14:53:20 PST 2022 

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 0c677e5142f21d9867315f8d2c5b96aa4a2536ba
      https://github.com/WebKit/WebKit/commit/0c677e5142f21d9867315f8d2c5b96aa4a2536ba
  Author: Alex Christensen <achristensen at webkit.org>
  Date:   2022-12-06 (Tue, 06 Dec 2022)

  Changed paths:
    M Source/WebCore/loader/FrameLoader.cpp
    M Source/WebCore/page/Frame.cpp
    M Source/WebCore/page/Frame.h
    M Source/WebCore/page/Page.cpp
    M Source/WebCore/page/PageConfiguration.h
    M Source/WebKit/Shared/LoadParameters.h
    M Source/WebKit/Shared/WebPageCreationParameters.cpp
    M Source/WebKit/Shared/WebPageCreationParameters.h
    M Source/WebKit/Sources.txt
    A Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp
    A Source/WebKit/UIProcess/ProvisionalFrameProxy.h
    M Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
    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/WebProcessProxy.cpp
    M Source/WebKit/UIProcess/WebProcessProxy.h
    M Source/WebKit/WebKit.xcodeproj/project.pbxproj
    M Source/WebKit/WebProcess/WebPage/WebFrame.cpp
    M Source/WebKit/WebProcess/WebPage/WebFrame.h
    M Source/WebKit/WebProcess/WebPage/WebFrame.messages.in
    M Source/WebKit/WebProcess/WebPage/WebPage.cpp
    M Source/WebKitLegacy/mac/WebView/WebFrame.mm
    M Source/WebKitLegacy/win/WebFrame.cpp
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm

  Log Message:
  -----------
  Load site-isolated cross-origin iframes in their own process
https://bugs.webkit.org/show_bug.cgi?id=248701
rdar://99665091

Reviewed by Chris Dumez.

This implements the flow needed to load cross-origin iframes in another process
when the off-by-default SiteIsolation experimental feature is turned on.

When a frame begins loading, decidePolicyForNavigationAction is sent to the UI process,
where the UI process finds or makes a new process to load the frame in.  The UI process
tells the process that loading will happen in a new process by responding with
NavigationPolicyDecision::StopAllLoads.  When this happens when navigating the main frame,
we need to stop all loads and we are done in the old process.  When this happens when
navigating an iframe, though, we need to stop the loads but we aren't done; we will be
informed later that the iframe has finished loading, and only thereafter can we consider
the parent frame to have finished loading.

Meanwhile, the UI process has taken the new process and populated it with the necessary
structures to load a frame: a WebPage and a WebFrame, which has been given the FrameIdentifier
from the UI process instead of generating a new one.  We then tell the new process's "main frame"
to load the URL of the iframe.  We use ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision
because we have already called decidePolicyForNavigationAction, so the new process does not need
to ask again.  However, when it receives the response, it does need to call decidePolicyForResponse,
but we haven't even committed the load yet.  We need an object similar to ProvisionalPageProxy
to handle messages before the load is committed, but it has slightly different responsibilities.
So we introduce ProvisionalFrameProxy to handle messages before the load is committed, at which point
the WebFrameProxy now has 2 processes: the process the web content is in, and the process the
WebCore::RemoteFrame is in, which represents an iframe in the DOM of the parent that has its content
in another process, so all it will be able to do is send messages to the other process.

As the load proceeds, it eventually calls WebFrameProxy::didFinishLoad, which needs to send
the new message WebFrame::DidFinishLoadInAnotherProcess to the remote frame's process to tell it
to tell its parent frame to fire the onload event, and finish loading the page.  Only after this
happens should the WKNavigationDelegate be informed that the loading completed.  I modified the
test to wait 0.1 seconds after sending the iframe's response header fields before sending the body
to make sure that the timing of the commit and load finished callbacks does not happen prematurely.

That is what it takes to load an iframe in another process.  There are lots of rough edges and
things to fix and polish later, but this is the minimum change to make it so that there is now
one case where it does the right thing.  Future PRs will make it so that it does the right thing
in all possible cases, but that can and should be broken up into many smaller PRs.  We will also
need to revisit the concept of "main frame", audit all uses of it, and teach the rest of the code
that the topmost Frame in this process might have a parent frame in another process.  We've already
stretched what a WebCore::Page/WebKit::WebPage is when swapping processes on cross-origin navigations,
and this stretches it a little further.  In this case, I'm using a WebPage as the container for the
topmost Frame in the current process.  There are many bugs that need fixing still, but that's ok.

* Source/WebCore/loader/FrameLoader.cpp:
(WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
Don't call checkCompleted() when we receive NavigationPolicyDecision::StopAllLoads when loading
an iframe.  This will be called later by didFinishLoadInAnotherProcess.  We aren't cancelling the
load, so it's not done yet.
* Source/WebCore/page/Frame.cpp:
(WebCore::Frame::Frame):
(WebCore::Frame::create):
Instead of calling FrameIdentifier::generate in the Frame constructor, pass it in as a parameter.
That way, we can add the main frame's identifier to WebPageCreationParameters for making a RemoteFrame's
counterpart Frame as the "main frame" in another process.
(WebCore::Frame::didFinishLoadInAnotherProcess):
* Source/WebCore/page/Frame.h:
* Source/WebCore/page/Page.cpp:
(WebCore::Page::Page):
* Source/WebCore/page/PageConfiguration.h:
* Source/WebKit/Shared/LoadParameters.h:
Prevent uninitialized memory.  I should be initializing all LoadParameters members, but at this
stage of development I'm not yet.
* Source/WebKit/Shared/WebPageCreationParameters.cpp:
(WebKit::WebPageCreationParameters::encode const):
(WebKit::WebPageCreationParameters::decode):
* Source/WebKit/Shared/WebPageCreationParameters.h:
Pass a FrameIdentifier to the main frame when making a page in a new process with the intent of having
its main frame be the remote part of a RemoteFrame in another process.
* Source/WebKit/Sources.txt:
* Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp: Added.
This class owns the process that a RemoteFrame is navigating in between when the navigation is initiated
and when the navigation is committed.  It does some set up and some message handling.
(WebKit::ProvisionalFrameProxy::ProvisionalFrameProxy):
(WebKit::ProvisionalFrameProxy::~ProvisionalFrameProxy):
(WebKit::ProvisionalFrameProxy::didReceiveMessage):
(WebKit::ProvisionalFrameProxy::decidePolicyForResponse):
(WebKit::ProvisionalFrameProxy::didCommitLoadForFrame):
(WebKit::ProvisionalFrameProxy::messageSenderConnection const):
(WebKit::ProvisionalFrameProxy::messageSenderDestinationID const):
* Source/WebKit/UIProcess/ProvisionalFrameProxy.h: Added.
(WebKit::ProvisionalFrameProxy::process):
* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
Add a check I noticed was necessary in ProvisionalFrameProxy to prevent assertions when destroying
a WebPageProxy between when a navigation is initiated and when it is committed.  Since ProvisionalPageProxy
has a similar design in this aspect, I added the connection existence check there too.
* Source/WebKit/UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::~WebFrameProxy):
(WebKit::WebFrameProxy::didFinishLoad):
(WebKit::WebFrameProxy::didFailLoad):
Add some logic to handle the times when a WebFrameProxy has two processes it represents: one with a
Frame and one with a RemoteFrame.  More logic is likely needed, but this is enough to get the 1 unit test
passing without asserting or crashing.
(WebKit::WebFrameProxy::swapToProcess):
This function is called when a cross-origin iframe navigation is initiated.  It just makes a ProvisionalFrameProxy
which handles all the exciting bits.
(WebKit::WebFrameProxy::commitProvisionalFrame):
This function is called when a cross-origin iframe navigation is committed.  It takes the process from the
ProvisionalFrameProxy and destroys the unneeded temporary steward of the process.
* Source/WebKit/UIProcess/WebFrameProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::continueNavigationInNewProcess):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::addProvisionalFrameProxy):
(WebKit::WebProcessProxy::removeProvisionalFrameProxy):
Like removeProvisionalPageProxy and addProvisionalPageProxy, we need to be informed about ProvisionalFrameProxies
because canTerminateAuxiliaryProcess needs to return false when we have provisional fames or provisional pages.
Otherwise, processes made for ProvisionalFrameProxy terminate immediately upon creation.
(WebProcessProxy::provisionalFrameCommitted):
(WebProcessProxy::removeFrameWithRemoteFrameProcess):
This is the beginning of what will probably need to become more like WebProcessProxy::addExistingWebPage
and WebProcessProxy::removeWebPage, but we need to have canTerminateAuxiliaryProcess not only return false when
there's a ProvisionalPageProxy using this process, but also if there's a frame with web content running in this
process even if it's not the main frame of a WKWebView.
(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::initWithCoreMainFrame):
(WebKit::WebFrame::createSubframe):
When the UI process gives us a FrameIdentifier to use as the main frame's identifier, don't send
WebPageProxy::DidCreateMainFrame messages, whose only purpose is to inform the UI process of the main frame's
FrameIdentifier.  Otherwise the UI process gets confused.
(WebKit::WebFrame::didCommitLoadInAnotherProcess):
(WebKit::WebFrame::didFinishLoadInAnotherProcess):
* Source/WebKit/WebProcess/WebPage/WebFrame.h:
* Source/WebKit/WebProcess/WebPage/WebFrame.messages.in:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::m_appHighlightsVisible):
* Source/WebKitLegacy/mac/WebView/WebFrame.mm:
(+[WebFrame _createFrameWithPage:frameName:frameView:ownerElement:]):
* Source/WebKitLegacy/win/WebFrame.cpp:
(WebFrame::createSubframeWithOwnerElement):
FrameIdentifier generation has been moved to callers of Frame::create to allow UI processes to assign main frames
their FrameIdentifier generated in another process.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::TEST):
Expand the test slightly to verify that didFinishNavigation is not called until the iframe is completely loaded.

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

More information about the webkit-changes mailing list