Skip to content

Commit

Permalink
Unreviewed, reverting r256138@main.
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=247264

Causes instacrash when loading some websites

Reverted changeset:

"Move more policy decision messages to WebFrameProxy"
https://bugs.webkit.org/show_bug.cgi?id=247201
https://commits.webkit.org/256138@main

Canonical link: https://commits.webkit.org/256153@main
  • Loading branch information
@webkit-commit-queue @bnham
webkit-commit-queue authored and bnham committed Oct 31, 2022
1 parent 22e4c03 commit 25a3ea9
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 58 deletions.
13 changes: 13 additions & 0 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
View file
Expand Up @@ -331,6 +331,14 @@ void ProvisionalPageProxy::didChangeProvisionalURLForFrame(FrameIdentifier frame
m_page.didChangeProvisionalURLForFrameShared(m_process.copyRef(), frameID, navigationID, WTFMove(url));
}

void ProvisionalPageProxy::decidePolicyForResponse(FrameIdentifier frameID, FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData& userData)
{
if (!validateInput(frameID, navigationID))
return;

m_page.decidePolicyForResponseShared(m_process.copyRef(), m_webPageID, frameID, WTFMove(frameInfo), identifier, navigationID, response, request, canShowMIMEType, downloadAttribute, listenerID, userData);
}

void ProvisionalPageProxy::didPerformServerRedirect(const String& sourceURLString, const String& destinationURLString, FrameIdentifier frameID)
{
if (!validateInput(frameID))
Expand Down Expand Up @@ -485,6 +493,11 @@ void ProvisionalPageProxy::didReceiveMessage(IPC::Connection& connection, IPC::D
return;
}

if (decoder.messageName() == Messages::WebPageProxy::DecidePolicyForResponse::name()) {
IPC::handleMessage<Messages::WebPageProxy::DecidePolicyForResponse>(connection, decoder, this, &ProvisionalPageProxy::decidePolicyForResponse);
return;
}

if (decoder.messageName() == Messages::WebPageProxy::DidChangeProvisionalURLForFrame::name()) {
IPC::handleMessage<Messages::WebPageProxy::DidChangeProvisionalURLForFrame>(connection, decoder, this, &ProvisionalPageProxy::didChangeProvisionalURLForFrame);
return;
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.h
View file
Expand Up @@ -126,6 +126,7 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
bool sendMessage(UniqueRef<IPC::Encoder>&&, OptionSet<IPC::SendOption>) final;
bool sendMessageWithAsyncReply(UniqueRef<IPC::Encoder>&&, AsyncReplyHandler, OptionSet<IPC::SendOption>) final;

void decidePolicyForResponse(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData&);
void didChangeProvisionalURLForFrame(WebCore::FrameIdentifier, uint64_t navigationID, URL&&);
void didPerformServerRedirect(const String& sourceURLString, const String& destinationURLString, WebCore::FrameIdentifier);
void didReceiveServerRedirectForProvisionalLoadForFrame(WebCore::FrameIdentifier, uint64_t navigationID, WebCore::ResourceRequest&&, const UserData&);
Expand Down
56 changes: 24 additions & 32 deletions Source/WebKit/UIProcess/WebFrameProxy.cpp
View file
Expand Up @@ -349,10 +349,16 @@ void WebFrameProxy::disconnect()

void WebFrameProxy::didCreateSubframe(WebCore::FrameIdentifier frameID)
{
// The DecidePolicyForNavigationActionSync IPC is synchronous and may therefore get processed before the DidCreateSubframe one.
// When this happens, decidePolicyForNavigationActionSync() calls didCreateSubframe() and we need to ignore the DidCreateSubframe
// IPC when it later gets processed.
if (WebFrameProxy::webFrame(frameID))
return;

MESSAGE_CHECK(m_process, m_page);
MESSAGE_CHECK(m_process, WebFrameProxy::canCreateFrame(frameID));
MESSAGE_CHECK(m_process, frameID.processIdentifier() == m_process->coreProcessIdentifier());
if (!m_page)
return;

auto child = WebFrameProxy::create(*m_page, m_process, m_webPageID, frameID);
child->m_parentFrame = *this;
m_childFrames.add(WTFMove(child));
Expand All @@ -361,12 +367,7 @@ void WebFrameProxy::didCreateSubframe(WebCore::FrameIdentifier frameID)
void WebFrameProxy::swapToProcess(WebProcessProxy& process)
{
ASSERT(!isMainFrame());
m_process->removeMessageReceiver(Messages::WebFrameProxy::messageReceiverName(), m_frameID.object());
m_process = process;

// FIXME: This identifier may collide with identifiers generated in the new process.
m_process->addMessageReceiver(Messages::WebFrameProxy::messageReceiverName(), m_frameID.object(), *this);

// FIXME: Do more here.
}

Expand All @@ -380,46 +381,37 @@ uint64_t WebFrameProxy::messageSenderDestinationID() const
return m_frameID.object().toUInt64();
}

void WebFrameProxy::decidePolicyForNavigationActionAsync(FrameInfoData&& frameInfo, PolicyCheckIdentifier identifier, uint64_t navigationID,
void WebFrameProxy::decidePolicyForNavigationActionAsync(FrameIdentifier frameID, FrameInfoData&& frameInfo, PolicyCheckIdentifier identifier, uint64_t navigationID,
NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request,
IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID)
{
if (!m_page)
return;
m_page->decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), m_webPageID, frameID(), WTFMove(frameInfo), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID);
m_page->decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), m_webPageID, frameID, WTFMove(frameInfo), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID);
}

