Skip to content

Commit

Permalink
Transfer remote page map to SuspendedPageProxy when suspending
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259148
rdar://112143525

Reviewed by J Pascoe.

When navigating from one page that has iframes to another, we use the remote page map
to broadcast frame tree updates.  We shouldn't broadcast to remote pages associated
with the suspended page.  The main frame is transferred, and now the remote page map
is transferred along with it.

The RemotePageDrawingAreaProxy needs to ignore a few drawing messages after it suspends.
These should be ignored because it is not longer drawing after it suspends, and the
drawing area has been destroyed.  SuspendedPageProxy also ignores messages it doesn't
recognize with no assertion.

When navigating to a SuspendedPageProxy, we need to restore the remote page map.
The remote page map follows the path of the main frame through the ProvisionalPageProxy
back to the WebPageProxy.

We were hitting assertions because the CachedFrame constructor would only remove LocalFrames.
To make it not assert, remove all Frames.  We still need to implement CachedFrame for
RemoteFrames, but this makes the test cover much of the navigation back to a SuspendedPageProxy
without assertion, with future work still to be done.

This change enables opening several popular news websites with site isolation on without asserting.

* Source/WebKit/UIProcess/RemotePageDrawingAreaProxy.cpp:
(WebKit::RemotePageDrawingAreaProxy::didReceiveMessage):
* Source/WebKit/UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
* Source/WebKit/UIProcess/SuspendedPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::takeRemotePageMap):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/266018@main
  • Loading branch information
achristensen07 committed Jul 13, 2023
1 parent 8f046fe commit ab78423
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 5 deletions.
7 changes: 5 additions & 2 deletions Source/WebCore/history/CachedFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,11 @@ CachedFrame::CachedFrame(LocalFrame& frame)
// We do this for two reasons:
// 1 - We reuse the main frame, so when it navigates to a new page load it needs to start with a blank FrameTree.
// 2 - It's much easier to destroy a CachedFrame while it resides in the BackForwardCache if it is disconnected from its parent.
for (unsigned i = 0; i < m_childFrames.size(); ++i)
frame.tree().removeChild(m_childFrames[i]->view()->frame());
Vector<Ref<Frame>> children;
for (auto* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
children.append(*child);
for (auto& child : children)
frame.tree().removeChild(child);

#ifndef NDEBUG
if (m_isMainFrame)
Expand Down
6 changes: 6 additions & 0 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ ProvisionalPageProxy::ProvisionalPageProxy(WebPageProxy& page, Ref<WebProcessPro
ASSERT(&suspendedPage->process() == m_process.ptr());
suspendedPage->unsuspend();
m_mainFrame = &suspendedPage->mainFrame();
m_remotePageMap = suspendedPage->takeRemotePageMap();
}

initializeWebPage(websitePolicies);
Expand Down Expand Up @@ -152,6 +153,11 @@ void ProvisionalPageProxy::setNavigation(API::Navigation& navigation)
navigation.setProcessID(m_process->coreProcessIdentifier());
}

std::optional<HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>>> ProvisionalPageProxy::takeRemotePageMap()
{
return std::exchange(m_remotePageMap, std::nullopt);
}

void ProvisionalPageProxy::cancel()
{
// If the provisional load started, then indicate that it failed due to cancellation by calling didFailProvisionalLoadForFrame().
Expand Down
4 changes: 4 additions & 0 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ class FormDataReference;
}

namespace WebCore {
class RegistrableDomain;
class ResourceRequest;
enum class ShouldTreatAsContinuingLoad : uint8_t;
}

