Diff
Modified: trunk/Source/WebKit/ChangeLog (236479 => 236480)
--- trunk/Source/WebKit/ChangeLog 2018-09-25 22:09:45 UTC (rev 236479)
+++ trunk/Source/WebKit/ChangeLog 2018-09-25 22:17:51 UTC (rev 236480)
@@ -1,3 +1,30 @@
+2018-09-25 Chris Dumez <cdu...@apple.com>
+
+ Revert some of the changes in r236471
+ https://bugs.webkit.org/show_bug.cgi?id=189973
+
+ Reviewed by Alex Christensen.
+
+ Revert some of the changes in r236471 as they should not be needed. In particular,
+ it should not be possible for the DecidePolicyForNavigationActionSync IPC to get
+ processed *before* the DidCreateMainFrame / DidCreateSubframe ones because those
+ use IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply. They are thus
+ processed early when necessary, the same way as synchronous IPC messages.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didCreateMainFrame):
+ (WebKit::WebPageProxy::didCreateSubframe):
+ (WebKit::WebPageProxy::decidePolicyForNavigationActionAsync):
+ (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+ (WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/WebPageProxy.messages.in:
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+ * WebProcess/WebPage/WebFrame.cpp:
+ (WebKit::WebFrame::createWithCoreMainFrame):
+ (WebKit::WebFrame::createSubframe):
+
2018-09-25 Sihui Liu <sihui_...@apple.com>
Move Service Worker Management from Storage Process to Network Process
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (236479 => 236480)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-09-25 22:09:45 UTC (rev 236479)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-09-25 22:17:51 UTC (rev 236480)
@@ -3357,9 +3357,6 @@
void WebPageProxy::didCreateMainFrame(uint64_t frameID)
{
- if (m_mainFrame && m_mainFrame->frameID() == frameID)
- return;
-
PageClientProtector protector(pageClient());
MESSAGE_CHECK(!m_mainFrame);
@@ -3382,10 +3379,6 @@
PageClientProtector protector(pageClient());
MESSAGE_CHECK(m_mainFrame);
-
- if (m_process->webFrame(frameID))
- return;
-
MESSAGE_CHECK(m_process->canCreateFrame(frameID));
RefPtr<WebFrameProxy> subFrame = WebFrameProxy::create(this, frameID);
@@ -4003,15 +3996,12 @@
void WebPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
{
- auto* frame = m_process->webFrame(frameID);
- MESSAGE_CHECK(frame);
-
- decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, PolicyDecisionSender::create([this, protectedThis = makeRef(*this), frameID, listenerID] (auto... args) {
+ decidePolicyForNavigationAction(frameID, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, PolicyDecisionSender::create([this, protectedThis = makeRef(*this), frameID, listenerID] (auto... args) {
m_process->send(Messages::WebPage::DidReceivePolicyDecision(frameID, listenerID, args...), m_pageID);
}));
}
-void WebPageProxy::decidePolicyForNavigationAction(WebFrameProxy& frame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&& sender)
+void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&& sender)
{
LOG(Loading, "WebPageProxy::decidePolicyForNavigationAction - Original URL %s, current target URL %s", originalRequest.url().string().utf8().data(), request.url().string().utf8().data());
@@ -4023,6 +4013,8 @@
if (!fromAPI)
m_pageLoadState.clearPendingAPIRequestURL(transaction);
+ auto* frame = m_process->webFrame(frameID);
+ MESSAGE_CHECK(frame);
MESSAGE_CHECK_URL(request.url());
MESSAGE_CHECK_URL(originalRequest.url());
@@ -4059,13 +4051,13 @@
navigation->setRequesterOrigin(navigationActionData.requesterOrigin);
#if ENABLE(CONTENT_FILTERING)
- if (frame.didHandleContentFilterUnblockNavigation(request))
+ if (frame->didHandleContentFilterUnblockNavigation(request))
return receivedPolicyDecision(PolicyAction::Ignore, m_navigationState->navigation(newNavigationID), std::nullopt, WTFMove(sender));
#else
UNUSED_PARAM(newNavigationID);
#endif
- auto listener = makeRef(frame.setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(frame), sender = WTFMove(sender), navigation] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&) mutable {
+ auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(*frame), sender = WTFMove(sender), navigation] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&) mutable {
// FIXME: do something with the SafeBrowsingResults.
receivedNavigationPolicyDecision(policyAction, navigation.get(), processSwapRequestedByClient, frame, policies, WTFMove(sender));
}, shouldSkipSafeBrowsingCheck == ShouldSkipSafeBrowsingCheck::Yes ? ShouldExpectSafeBrowsingResult::No : ShouldExpectSafeBrowsingResult::Yes));
@@ -4072,18 +4064,18 @@
if (shouldSkipSafeBrowsingCheck == ShouldSkipSafeBrowsingCheck::No)
beginSafeBrowsingCheck(request.url(), listener);
- API::Navigation* mainFrameNavigation = frame.isMainFrame() ? navigation.get() : nullptr;
+ API::Navigation* mainFrameNavigation = frame->isMainFrame() ? navigation.get() : nullptr;
WebFrameProxy* originatingFrame = m_process->webFrame(originatingFrameInfoData.frameID);
if (auto* resourceLoadStatisticsStore = websiteDataStore().resourceLoadStatistics())
- resourceLoadStatisticsStore->logFrameNavigation(frame, URL(URL(), m_pageLoadState.url()), request, redirectResponse.url());
+ resourceLoadStatisticsStore->logFrameNavigation(*frame, URL(URL(), m_pageLoadState.url()), request, redirectResponse.url());
if (m_policyClient)
- m_policyClient->decidePolicyForNavigationAction(*this, &frame, WTFMove(navigationActionData), originatingFrame, originalRequest, WTFMove(request), WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
+ m_policyClient->decidePolicyForNavigationAction(*this, frame, WTFMove(navigationActionData), originatingFrame, originalRequest, WTFMove(request), WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
else {
- auto destinationFrameInfo = API::FrameInfo::create(frame, frameSecurityOrigin.securityOrigin());
+ auto destinationFrameInfo = API::FrameInfo::create(*frame, frameSecurityOrigin.securityOrigin());
RefPtr<API::FrameInfo> sourceFrameInfo;
- if (!fromAPI && originatingFrame == &frame)
+ if (!fromAPI && originatingFrame == frame)
sourceFrameInfo = destinationFrameInfo.copyRef();
else if (!fromAPI)
sourceFrameInfo = API::FrameInfo::create(originatingFrameInfoData, originatingPageID ? m_process->webPage(originatingPageID) : nullptr);
@@ -4099,23 +4091,11 @@
m_shouldSuppressAppLinksInNextNavigationPolicyDecision = false;
}
-void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
+void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
{
auto sender = PolicyDecisionSender::create(WTFMove(reply));
-
- auto* frame = m_process->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)
- didCreateMainFrame(frameID);
- else
- didCreateSubframe(frameID);
-
- frame = m_process->webFrame(frameID);
- RELEASE_ASSERT(frame);
- }
- decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, sender.copyRef());
+ decidePolicyForNavigationAction(frameID, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, sender.copyRef());
// If the client did not respond synchronously, proceed with the load.
sender->send(PolicyAction::Use, navigationID, DownloadID(), std::nullopt);
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (236479 => 236480)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-09-25 22:09:45 UTC (rev 236479)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-09-25 22:17:51 UTC (rev 236480)
@@ -1434,9 +1434,9 @@
void didDestroyNavigation(uint64_t navigationID);
- void decidePolicyForNavigationAction(WebFrameProxy&, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&&);
+ void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&&);
void decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, uint64_t listenerID);
- void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
+ void decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
void decidePolicyForNewWindowAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, NavigationActionData&&, WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&);
void unableToImplementPolicy(uint64_t frameID, const WebCore::ResourceError&, const UserData&);
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in (236479 => 236480)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in 2018-09-25 22:09:45 UTC (rev 236479)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in 2018-09-25 22:17:51 UTC (rev 236480)
@@ -108,7 +108,7 @@
# Policy messages
DecidePolicyForResponse(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, uint64_t listenerID, WebKit::UserData userData)
DecidePolicyForNavigationActionAsync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
- DecidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck) -> (enum WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
+ DecidePolicyForNavigationActionSync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck) -> (enum WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
DecidePolicyForNewWindowAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, struct WebKit::NavigationActionData navigationActionData, WebCore::ResourceRequest request, String frameName, uint64_t listenerID, WebKit::UserData userData)
UnableToImplementPolicy(uint64_t frameID, WebCore::ResourceError error, WebKit::UserData userData)
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (236479 => 236480)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2018-09-25 22:09:45 UTC (rev 236479)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2018-09-25 22:17:51 UTC (rev 236480)
@@ -894,7 +894,7 @@
DownloadID downloadID;
std::optional<WebsitePoliciesData> websitePolicies;
- if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), m_frame->isMainFrame(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(policyAction, newNavigationID, downloadID, websitePolicies))) {
+ if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(policyAction, newNavigationID, downloadID, websitePolicies))) {
m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
return;
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp (236479 => 236480)
--- trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp 2018-09-25 22:09:45 UTC (rev 236479)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp 2018-09-25 22:17:51 UTC (rev 236480)
@@ -111,6 +111,8 @@
Ref<WebFrame> WebFrame::createWithCoreMainFrame(WebPage* page, WebCore::Frame* coreFrame)
{
auto frame = create(std::unique_ptr<WebFrameLoaderClient>(static_cast<WebFrameLoaderClient*>(&coreFrame->loader().client())));
+ // DispatchMessageEvenWhenWaitingForSyncReply SendOption is needed to ensure that this IPC always gets received before the DecidePolicyForNavigationSync synchronous
+ // IPC for this frame.
page->send(Messages::WebPageProxy::DidCreateMainFrame(frame->frameID()), page->pageID(), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
frame->m_coreFrame = coreFrame;
@@ -122,6 +124,8 @@
Ref<WebFrame> WebFrame::createSubframe(WebPage* page, const String& frameName, HTMLFrameOwnerElement* ownerElement)
{
auto frame = create(std::make_unique<WebFrameLoaderClient>());
+ // DispatchMessageEvenWhenWaitingForSyncReply SendOption is needed to ensure that this IPC always gets received before the DecidePolicyForNavigationSync synchronous
+ // IPC for this frame.
page->send(Messages::WebPageProxy::DidCreateSubframe(frame->frameID()), page->pageID(), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
auto coreFrame = Frame::create(page->corePage(), ownerElement, frame->m_frameLoaderClient.get());