void WebFrameProxy::decidePolicyForNavigationActionSync(bool isMainFrame, FrameInfoData&& frameInfo, PolicyCheckIdentifier identifier,
void WebFrameProxy::decidePolicyForNavigationActionSync(FrameIdentifier frameID, bool isMainFrame, FrameInfoData&& frameInfo, PolicyCheckIdentifier identifier,
uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID,
const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse,
const UserData& userData, Messages::WebFrameProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
{
if (!m_page)
return reply({ });
m_page->decidePolicyForNavigationActionSyncShared(m_process.copyRef(), m_webPageID, frameID(), isMainFrame, WTFMove(frameInfo), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(reply));
}

void WebFrameProxy::decidePolicyForNewWindowAction(FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, NavigationActionData&& navigationActionData,
WebCore::ResourceRequest&& request, const String& frameName, uint64_t listenerID, const UserData& userData)
{
if (!m_page)
return;
m_page->decidePolicyForNewWindowAction(frameID(), WTFMove(frameInfo), policyCheckIdentifier, WTFMove(navigationActionData), WTFMove(request), frameName, listenerID, userData);
}

void WebFrameProxy::decidePolicyForResponse(FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID,
const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData& userData)
{
if (!m_page)
return;
m_page->decidePolicyForResponseShared(m_process.copyRef(), m_webPageID, frameID(), WTFMove(frameInfo), policyCheckIdentifier, navigationID, response, request, canShowMIMEType, downloadAttribute, listenerID, userData);
}
RefPtr frame = WebFrameProxy::webFrame(frameID);
if (!frame) {
// This synchronous IPC message was processed before the asynchronous DidCreateMainFrame / DidCreateSubframe one so we do not know about this frameID yet.
if (isMainFrame)
m_page->didCreateMainFrame(frameID);
else {
MESSAGE_CHECK(m_process, frameInfo.parentFrameID);
RefPtr parentFrame = WebFrameProxy::webFrame(*frameInfo.parentFrameID);
MESSAGE_CHECK(m_process, parentFrame);
parentFrame->didCreateSubframe(frameID);
}
}

void WebFrameProxy::unableToImplementPolicy(const WebCore::ResourceError& error, const UserData& userData)
{
if (!m_page)
return;
m_page->unableToImplementPolicy(frameID(), error, userData);
m_page->decidePolicyForNavigationActionSyncShared(m_process.copyRef(), m_webPageID, frameID, isMainFrame, WTFMove(frameInfo), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(reply));
}

} // namespace WebKit
15 changes: 5 additions & 10 deletions Source/WebKit/UIProcess/WebFrameProxy.h
View file
Expand Up @@ -145,20 +145,15 @@ class WebFrameProxy : public API::ObjectImpl<API::Object::Type::Frame>, public I
void didReceiveMessage(IPC::Connection&, IPC::Decoder&);
bool didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, UniqueRef<IPC::Encoder>&);

private:
WebFrameProxy(WebPageProxy&, WebProcessProxy&, WebCore::PageIdentifier, WebCore::FrameIdentifier);