namespace WebKit {

class DrawingAreaProxy;
class RemotePageProxy;
class SuspendedPageProxy;
class UserData;
class WebBackForwardListItem;
Expand Down Expand Up @@ -90,6 +92,7 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
ProcessSwapRequestedByClient processSwapRequestedByClient() const { return m_processSwapRequestedByClient; }
uint64_t navigationID() const { return m_navigationID; }
const URL& provisionalURL() const { return m_provisionalLoadURL; }
std::optional<HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>>> takeRemotePageMap();

bool isProcessSwappingOnNavigationResponse() const { return m_isProcessSwappingOnNavigationResponse; }

Expand Down Expand Up @@ -202,6 +205,7 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
LayerHostingContextID m_contextIDForVisibilityPropagationInGPUProcess { 0 };
#endif
#endif
std::optional<HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>>> m_remotePageMap;
};

} // namespace WebKit
2 changes: 0 additions & 2 deletions Source/WebKit/UIProcess/RemotePageDrawingAreaProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ void RemotePageDrawingAreaProxy::didReceiveMessage(IPC::Connection& connection,
{
if (m_drawingArea)
m_drawingArea->didReceiveMessage(connection, decoder);
else
ASSERT_NOT_REACHED();
}

bool RemotePageDrawingAreaProxy::didReceiveSyncMessage(IPC::Connection& connection, IPC::Decoder& decoder, UniqueRef<IPC::Encoder>& encoder)
Expand Down
6 changes: 6 additions & 0 deletions Source/WebKit/UIProcess/SuspendedPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&
, m_contextIDForVisibilityPropagationInGPUProcess(page.contextIDForVisibilityPropagationInGPUProcess())
#endif
#endif
, m_remotePageMap(page.takeRemotePageMap())
{
allSuspendedPages().add(this);
m_process->addSuspendedPageProxy(*this);
Expand Down Expand Up @@ -138,6 +139,11 @@ SuspendedPageProxy::~SuspendedPageProxy()
m_process->removeSuspendedPageProxy(*this);
}

HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> SuspendedPageProxy::takeRemotePageMap()
{
return std::exchange(m_remotePageMap, { });
}

void SuspendedPageProxy::didDestroyNavigation(uint64_t navigationID)
{
m_page.didDestroyNavigationShared(m_process.copyRef(), navigationID);
Expand Down
3 changes: 3 additions & 0 deletions Source/WebKit/UIProcess/SuspendedPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class RegistrableDomain;

namespace WebKit {

class RemotePageProxy;
class WebBackForwardCache;
class WebPageProxy;
class WebProcessPool;
Expand All @@ -64,6 +65,7 @@ class SuspendedPageProxy final: public IPC::MessageReceiver, public IPC::Message
WebCore::PageIdentifier webPageID() const { return m_webPageID; }
WebProcessProxy& process() const { return m_process.get(); }
WebFrameProxy& mainFrame() { return m_mainFrame.get(); }
HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> takeRemotePageMap();

WebBackForwardCache& backForwardCache() const;

Expand Down Expand Up @@ -125,6 +127,7 @@ class SuspendedPageProxy final: public IPC::MessageReceiver, public IPC::Message
LayerHostingContextID m_contextIDForVisibilityPropagationInGPUProcess { 0 };
#endif
#endif
HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> m_remotePageMap;
};

} // namespace WebKit
8 changes: 8 additions & 0 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,9 @@ void WebPageProxy::swapToProvisionalPage(std::unique_ptr<ProvisionalPageProxy> p
ASSERT(!m_mainFrame);
m_mainFrame = provisionalPage->mainFrame();

if (auto map = provisionalPage->takeRemotePageMap())
internals().domainToRemotePageProxyMap = WTFMove(*map);

m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::No);
addAllMessageReceivers();

Expand Down Expand Up @@ -12662,6 +12665,11 @@ void WebPageProxy::addOpenedRemotePageProxy(Ref<RemotePageProxy>&& page)
internals().openedRemotePageProxies.add(WTFMove(page));
}

HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> WebPageProxy::takeRemotePageMap()
{
return std::exchange(internals().domainToRemotePageProxyMap, { });
}

#if ENABLE(ADVANCED_PRIVACY_PROTECTIONS)

Vector<LinkDecorationFilteringData>& WebPageProxy::cachedAllowedQueryParametersForAdvancedPrivacyProtections()
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -2167,9 +2167,9 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
RemotePageProxy* remotePageProxyForRegistrableDomain(WebCore::RegistrableDomain) const;
void addRemotePageProxy(const WebCore::RegistrableDomain&, WeakPtr<RemotePageProxy>&&);
void removeRemotePageProxy(const WebCore::RegistrableDomain&);

void setRemotePageProxyInOpenerProcess(Ref<RemotePageProxy>&&);
void addOpenedRemotePageProxy(Ref<RemotePageProxy>&&);
HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> takeRemotePageMap();

void createRemoteSubframesInOtherProcesses(WebFrameProxy&);

Expand Down
28 changes: 28 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1096,4 +1096,32 @@ HTTPServer server({
// FIXME: Call checkFrameTreesInProcesses here and make sure the frames are in reasonable processes.
}

TEST(SiteIsolation, NavigationWithIFrames)
{
HTTPServer server({
{ "/1"_s, { "<iframe src='https://domain2.com/2'></iframe>"_s } },
{ "/2"_s, { "hi!"_s } },
{ "/3"_s, { "<iframe src='https://domain4.com/4'></iframe>"_s } },
{ "/4"_s, { "<iframe src='https://domain5.com/5'></iframe>"_s } },
{ "/5"_s, { "hi!"_s } }
}, HTTPServer::Protocol::HttpsProxy);

auto navigationDelegate = adoptNS([TestNavigationDelegate new]);
[navigationDelegate allowAnyTLSCertificate];
auto configuration = server.httpsProxyConfiguration();
enableSiteIsolation(configuration);
auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectZero configuration:configuration]);
webView.get().navigationDelegate = navigationDelegate.get();

[webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://domain1.com/1"]]];
[navigationDelegate waitForDidFinishNavigation];

[webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://domain3.com/3"]]];
[navigationDelegate waitForDidFinishNavigation];

[webView goBack];
[navigationDelegate waitForDidFinishNavigation];
// FIXME: Implement CachedFrame for RemoteFrames and verify the page is resumed correctly.
}

}

0 comments on commit ab78423

Please sign in to comment.