void decidePolicyForNavigationActionAsync(FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo,
void decidePolicyForNavigationActionAsync(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo,
std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody,
WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID);
void decidePolicyForNavigationActionSync(bool isMainFrame, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo,
void decidePolicyForNavigationActionSync(WebCore::FrameIdentifier, bool isMainFrame, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo,
std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody,
WebCore::ResourceResponse&& redirectResponse, const UserData&, CompletionHandler<void(const PolicyDecision&)>&&);
void decidePolicyForNewWindowAction(FrameInfoData&&, WebCore::PolicyCheckIdentifier, NavigationActionData&&,
WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
void decidePolicyForResponse(FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID,
const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData&);
void unableToImplementPolicy(const WebCore::ResourceError&, const UserData&);

private:
WebFrameProxy(WebPageProxy&, WebProcessProxy&, WebCore::PageIdentifier, WebCore::FrameIdentifier);

std::optional<WebCore::PageIdentifier> pageIdentifier() const;

Expand Down
8 changes: 3 additions & 5 deletions Source/WebKit/UIProcess/WebFrameProxy.messages.in
View file
Expand Up @@ -23,9 +23,7 @@
messages -> WebFrameProxy {
DidCreateSubframe(WebCore::FrameIdentifier frameID)

DecidePolicyForNavigationActionAsync(struct WebKit::FrameInfoData frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, std::optional<WebKit::WebPageProxyIdentifier> originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, uint64_t listenerID)
DecidePolicyForNavigationActionSync(bool isMainFrame, struct WebKit::FrameInfoData frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, std::optional<WebKit::WebPageProxyIdentifier> originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData) -> (struct WebKit::PolicyDecision PolicyDecision) Synchronous
DecidePolicyForResponse(struct WebKit::FrameInfoData frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, String downloadAttribute, uint64_t listenerID, WebKit::UserData userData)
DecidePolicyForNewWindowAction(struct WebKit::FrameInfoData frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, struct WebKit::NavigationActionData navigationActionData, WebCore::ResourceRequest request, String frameName, uint64_t listenerID, WebKit::UserData userData)
UnableToImplementPolicy(WebCore::ResourceError error, WebKit::UserData userData)
DecidePolicyForNavigationActionAsync(WebCore::FrameIdentifier frameID, struct WebKit::FrameInfoData frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, std::optional<WebKit::WebPageProxyIdentifier> originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, uint64_t listenerID)
DecidePolicyForNavigationActionSync(WebCore::FrameIdentifier frameID, bool isMainFrame, struct WebKit::FrameInfoData frameInfo, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, std::optional<WebKit::WebPageProxyIdentifier> originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData) -> (struct WebKit::PolicyDecision PolicyDecision) Synchronous

}
6 changes: 6 additions & 0 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
View file
Expand Up @@ -5934,6 +5934,12 @@ void WebPageProxy::decidePolicyForNewWindowAction(FrameIdentifier frameID, Frame
m_navigationClient->decidePolicyForNavigationAction(*this, navigationAction.get(), WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
}

void WebPageProxy::decidePolicyForResponse(FrameIdentifier frameID, FrameInfoData&& frameInfo, PolicyCheckIdentifier identifier,
uint64_t navigationID, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData& userData)
{
decidePolicyForResponseShared(m_process.copyRef(), m_webPageID, frameID, WTFMove(frameInfo), identifier, navigationID, response, request, canShowMIMEType, downloadAttribute, listenerID, userData);
}

void WebPageProxy::decidePolicyForResponseShared(Ref<WebProcessProxy>&& process, PageIdentifier webPageID, FrameIdentifier frameID, FrameInfoData&& frameInfo, PolicyCheckIdentifier identifier, uint64_t navigationID, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData& userData)
{
PageClientProtector protector(pageClient());
Expand Down
8 changes: 5 additions & 3 deletions Source/WebKit/UIProcess/WebPageProxy.h
View file
Expand Up @@ -1847,9 +1847,6 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>
void didChangeProvisionalURLForFrameShared(Ref<WebProcessProxy>&&, WebCore::FrameIdentifier, uint64_t navigationID, URL&&);
void decidePolicyForNavigationActionAsyncShared(Ref<WebProcessProxy>&&, WebCore::PageIdentifier, WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID);
void decidePolicyForResponseShared(Ref<WebProcessProxy>&&, WebCore::PageIdentifier, WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData&);
void decidePolicyForNewWindowAction(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, NavigationActionData&&,
WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
void unableToImplementPolicy(WebCore::FrameIdentifier, const WebCore::ResourceError&, const UserData&);
void startURLSchemeTaskShared(Ref<WebProcessProxy>&&, WebCore::PageIdentifier, URLSchemeTaskParameters&&);
void loadDataWithNavigationShared(Ref<WebProcessProxy>&&, WebCore::PageIdentifier, API::Navigation&, const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, std::optional<NavigatingToAppBoundDomain>, std::optional<WebsitePoliciesData>&&, WebCore::ShouldOpenExternalURLsPolicy, WebCore::SubstituteData::SessionHistoryVisibility);
void loadRequestWithNavigationShared(Ref<WebProcessProxy>&&, WebCore::PageIdentifier, API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, std::optional<NavigatingToAppBoundDomain>, std::optional<WebsitePoliciesData>&& = std::nullopt, std::optional<NetworkResourceLoadIdentifier> existingNetworkResourceLoadIdentifierToResume = std::nullopt);
Expand Down Expand Up @@ -2245,6 +2242,11 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>
void decidePolicyForNavigationAction(Ref<WebProcessProxy>&&, WebCore::PageIdentifier, WebFrameProxy&, FrameInfoData&&, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo,
std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody,
WebCore::ResourceResponse&& redirectResponse, const UserData&, Ref<PolicyDecisionSender>&&);
void decidePolicyForNewWindowAction(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, NavigationActionData&&,
WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
void decidePolicyForResponse(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID,
const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID, const UserData&);
void unableToImplementPolicy(WebCore::FrameIdentifier, const WebCore::ResourceError&, const UserData&);
void beginSafeBrowsingCheck(const URL&, bool, WebFramePolicyListenerProxy&);

WebContentMode effectiveContentModeAfterAdjustingPolicies(API::WebsitePolicies&, const WebCore::ResourceRequest&);
Expand Down

0 comments on commit 25a3ea9

Please sign in to comment.