WebKit Bugzilla
Attachment 360993 Details for
Bug 194189
: Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for landing
bug-194189-20190202200655.patch (text/plain), 102.86 KB, created by
Ryosuke Niwa
on 2019-02-02 20:06:56 PST
(
hide
)
Description:
Patch for landing
Filename:
MIME Type:
Creator:
Ryosuke Niwa
Created:
2019-02-02 20:06:56 PST
Size:
102.86 KB
patch
obsolete
>Index: Source/WebCore/ChangeLog >=================================================================== >--- Source/WebCore/ChangeLog (revision 240901) >+++ Source/WebCore/ChangeLog (working copy) >@@ -1,3 +1,56 @@ >+2019-02-02 Ryosuke Niwa <rniwa@webkit.org> >+ >+ Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy >+ https://bugs.webkit.org/show_bug.cgi?id=194189 >+ >+ Reviewed by Geoffrey Garen. >+ >+ Introduced PolicyCheckIdentifier to pair each navigation policy check request with a decision, >+ and deployed it in PolicyChecker. The identifier is passed from WebContent process to UI process >+ in WebKit2, and passed it back with the policy decision. >+ >+ Because PolicyCheckIdentifier embeds the process identifier from which a navigation policy is checked, >+ we would be able to detect when UI process had sent the decision to a wrong WebContent process. >+ >+ This patch also adds release assertions to make sure history().provisionalItem() is set whenever >+ we're requesting a navigation policy check. >+ >+ These code changes should either: >+ 1. Fix crashes in FrameLoader::continueLoadAfterNavigationPolicy where isBackForwardLoadType would >+ return true yet history().provisionalItem() is null. >+ 2. Detect a bug that UI process can send a navigation policy decision to a wrong WebContent process. >+ 3. Rule out the possibility that (2) exists. >+ >+ * loader/DocumentLoader.cpp: >+ (WebCore::DocumentLoader::willSendRequest): >+ (WebCore::DocumentLoader::responseReceived): >+ * loader/EmptyClients.cpp: >+ (WebCore::EmptyFrameLoaderClient::dispatchDecidePolicyForNewWindowAction): >+ (WebCore::EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction): >+ * loader/EmptyFrameLoaderClient.h: >+ * loader/FrameLoader.cpp: >+ (WebCore::FrameLoader::checkContentPolicy): >+ (WebCore::FrameLoader::loadURL): >+ (WebCore::FrameLoader::load): >+ (WebCore::FrameLoader::loadWithDocumentLoader): >+ (WebCore::FrameLoader::loadPostRequest): >+ * loader/FrameLoader.h: >+ * loader/FrameLoaderClient.h: >+ * loader/FrameLoaderTypes.h: >+ (WebCore::PolicyCheckIdentifier): Added. >+ (WebCore::PolicyCheckIdentifier::operator== const): Added. >+ (WebCore::PolicyCheckIdentifier::PolicyCheckIdentifier): Added. >+ (WebCore::PolicyCheckIdentifier::encode const): Added. >+ (WebCore::PolicyCheckIdentifier::decode): Added. >+ * loader/PolicyChecker.cpp: >+ (WebCore::PolicyCheckIdentifier::generate): >+ (WebCore::PolicyCheckIdentifier::isValidFor): Returns true if the identifer matches. Also release asserts >+ that the process ID is same, and that m_check is always not zero (meaning it's a generated value). >+ The failure of these release assertions would indicate that there is a bug in UI process, which results in >+ a policy decision response being sent to a wrong Web process. >+ (WebCore::PolicyChecker::checkNavigationPolicy): Exit early if isValidFor fails. >+ (WebCore::PolicyChecker::checkNewWindowPolicy): >+ > 2019-02-02 Simon Fraser <simon.fraser@apple.com> > > Tidy up data memebers of FrameView and related classes to shrink class sizes >Index: Source/WebCore/loader/DocumentLoader.cpp >=================================================================== >--- Source/WebCore/loader/DocumentLoader.cpp (revision 240901) >+++ Source/WebCore/loader/DocumentLoader.cpp (working copy) >@@ -56,6 +56,7 @@ > #include "HTTPHeaderField.h" > #include "HTTPHeaderNames.h" > #include "HistoryItem.h" >+#include "HistoryController.h" > #include "IconLoader.h" > #include "InspectorInstrumentation.h" > #include "LinkIconCollector.h" >@@ -662,7 +663,10 @@ void DocumentLoader::willSendRequest(Res > ASSERT(!m_waitingForNavigationPolicy); > m_waitingForNavigationPolicy = true; > >- frameLoader()->policyChecker().checkNavigationPolicy(WTFMove(newRequest), redirectResponse, WTFMove(navigationPolicyCompletionHandler)); >+ // FIXME: Add a load type check. >+ auto& policyChecker = frameLoader()->policyChecker(); >+ RELEASE_ASSERT(!isBackForwardLoadType(policyChecker.loadType()) || frameLoader()->history().provisionalItem()); >+ policyChecker.checkNavigationPolicy(WTFMove(newRequest), redirectResponse, WTFMove(navigationPolicyCompletionHandler)); > } > > bool DocumentLoader::tryLoadingRequestFromApplicationCache() >@@ -839,7 +843,10 @@ void DocumentLoader::responseReceived(co > RefPtr<SubresourceLoader> mainResourceLoader = this->mainResourceLoader(); > if (mainResourceLoader) > mainResourceLoader->markInAsyncResponsePolicyCheck(); >- frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader), completionHandler = completionHandlerCaller.release()] (PolicyAction policy) mutable { >+ auto requestIdentifier = PolicyCheckIdentifier::create(); >+ frameLoader()->checkContentPolicy(m_response, requestIdentifier, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader), >+ completionHandler = completionHandlerCaller.release(), requestIdentifier] (PolicyAction policy, PolicyCheckIdentifier responseIdentifeir) mutable { >+ RELEASE_ASSERT(responseIdentifeir.isValidFor(requestIdentifier)); > continueAfterContentPolicy(policy); > if (mainResourceLoader) > mainResourceLoader->didReceiveResponsePolicy(); >Index: Source/WebCore/loader/EmptyClients.cpp >=================================================================== >--- Source/WebCore/loader/EmptyClients.cpp (revision 240901) >+++ Source/WebCore/loader/EmptyClients.cpp (working copy) >@@ -452,11 +452,11 @@ PAL::SessionID EmptyFrameLoaderClient::s > return PAL::SessionID::defaultSessionID(); > } > >-void EmptyFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String&, FramePolicyFunction&&) >+void EmptyFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String&, PolicyCheckIdentifier, FramePolicyFunction&&) > { > } > >-void EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse&, FormState*, PolicyDecisionMode, FramePolicyFunction&&) >+void EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse&, FormState*, PolicyDecisionMode, PolicyCheckIdentifier, FramePolicyFunction&&) > { > } > >Index: Source/WebCore/loader/EmptyFrameLoaderClient.h >=================================================================== >--- Source/WebCore/loader/EmptyFrameLoaderClient.h (revision 240901) >+++ Source/WebCore/loader/EmptyFrameLoaderClient.h (working copy) >@@ -93,9 +93,9 @@ class WEBCORE_EXPORT EmptyFrameLoaderCli > Frame* dispatchCreatePage(const NavigationAction&) final { return nullptr; } > void dispatchShow() final { } > >- void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, FramePolicyFunction&&) final { } >- void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String&, FramePolicyFunction&&) final; >- void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse& redirectResponse, FormState*, PolicyDecisionMode, FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, PolicyCheckIdentifier, FramePolicyFunction&&) final { } >+ void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String&, PolicyCheckIdentifier, FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse& redirectResponse, FormState*, PolicyDecisionMode, PolicyCheckIdentifier, FramePolicyFunction&&) final; > void cancelPolicyCheck() final { } > > void dispatchUnableToImplementPolicy(const ResourceError&) final { } >Index: Source/WebCore/loader/FrameLoader.cpp >=================================================================== >--- Source/WebCore/loader/FrameLoader.cpp (revision 240901) >+++ Source/WebCore/loader/FrameLoader.cpp (working copy) >@@ -363,15 +363,16 @@ void FrameLoader::setDefersLoading(bool > } > } > >-void FrameLoader::checkContentPolicy(const ResourceResponse& response, ContentPolicyDecisionFunction&& function) >+void FrameLoader::checkContentPolicy(const ResourceResponse& response, PolicyCheckIdentifier identifier, ContentPolicyDecisionFunction&& function) > { > if (!activeDocumentLoader()) { > // Load was cancelled >- function(PolicyAction::Ignore); >+ function(PolicyAction::Ignore, identifier); > return; > } > >- client().dispatchDecidePolicyForResponse(response, activeDocumentLoader()->request(), WTFMove(function)); >+ // FIXME: Validate the policy check identifier. >+ client().dispatchDecidePolicyForResponse(response, activeDocumentLoader()->request(), identifier, WTFMove(function)); > } > > void FrameLoader::changeLocation(FrameLoadRequest&& request) >@@ -1378,6 +1379,7 @@ void FrameLoader::loadURL(FrameLoadReque > > if (!targetFrame && !effectiveFrameName.isEmpty()) { > action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest)); >+ RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType())); > policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), WTFMove(formState), effectiveFrameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) mutable { > continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy); > completionHandler(); >@@ -1398,6 +1400,7 @@ void FrameLoader::loadURL(FrameLoadReque > oldDocumentLoader->setLastCheckedRequest(ResourceRequest()); > policyChecker().stopCheck(); > policyChecker().setLoadType(newLoadType); >+ RELEASE_ASSERT(!isBackForwardLoadType(newLoadType) || history().provisionalItem()); > policyChecker().checkNavigationPolicy(WTFMove(request), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, NavigationPolicyDecision navigationPolicyDecision) { > continueFragmentScrollAfterNavigationPolicy(request, navigationPolicyDecision == NavigationPolicyDecision::ContinueLoad); > }, PolicyDecisionMode::Synchronous); >@@ -1461,6 +1464,7 @@ void FrameLoader::load(FrameLoadRequest& > > if (request.shouldCheckNewWindowPolicy()) { > NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() }; >+ RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType())); > policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), { }, request.frameName(), [this] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) { > continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress); > }); >@@ -1584,6 +1588,7 @@ void FrameLoader::loadWithDocumentLoader > } > > policyChecker().setLoadType(type); >+ RELEASE_ASSERT(!isBackForwardLoadType(type) || history().provisionalItem()); > bool isFormSubmission = formState; > > const String& httpMethod = loader->request().httpMethod(); >@@ -1595,6 +1600,7 @@ void FrameLoader::loadWithDocumentLoader > oldDocumentLoader->setTriggeringAction(WTFMove(action)); > oldDocumentLoader->setLastCheckedRequest(ResourceRequest()); > policyChecker().stopCheck(); >+ RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType()) || history().provisionalItem()); > policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, NavigationPolicyDecision navigationPolicyDecision) { > continueFragmentScrollAfterNavigationPolicy(request, navigationPolicyDecision == NavigationPolicyDecision::ContinueLoad); > }, PolicyDecisionMode::Synchronous); >@@ -1629,6 +1635,7 @@ void FrameLoader::loadWithDocumentLoader > return; > } > >+ RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType()) || history().provisionalItem()); > policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { } /* redirectResponse */, loader, WTFMove(formState), [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, NavigationPolicyDecision navigationPolicyDecision) mutable { > continueLoadAfterNavigationPolicy(request, formState.get(), navigationPolicyDecision, allowNavigationToInvalidURL); > completionHandler(); >@@ -3010,6 +3017,7 @@ void FrameLoader::loadPostRequest(FrameL > return; > } > >+ RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType())); > policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(workingResourceRequest), WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) mutable { > continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy); > completionHandler(); >Index: Source/WebCore/loader/FrameLoader.h >=================================================================== >--- Source/WebCore/loader/FrameLoader.h (revision 240901) >+++ Source/WebCore/loader/FrameLoader.h (working copy) >@@ -92,7 +92,7 @@ struct WindowFeatures; > WEBCORE_EXPORT bool isBackForwardLoadType(FrameLoadType); > WEBCORE_EXPORT bool isReload(FrameLoadType); > >-using ContentPolicyDecisionFunction = WTF::Function<void(PolicyAction)>; >+using ContentPolicyDecisionFunction = WTF::Function<void(PolicyAction, PolicyCheckIdentifier)>; > > class FrameLoader { > WTF_MAKE_NONCOPYABLE(FrameLoader); >@@ -224,7 +224,7 @@ public: > > void setDefersLoading(bool); > >- void checkContentPolicy(const ResourceResponse&, ContentPolicyDecisionFunction&&); >+ void checkContentPolicy(const ResourceResponse&, PolicyCheckIdentifier, ContentPolicyDecisionFunction&&); > > void didExplicitOpen(); > >Index: Source/WebCore/loader/FrameLoaderClient.h >=================================================================== >--- Source/WebCore/loader/FrameLoaderClient.h (revision 240901) >+++ Source/WebCore/loader/FrameLoaderClient.h (working copy) >@@ -107,7 +107,7 @@ enum class PolicyDecisionMode; > > struct StringWithDirection; > >-typedef WTF::Function<void (PolicyAction)> FramePolicyFunction; >+typedef WTF::Function<void (PolicyAction, PolicyCheckIdentifier)> FramePolicyFunction; > > class WEBCORE_EXPORT FrameLoaderClient { > public: >@@ -190,9 +190,9 @@ public: > virtual Frame* dispatchCreatePage(const NavigationAction&) = 0; > virtual void dispatchShow() = 0; > >- virtual void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, FramePolicyFunction&&) = 0; >- virtual void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String& frameName, FramePolicyFunction&&) = 0; >- virtual void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse& redirectResponse, FormState*, PolicyDecisionMode, FramePolicyFunction&&) = 0; >+ virtual void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, PolicyCheckIdentifier, FramePolicyFunction&&) = 0; >+ virtual void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String& frameName, PolicyCheckIdentifier, FramePolicyFunction&&) = 0; >+ virtual void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, const ResourceResponse& redirectResponse, FormState*, PolicyDecisionMode, PolicyCheckIdentifier, FramePolicyFunction&&) = 0; > virtual void cancelPolicyCheck() = 0; > > virtual void dispatchUnableToImplementPolicy(const ResourceError&) = 0; >Index: Source/WebCore/loader/FrameLoaderTypes.h >=================================================================== >--- Source/WebCore/loader/FrameLoaderTypes.h (revision 240901) >+++ Source/WebCore/loader/FrameLoaderTypes.h (working copy) >@@ -29,6 +29,7 @@ > #pragma once > > #include "IntRect.h" >+#include "ProcessIdentifier.h" > > namespace WebCore { > >@@ -66,6 +67,48 @@ enum class FrameLoadType : uint8_t { > ReloadExpiredOnly > }; > >+class PolicyCheckIdentifier { >+public: >+ PolicyCheckIdentifier() = default; >+ >+ static PolicyCheckIdentifier create(); >+ >+ bool isValidFor(PolicyCheckIdentifier); >+ bool operator==(const PolicyCheckIdentifier& other) const { return m_process == other.m_process && m_policyCheck == other.m_policyCheck; } >+ >+ template<class Encoder> void encode(Encoder&) const; >+ template<class Decoder> static Optional<PolicyCheckIdentifier> decode(Decoder&); >+ >+private: >+ PolicyCheckIdentifier(ProcessIdentifier process, uint64_t policyCheck) >+ : m_process(process) >+ , m_policyCheck(policyCheck) >+ { } >+ >+ ProcessIdentifier m_process; >+ uint64_t m_policyCheck { 0 }; >+}; >+ >+template<class Encoder> >+void PolicyCheckIdentifier::encode(Encoder& encoder) const >+{ >+ encoder << m_process << m_policyCheck; >+} >+ >+template<class Decoder> >+Optional<PolicyCheckIdentifier> PolicyCheckIdentifier::decode(Decoder& decoder) >+{ >+ auto process = ProcessIdentifier::decode(decoder); >+ if (!process) >+ return WTF::nullopt; >+ >+ uint64_t policyCheck; >+ if (!decoder.decode(policyCheck)) >+ return WTF::nullopt; >+ >+ return PolicyCheckIdentifier { *process, policyCheck }; >+} >+ > enum class NewFrameOpenerPolicy : uint8_t { > Suppress, > Allow >Index: Source/WebCore/loader/PolicyChecker.cpp >=================================================================== >--- Source/WebCore/loader/PolicyChecker.cpp (revision 240901) >+++ Source/WebCore/loader/PolicyChecker.cpp (working copy) >@@ -71,6 +71,21 @@ static bool isAllowedByContentSecurityPo > return ownerElement->document().contentSecurityPolicy()->allowChildFrameFromSource(url, redirectResponseReceived); > } > >+PolicyCheckIdentifier PolicyCheckIdentifier::create() >+{ >+ static uint64_t identifier = 0; >+ identifier++; >+ return PolicyCheckIdentifier { Process::identifier(), identifier }; >+} >+ >+bool PolicyCheckIdentifier::isValidFor(PolicyCheckIdentifier expectedIdentifier) >+{ >+ RELEASE_ASSERT_WITH_MESSAGE(m_process == expectedIdentifier.m_process, "Received a policy check response for a wrong process"); >+ RELEASE_ASSERT_WITH_MESSAGE(m_policyCheck, "Received 0 as the policy check identifier"); >+ RELEASE_ASSERT_WITH_MESSAGE(m_policyCheck <= expectedIdentifier.m_policyCheck, "Received a policy check response from the future"); >+ return m_policyCheck == expectedIdentifier.m_policyCheck; >+} >+ > PolicyChecker::PolicyChecker(Frame& frame) > : m_frame(frame) > , m_delegateIsDecidingNavigationPolicy(false) >@@ -170,9 +185,16 @@ void PolicyChecker::checkNavigationPolic > > auto blobURLLifetimeExtension = policyDecisionMode == PolicyDecisionMode::Asynchronous ? extendBlobURLLifetimeIfNecessary(request) : CompletionHandlerCallingScope { }; > >+ auto requestIdentifier = PolicyCheckIdentifier::create(); > m_delegateIsDecidingNavigationPolicy = true; > String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute(); >- m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, redirectResponse, formState.get(), policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = WTFMove(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable { >+ m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, redirectResponse, formState.get(), policyDecisionMode, requestIdentifier, >+ [this, function = WTFMove(function), request = ResourceRequest(request), formState = WTFMove(formState), suggestedFilename = WTFMove(suggestedFilename), >+ blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension), requestIdentifier] (PolicyAction policyAction, PolicyCheckIdentifier responseIdentifier) mutable { >+ >+ if (!responseIdentifier.isValidFor(requestIdentifier)) >+ return; >+ > m_delegateIsDecidingNavigationPolicy = false; > > switch (policyAction) { >@@ -206,7 +228,14 @@ void PolicyChecker::checkNewWindowPolicy > > auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request); > >- m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState.get(), frameName, [frame = makeRef(m_frame), request, formState = WTFMove(formState), frameName, navigationAction, function = WTFMove(function), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable { >+ auto requestIdentifier = PolicyCheckIdentifier::create(); >+ m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState.get(), frameName, requestIdentifier, [frame = makeRef(m_frame), request, >+ formState = WTFMove(formState), frameName, navigationAction, function = WTFMove(function), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension), >+ requestIdentifier] (PolicyAction policyAction, PolicyCheckIdentifier responseIdentifier) mutable { >+ >+ if (!responseIdentifier.isValidFor(requestIdentifier)) >+ return; >+ > switch (policyAction) { > case PolicyAction::Download: > frame->loader().client().startDownload(request); >Index: Source/WebKit/ChangeLog >=================================================================== >--- Source/WebKit/ChangeLog (revision 240901) >+++ Source/WebKit/ChangeLog (working copy) >@@ -1,3 +1,51 @@ >+2019-02-02 Ryosuke Niwa <rniwa@webkit.org> >+ >+ Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy >+ https://bugs.webkit.org/show_bug.cgi?id=194189 >+ >+ Reviewed by Geoffrey Garen. >+ >+ Pass the policy check identifier around functions and store it in PolicyDecisionSender >+ so that we can send it back to WebCore with the navigation policy decision. >+ >+ We also store it in WebFrame in the case the policy decision had to be invalidated >+ before the decision was received (via WebFrame::invalidatePolicyListener). >+ >+ * Scripts/webkit/messages.py: >+ * UIProcess/ProvisionalPageProxy.cpp: >+ (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync): >+ (WebKit::ProvisionalPageProxy::decidePolicyForResponse): >+ * UIProcess/ProvisionalPageProxy.h: >+ * UIProcess/WebPageProxy.cpp: >+ (WebKit::WebPageProxy::PolicyDecisionSender): Added PolicyCheckIdentifier as a member. >+ (WebKit::WebPageProxy::PolicyDecisionSender::create): >+ (WebKit::WebPageProxy::PolicyDecisionSender::send): >+ (WebKit::WebPageProxy::PolicyDecisionSender::PolicyDecisionSender): >+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision): >+ (WebKit::WebPageProxy::decidePolicyForNavigationActionAsync): >+ (WebKit::WebPageProxy::decidePolicyForNavigationActionAsyncShared): >+ (WebKit::WebPageProxy::decidePolicyForNavigationAction): >+ (WebKit::WebPageProxy::decidePolicyForNavigationActionSync): >+ (WebKit::WebPageProxy::decidePolicyForNewWindowAction): >+ (WebKit::WebPageProxy::decidePolicyForResponse): >+ (WebKit::WebPageProxy::decidePolicyForResponseShared): >+ * UIProcess/WebPageProxy.h: >+ * UIProcess/WebPageProxy.messages.in: >+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp: >+ (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse): >+ (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction): >+ (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): >+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.h: >+ * WebProcess/WebPage/WebFrame.cpp: >+ (WebKit::WebFrame::setUpPolicyListener): >+ (WebKit::WebFrame::invalidatePolicyListener): >+ (WebKit::WebFrame::didReceivePolicyDecision): >+ * WebProcess/WebPage/WebFrame.h: >+ * WebProcess/WebPage/WebPage.cpp: >+ (WebKit::WebPage::didReceivePolicyDecision): >+ * WebProcess/WebPage/WebPage.h: >+ * WebProcess/WebPage/WebPage.messages.in: >+ > 2019-02-02 Simon Fraser <simon.fraser@apple.com> > > Tidy up data memebers of FrameView and related classes to shrink class sizes >Index: Source/WebKit/Scripts/webkit/messages.py >=================================================================== >--- Source/WebKit/Scripts/webkit/messages.py (revision 240901) >+++ Source/WebKit/Scripts/webkit/messages.py (working copy) >@@ -420,6 +420,7 @@ def headers_for_type(type): > 'WebCore::PaymentMethodUpdate': ['<WebCore/ApplePaySessionPaymentRequest.h>'], > 'WebCore::PluginInfo': ['<WebCore/PluginData.h>'], > 'WebCore::PolicyAction': ['<WebCore/FrameLoaderTypes.h>'], >+ 'WebCore::PolicyCheckIdentifier': ['<WebCore/FrameLoaderTypes.h>'], > 'WebCore::RecentSearch': ['<WebCore/SearchPopupMenu.h>'], > 'WebCore::RouteSharingPolicy': ['<WebCore/AudioSession.h>'], > 'WebCore::SWServerConnectionIdentifier': ['<WebCore/ServiceWorkerTypes.h>'], >Index: Source/WebKit/UIProcess/ProvisionalPageProxy.cpp >=================================================================== >--- Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (revision 240901) >+++ Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (working copy) >@@ -262,16 +262,20 @@ void ProvisionalPageProxy::didChangeProv > m_page.didChangeProvisionalURLForFrameShared(m_process.copyRef(), frameID, navigationID, WTFMove(url)); > } > >-void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID) >+void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, WebCore::PolicyCheckIdentifier identifier, >+ uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, >+ WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID) > { > ASSERT(m_mainFrame); > ASSERT(m_mainFrame->frameID() == frameID); >- m_page.decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID); >+ m_page.decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), identifier, navigationID, WTFMove(navigationActionData), >+ WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID); > } > >-void ProvisionalPageProxy::decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData) >+void ProvisionalPageProxy::decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, WebCore::PolicyCheckIdentifier identifier, >+ uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData) > { >- m_page.decidePolicyForResponseShared(m_process.copyRef(), frameID, frameSecurityOrigin, navigationID, response, request, canShowMIMEType, listenerID, userData); >+ m_page.decidePolicyForResponseShared(m_process.copyRef(), frameID, frameSecurityOrigin, identifier, navigationID, response, request, canShowMIMEType, listenerID, userData); > } > > void ProvisionalPageProxy::startURLSchemeTask(URLSchemeTaskParameters&& parameters) >Index: Source/WebKit/UIProcess/ProvisionalPageProxy.h >=================================================================== >--- Source/WebKit/UIProcess/ProvisionalPageProxy.h (revision 240901) >+++ Source/WebKit/UIProcess/ProvisionalPageProxy.h (working copy) >@@ -73,8 +73,11 @@ private: > void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final; > void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&) final; > >- void decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID); >- 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 decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&&, >+ uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, >+ WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID); >+ void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, const WebCore::ResourceResponse&, >+ const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&); > void didChangeProvisionalURLForFrame(uint64_t frameID, uint64_t navigationID, URL&&); > void didNavigateWithNavigationData(const WebNavigationDataStore&, uint64_t frameID); > void didPerformClientRedirect(const String& sourceURLString, const String& destinationURLString, uint64_t frameID); >Index: Source/WebKit/UIProcess/WebPageProxy.cpp >=================================================================== >--- Source/WebKit/UIProcess/WebPageProxy.cpp (revision 240901) >+++ Source/WebKit/UIProcess/WebPageProxy.cpp (working copy) >@@ -2674,22 +2674,26 @@ void WebPageProxy::centerSelectionInVisi > > class WebPageProxy::PolicyDecisionSender : public RefCounted<PolicyDecisionSender> { > public: >- using SendFunction = CompletionHandler<void(PolicyAction, uint64_t newNavigationID, DownloadID, Optional<WebsitePoliciesData>)>; >- static Ref<PolicyDecisionSender> create(SendFunction&& sendFunction) >+ using SendFunction = CompletionHandler<void(PolicyCheckIdentifier, PolicyAction, uint64_t newNavigationID, DownloadID, Optional<WebsitePoliciesData>)>; >+ >+ static Ref<PolicyDecisionSender> create(PolicyCheckIdentifier identifier, SendFunction&& sendFunction) > { >- return adoptRef(*new PolicyDecisionSender(WTFMove(sendFunction))); >+ return adoptRef(*new PolicyDecisionSender(identifier, WTFMove(sendFunction))); > } > > template<typename... Args> void send(Args... args) > { > if (m_sendFunction) >- m_sendFunction(std::forward<Args>(args)...); >+ m_sendFunction(m_identifier, std::forward<Args>(args)...); > } > private: >- PolicyDecisionSender(SendFunction sendFunction) >- : m_sendFunction(WTFMove(sendFunction)) { } >+ PolicyDecisionSender(PolicyCheckIdentifier identifier, SendFunction sendFunction) >+ : m_sendFunction(WTFMove(sendFunction)) >+ , m_identifier(identifier) >+ { } > > SendFunction m_sendFunction; >+ PolicyCheckIdentifier m_identifier; > }; > > void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, API::Navigation* navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, WebFrameProxy& frame, API::WebsitePolicies* policies, Ref<PolicyDecisionSender>&& sender) >@@ -2709,7 +2713,8 @@ void WebPageProxy::receivedNavigationPol > return; > } > >- process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, [this, protectedThis = makeRef(*this), policyAction, navigation = makeRef(*navigation), data = WTFMove(data), sender = WTFMove(sender), processSwapRequestedByClient](Ref<WebProcessProxy>&& processForNavigation, SuspendedPageProxy* destinationSuspendedPage, const String& reason) mutable { >+ process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, [this, protectedThis = makeRef(*this), policyAction, navigation = makeRef(*navigation), >+ data = WTFMove(data), sender = WTFMove(sender), processSwapRequestedByClient] (Ref<WebProcessProxy>&& processForNavigation, SuspendedPageProxy* destinationSuspendedPage, const String& reason) mutable { > // If the navigation has been destroyed, then no need to proceed. > if (isClosed() || !navigationState().hasNavigation(navigation->navigationID())) { > receivedPolicyDecision(policyAction, navigation.ptr(), WTFMove(data), WTFMove(sender)); >@@ -4355,22 +4360,33 @@ void WebPageProxy::beginSafeBrowsingChec > } > #endif > >-void WebPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID) >-{ >- decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID); >+void WebPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, PolicyCheckIdentifier identifier, uint64_t navigationID, >+ NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, >+ IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID) >+{ >+ decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), identifier, navigationID, WTFMove(navigationActionData), >+ WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID); > } > >-void WebPageProxy::decidePolicyForNavigationActionAsyncShared(Ref<WebProcessProxy>&& process, uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID) >+void WebPageProxy::decidePolicyForNavigationActionAsyncShared(Ref<WebProcessProxy>&& process, uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, >+ WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, >+ const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, >+ const UserData& userData, uint64_t listenerID) > { > auto* frame = process->webFrame(frameID); > MESSAGE_CHECK(process, frame); > >- decidePolicyForNavigationAction(process.copyRef(), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, PolicyDecisionSender::create([this, protectedThis = makeRef(*this), frameID, listenerID, process = process.copyRef()] (auto... args) { >+ auto sender = PolicyDecisionSender::create(identifier, [this, protectedThis = makeRef(*this), frameID, listenerID, process = process.copyRef()] (auto... args) { > process->send(Messages::WebPage::DidReceivePolicyDecision(frameID, listenerID, args...), m_pageID); >- })); >+ }); >+ >+ decidePolicyForNavigationAction(process.copyRef(), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, >+ originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(sender)); > } > >-void WebPageProxy::decidePolicyForNavigationAction(Ref<WebProcessProxy>&& process, WebFrameProxy& frame, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, Ref<PolicyDecisionSender>&& sender) >+void WebPageProxy::decidePolicyForNavigationAction(Ref<WebProcessProxy>&& process, WebFrameProxy& frame, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, >+ NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, >+ IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, Ref<PolicyDecisionSender>&& sender) > { > LOG(Loading, "WebPageProxy::decidePolicyForNavigationAction - Original URL %s, current target URL %s", originalRequest.url().string().utf8().data(), request.url().string().utf8().data()); > >@@ -4429,7 +4445,7 @@ void WebPageProxy::decidePolicyForNaviga > shouldExpectSafeBrowsingResult = ShouldExpectSafeBrowsingResult::No; > > auto listener = makeRef(frame.setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(frame), sender = WTFMove(sender), navigation] (PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning) mutable { >- >+ > auto completionHandler = [this, protectedThis = protectedThis.copyRef(), frame = frame.copyRef(), sender = WTFMove(sender), navigation = WTFMove(navigation), processSwapRequestedByClient, policies = makeRefPtr(policies)] (PolicyAction policyAction) mutable { > receivedNavigationPolicyDecision(policyAction, navigation.get(), processSwapRequestedByClient, frame, policies.get(), WTFMove(sender)); > }; >@@ -4553,9 +4569,12 @@ void WebPageProxy::logFrameNavigation(co > } > #endif > >-void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply) >+void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&& frameSecurityOrigin, PolicyCheckIdentifier identifier, >+ uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, >+ const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, >+ const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply) > { >- auto sender = PolicyDecisionSender::create(WTFMove(reply)); >+ auto sender = PolicyDecisionSender::create(identifier, WTFMove(reply)); > > auto* frame = m_process->webFrame(frameID); > if (!frame) { >@@ -4569,13 +4588,15 @@ void WebPageProxy::decidePolicyForNaviga > RELEASE_ASSERT(frame); > } > >- decidePolicyForNavigationAction(m_process.copyRef(), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, sender.copyRef()); >+ decidePolicyForNavigationAction(m_process.copyRef(), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), >+ originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, sender.copyRef()); > > // If the client did not respond synchronously, proceed with the load. > sender->send(PolicyAction::Use, navigationID, DownloadID(), WTF::nullopt); > } > >-void WebPageProxy::decidePolicyForNewWindowAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, NavigationActionData&& navigationActionData, ResourceRequest&& request, const String& frameName, uint64_t listenerID, const UserData& userData) >+void WebPageProxy::decidePolicyForNewWindowAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, PolicyCheckIdentifier identifier, >+ NavigationActionData&& navigationActionData, ResourceRequest&& request, const String& frameName, uint64_t listenerID, const UserData& userData) > { > PageClientProtector protector(pageClient()); > >@@ -4583,13 +4604,16 @@ void WebPageProxy::decidePolicyForNewWin > MESSAGE_CHECK(m_process, frame); > MESSAGE_CHECK_URL(m_process, request.url()); > >- auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), listenerID, frameID] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning) mutable { >+ auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), identifier, listenerID, frameID] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning) mutable { > // FIXME: Assert the API::WebsitePolicies* is nullptr here once clients of WKFramePolicyListenerUseWithPolicies go away. > RELEASE_ASSERT(processSwapRequestedByClient == ProcessSwapRequestedByClient::No); > ASSERT_UNUSED(safeBrowsingWarning, !safeBrowsingWarning); >- receivedPolicyDecision(policyAction, nullptr, WTF::nullopt, PolicyDecisionSender::create([this, protectedThis = WTFMove(protectedThis), frameID, listenerID] (auto... args) { >+ >+ auto sender = PolicyDecisionSender::create(identifier, [this, protectedThis = WTFMove(protectedThis), frameID, listenerID] (auto... args) { > m_process->send(Messages::WebPage::DidReceivePolicyDecision(frameID, listenerID, args...), m_pageID); >- })); >+ }); >+ >+ receivedPolicyDecision(policyAction, nullptr, WTF::nullopt, WTFMove(sender)); > }, ShouldExpectSafeBrowsingResult::No)); > > if (m_policyClient) >@@ -4609,12 +4633,14 @@ void WebPageProxy::decidePolicyForNewWin > > } > >-void WebPageProxy::decidePolicyForResponse(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData) >+void WebPageProxy::decidePolicyForResponse(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, PolicyCheckIdentifier identifier, >+ uint64_t navigationID, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData) > { >- decidePolicyForResponseShared(m_process.copyRef(), frameID, frameSecurityOrigin, navigationID, response, request, canShowMIMEType, listenerID, userData); >+ decidePolicyForResponseShared(m_process.copyRef(), frameID, frameSecurityOrigin, identifier, navigationID, response, request, canShowMIMEType, listenerID, userData); > } > >-void WebPageProxy::decidePolicyForResponseShared(Ref<WebProcessProxy>&& process, uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData) >+void WebPageProxy::decidePolicyForResponseShared(Ref<WebProcessProxy>&& process, uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, PolicyCheckIdentifier identifier, >+ uint64_t navigationID, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData) > { > PageClientProtector protector(pageClient()); > >@@ -4626,13 +4652,17 @@ void WebPageProxy::decidePolicyForRespon > MESSAGE_CHECK_URL(process, response.url()); > > RefPtr<API::Navigation> navigation = navigationID ? m_navigationState->navigation(navigationID) : nullptr; >- auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frameID, listenerID, navigation = WTFMove(navigation), process = process.copyRef()] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning) mutable { >+ auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frameID, identifier, listenerID, navigation = WTFMove(navigation), >+ process = process.copyRef()] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning) mutable { > // FIXME: Assert the API::WebsitePolicies* is nullptr here once clients of WKFramePolicyListenerUseWithPolicies go away. > RELEASE_ASSERT(processSwapRequestedByClient == ProcessSwapRequestedByClient::No); > ASSERT_UNUSED(safeBrowsingWarning, !safeBrowsingWarning); >- receivedPolicyDecision(policyAction, navigation.get(), WTF::nullopt, PolicyDecisionSender::create([this, protectedThis = WTFMove(protectedThis), frameID, listenerID, process = WTFMove(process)] (auto... args) { >+ >+ auto sender = PolicyDecisionSender::create(identifier, [this, protectedThis = WTFMove(protectedThis), frameID, listenerID, process = WTFMove(process)] (auto... args) { > process->send(Messages::WebPage::DidReceivePolicyDecision(frameID, listenerID, args...), m_pageID); >- })); >+ }); >+ >+ receivedPolicyDecision(policyAction, navigation.get(), WTF::nullopt, WTFMove(sender)); > }, ShouldExpectSafeBrowsingResult::No)); > > if (m_policyClient) >Index: Source/WebKit/UIProcess/WebPageProxy.h >=================================================================== >--- Source/WebKit/UIProcess/WebPageProxy.h (revision 240901) >+++ Source/WebKit/UIProcess/WebPageProxy.h (working copy) >@@ -1436,8 +1436,11 @@ public: > void didPerformClientRedirectShared(Ref<WebProcessProxy>&&, const String& sourceURLString, const String& destinationURLString, uint64_t frameID); > void didNavigateWithNavigationDataShared(Ref<WebProcessProxy>&&, const WebNavigationDataStore&, uint64_t frameID); > void didChangeProvisionalURLForFrameShared(Ref<WebProcessProxy>&&, uint64_t frameID, uint64_t navigationID, URL&&); >- void decidePolicyForNavigationActionAsyncShared(Ref<WebProcessProxy>&&, uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID); >- void decidePolicyForResponseShared(Ref<WebProcessProxy>&&, uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&); >+ void decidePolicyForNavigationActionAsyncShared(Ref<WebProcessProxy>&&, uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, WebCore::PolicyCheckIdentifier, >+ uint64_t navigationID, NavigationActionData&&, FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, >+ IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID); >+ void decidePolicyForResponseShared(Ref<WebProcessProxy>&&, uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, WebCore::PolicyCheckIdentifier, >+ uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&); > void startURLSchemeTaskShared(Ref<WebProcessProxy>&&, URLSchemeTaskParameters&&); > void loadDataWithNavigationShared(Ref<WebProcessProxy>&&, API::Navigation&, const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, Optional<WebsitePoliciesData>&& = WTF::nullopt); > void loadRequestWithNavigationShared(Ref<WebProcessProxy>&&, API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, Optional<WebsitePoliciesData>&& = WTF::nullopt); >@@ -1523,11 +1526,19 @@ private: > > void didDestroyNavigation(uint64_t navigationID); > >- void decidePolicyForNavigationAction(Ref<WebProcessProxy>&&, WebFrameProxy&, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, Ref<PolicyDecisionSender>&&); >- void decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID); >- void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, 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 decidePolicyForNavigationAction(Ref<WebProcessProxy>&&, WebFrameProxy&, WebCore::SecurityOriginData&&, uint64_t navigationID, NavigationActionData&&, >+ FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, >+ WebCore::ResourceResponse&& redirectResponse, const UserData&, Ref<PolicyDecisionSender>&&); >+ void decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, >+ FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, >+ WebCore::ResourceResponse&& redirectResponse, const UserData&, uint64_t listenerID); >+ void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, >+ FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, >+ WebCore::ResourceResponse&& redirectResponse, const UserData&, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&); >+ void decidePolicyForNewWindowAction(uint64_t frameID, const WebCore::SecurityOriginData&, WebCore::PolicyCheckIdentifier, NavigationActionData&&, >+ WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&); >+ void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData&, WebCore::PolicyCheckIdentifier, 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&); > void beginSafeBrowsingCheck(const URL&, bool, WebFramePolicyListenerProxy&); > >Index: Source/WebKit/UIProcess/WebPageProxy.messages.in >=================================================================== >--- Source/WebKit/UIProcess/WebPageProxy.messages.in (revision 240901) >+++ Source/WebKit/UIProcess/WebPageProxy.messages.in (working copy) >@@ -105,10 +105,10 @@ messages -> WebPageProxy { > #endif > > # 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, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, 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, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData) -> (enum:uint8_t WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, 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) >+ DecidePolicyForResponse(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, WebCore::PolicyCheckIdentifier policyCheckIdentifier, 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, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, uint64_t listenerID) >+ DecidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, struct WebCore::SecurityOriginData frameSecurityOrigin, WebCore::PolicyCheckIdentifier policyCheckIdentifier, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData) -> (WebCore::PolicyCheckIdentifier policyCheckIdentifier, enum:uint8_t WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, Optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed >+ DecidePolicyForNewWindowAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, WebCore::PolicyCheckIdentifier policyCheckIdentifier, 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) > > # Progress messages >Index: Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp >=================================================================== >--- Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (revision 240901) >+++ Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (working copy) >@@ -730,16 +730,16 @@ void WebFrameLoaderClient::dispatchShow( > webPage->show(); > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceResponse& response, const ResourceRequest& request, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceResponse& response, const ResourceRequest& request, WebCore::PolicyCheckIdentifier identifier, FramePolicyFunction&& function) > { > WebPage* webPage = m_frame ? m_frame->page() : nullptr; > if (!webPage) { >- function(PolicyAction::Ignore); >+ function(PolicyAction::Ignore, identifier); > return; > } > > if (!request.url().string()) { >- function(PolicyAction::Use); >+ function(PolicyAction::Use, identifier); > return; > } > >@@ -748,7 +748,7 @@ void WebFrameLoaderClient::dispatchDecid > // Notify the bundle client. > WKBundlePagePolicyAction policy = webPage->injectedBundlePolicyClient().decidePolicyForResponse(webPage, m_frame, response, request, userData); > if (policy == WKBundlePagePolicyActionUse) { >- function(PolicyAction::Use); >+ function(PolicyAction::Use, identifier); > return; > } > >@@ -759,16 +759,18 @@ void WebFrameLoaderClient::dispatchDecid > auto navigationID = policyDocumentLoader ? static_cast<WebDocumentLoader&>(*policyDocumentLoader).navigationID() : 0; > > Ref<WebFrame> protector(*m_frame); >- uint64_t listenerID = m_frame->setUpPolicyListener(WTFMove(function), WebFrame::ForNavigationAction::No); >- if (!webPage->send(Messages::WebPageProxy::DecidePolicyForResponse(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), navigationID, response, request, canShowResponse, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())))) >- m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { }); >+ uint64_t listenerID = m_frame->setUpPolicyListener(identifier, WTFMove(function), WebFrame::ForNavigationAction::No); >+ if (!webPage->send(Messages::WebPageProxy::DecidePolicyForResponse(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), identifier, navigationID, response, request, >+ canShowResponse, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())))) >+ m_frame->didReceivePolicyDecision(listenerID, identifier, PolicyAction::Ignore, 0, { }, { }); > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& navigationAction, const ResourceRequest& request, >+ FormState* formState, const String& frameName, WebCore::PolicyCheckIdentifier identifier, FramePolicyFunction&& function) > { > WebPage* webPage = m_frame ? m_frame->page() : nullptr; > if (!webPage) { >- function(PolicyAction::Ignore); >+ function(PolicyAction::Ignore, identifier); > return; > } > >@@ -779,11 +781,11 @@ void WebFrameLoaderClient::dispatchDecid > // Notify the bundle client. > WKBundlePagePolicyAction policy = webPage->injectedBundlePolicyClient().decidePolicyForNewWindowAction(webPage, m_frame, action.ptr(), request, frameName, userData); > if (policy == WKBundlePagePolicyActionUse) { >- function(PolicyAction::Use); >+ function(PolicyAction::Use, identifier); > return; > } > >- uint64_t listenerID = m_frame->setUpPolicyListener(WTFMove(function), WebFrame::ForNavigationAction::No); >+ uint64_t listenerID = m_frame->setUpPolicyListener(identifier, WTFMove(function), WebFrame::ForNavigationAction::No); > > NavigationActionData navigationActionData; > navigationActionData.navigationType = action->navigationType(); >@@ -797,7 +799,8 @@ void WebFrameLoaderClient::dispatchDecid > navigationActionData.downloadAttribute = navigationAction.downloadAttribute(); > > WebCore::Frame* coreFrame = m_frame ? m_frame->coreFrame() : nullptr; >- webPage->send(Messages::WebPageProxy::DecidePolicyForNewWindowAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), navigationActionData, request, frameName, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()))); >+ webPage->send(Messages::WebPageProxy::DecidePolicyForNewWindowAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), identifier, navigationActionData, request, >+ frameName, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()))); > } > > void WebFrameLoaderClient::applyToDocumentLoader(WebsitePoliciesData&& websitePolicies) >@@ -818,11 +821,12 @@ void WebFrameLoaderClient::applyToDocume > WebsitePoliciesData::applyToDocumentLoader(WTFMove(websitePolicies), *documentLoader); > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& navigationAction, const ResourceRequest& request, const ResourceResponse& redirectResponse, FormState* formState, PolicyDecisionMode policyDecisionMode, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& navigationAction, const ResourceRequest& request, const ResourceResponse& redirectResponse, >+ FormState* formState, PolicyDecisionMode policyDecisionMode, WebCore::PolicyCheckIdentifier requestIdentifier, FramePolicyFunction&& function) > { > WebPage* webPage = m_frame ? m_frame->page() : nullptr; > if (!webPage) { >- function(PolicyAction::Ignore); >+ function(PolicyAction::Ignore, requestIdentifier); > return; > } > >@@ -830,7 +834,7 @@ void WebFrameLoaderClient::dispatchDecid > > // Always ignore requests with empty URLs. > if (request.isEmpty()) { >- function(PolicyAction::Ignore); >+ function(PolicyAction::Ignore, requestIdentifier); > return; > } > >@@ -841,11 +845,11 @@ void WebFrameLoaderClient::dispatchDecid > // Notify the bundle client. > WKBundlePagePolicyAction policy = webPage->injectedBundlePolicyClient().decidePolicyForNavigationAction(webPage, m_frame, action.ptr(), request, userData); > if (policy == WKBundlePagePolicyActionUse) { >- function(PolicyAction::Use); >+ function(PolicyAction::Use, requestIdentifier); > return; > } >- >- uint64_t listenerID = m_frame->setUpPolicyListener(WTFMove(function), WebFrame::ForNavigationAction::Yes); >+ >+ uint64_t listenerID = m_frame->setUpPolicyListener(requestIdentifier, WTFMove(function), WebFrame::ForNavigationAction::Yes); > > ASSERT(navigationAction.requester()); > auto requester = navigationAction.requester().value(); >@@ -881,7 +885,7 @@ void WebFrameLoaderClient::dispatchDecid > > WebCore::Frame* coreFrame = m_frame->coreFrame(); > if (!coreFrame) >- return function(PolicyAction::Ignore); >+ return function(PolicyAction::Ignore, requestIdentifier); > WebDocumentLoader* documentLoader = static_cast<WebDocumentLoader*>(coreFrame->loader().policyDocumentLoader()); > if (!documentLoader) { > // FIXME: When we receive a redirect after the navigation policy has been decided for the initial request, >@@ -898,22 +902,28 @@ void WebFrameLoaderClient::dispatchDecid > > if (policyDecisionMode == PolicyDecisionMode::Synchronous) { > uint64_t newNavigationID; >+ WebCore::PolicyCheckIdentifier responseIdentifier; > PolicyAction policyAction; > DownloadID downloadID; > 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, IPC::FormDataReference { request.httpBody() }, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(policyAction, newNavigationID, downloadID, websitePolicies))) { >- m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { }); >+ if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), m_frame->isMainFrame(), SecurityOriginData::fromFrame(coreFrame), >+ requestIdentifier, documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, >+ IPC::FormDataReference { request.httpBody() }, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), >+ Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(responseIdentifier, policyAction, newNavigationID, downloadID, websitePolicies))) { >+ m_frame->didReceivePolicyDecision(listenerID, responseIdentifier, PolicyAction::Ignore, 0, { }, { }); > return; > } > >- m_frame->didReceivePolicyDecision(listenerID, policyAction, 0, downloadID, { }); >+ m_frame->didReceivePolicyDecision(listenerID, responseIdentifier, policyAction, 0, downloadID, { }); > return; > } > > ASSERT(policyDecisionMode == PolicyDecisionMode::Asynchronous); >- if (!webPage->send(Messages::WebPageProxy::DecidePolicyForNavigationActionAsync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, IPC::FormDataReference { request.httpBody() }, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), listenerID))) >- m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { }); >+ if (!webPage->send(Messages::WebPageProxy::DecidePolicyForNavigationActionAsync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), >+ requestIdentifier, documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, >+ IPC::FormDataReference { request.httpBody() }, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), listenerID))) >+ m_frame->didReceivePolicyDecision(listenerID, requestIdentifier, PolicyAction::Ignore, 0, { }, { }); > } > > void WebFrameLoaderClient::cancelPolicyCheck() >Index: Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h >=================================================================== >--- Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (revision 240901) >+++ Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (working copy) >@@ -123,9 +123,9 @@ private: > WebCore::Frame* dispatchCreatePage(const WebCore::NavigationAction&) final; > void dispatchShow() final; > >- void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) final; >- void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const String& frameName, WebCore::FramePolicyFunction&&) final; >- void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const String& frameName, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) final; > void cancelPolicyCheck() final; > > void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) final; >Index: Source/WebKit/WebProcess/WebPage/WebFrame.cpp >=================================================================== >--- Source/WebKit/WebProcess/WebPage/WebFrame.cpp (revision 240901) >+++ Source/WebKit/WebProcess/WebPage/WebFrame.cpp (working copy) >@@ -210,12 +210,13 @@ void WebFrame::invalidate() > m_coreFrame = 0; > } > >-uint64_t WebFrame::setUpPolicyListener(WebCore::FramePolicyFunction&& policyFunction, ForNavigationAction forNavigationAction) >+uint64_t WebFrame::setUpPolicyListener(WebCore::PolicyCheckIdentifier identifier, WebCore::FramePolicyFunction&& policyFunction, ForNavigationAction forNavigationAction) > { > // FIXME: <rdar://5634381> We need to support multiple active policy listeners. > > invalidatePolicyListener(); > >+ m_policyIdentifier = identifier; > m_policyListenerID = generateListenerID(); > m_policyFunction = WTFMove(policyFunction); > m_policyFunctionForNavigationAction = forNavigationAction; >@@ -244,8 +245,10 @@ void WebFrame::invalidatePolicyListener( > > m_policyDownloadID = { }; > m_policyListenerID = 0; >+ auto identifier = m_policyIdentifier; >+ m_policyIdentifier = WTF::nullopt; > if (auto function = std::exchange(m_policyFunction, nullptr)) >- function(PolicyAction::Ignore); >+ function(PolicyAction::Ignore, *identifier); > m_policyFunctionForNavigationAction = ForNavigationAction::No; > > auto willSubmitFormCompletionHandlers = WTFMove(m_willSubmitFormCompletionHandlers); >@@ -253,11 +256,14 @@ void WebFrame::invalidatePolicyListener( > completionHandler(); > } > >-void WebFrame::didReceivePolicyDecision(uint64_t listenerID, PolicyAction action, uint64_t navigationID, DownloadID downloadID, Optional<WebsitePoliciesData>&& websitePolicies) >+void WebFrame::didReceivePolicyDecision(uint64_t listenerID, WebCore::PolicyCheckIdentifier identifier, PolicyAction action, uint64_t navigationID, DownloadID downloadID, Optional<WebsitePoliciesData>&& websitePolicies) > { > if (!m_coreFrame || !m_policyListenerID || listenerID != m_policyListenerID || !m_policyFunction) > return; > >+ ASSERT(identifier == m_policyIdentifier); >+ m_policyIdentifier = WTF::nullopt; >+ > FramePolicyFunction function = WTFMove(m_policyFunction); > bool forNavigationAction = m_policyFunctionForNavigationAction == ForNavigationAction::Yes; > >@@ -272,7 +278,7 @@ void WebFrame::didReceivePolicyDecision( > documentLoader->setNavigationID(navigationID); > } > >- function(action); >+ function(action, identifier); > } > > void WebFrame::startDownload(const WebCore::ResourceRequest& request, const String& suggestedName) >Index: Source/WebKit/WebProcess/WebPage/WebFrame.h >=================================================================== >--- Source/WebKit/WebProcess/WebPage/WebFrame.h (revision 240901) >+++ Source/WebKit/WebProcess/WebPage/WebFrame.h (working copy) >@@ -83,9 +83,9 @@ public: > uint64_t frameID() const { return m_frameID; } > > enum class ForNavigationAction { No, Yes }; >- uint64_t setUpPolicyListener(WebCore::FramePolicyFunction&&, ForNavigationAction); >+ uint64_t setUpPolicyListener(WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&, ForNavigationAction); > void invalidatePolicyListener(); >- void didReceivePolicyDecision(uint64_t listenerID, WebCore::PolicyAction, uint64_t navigationID, DownloadID, Optional<WebsitePoliciesData>&&); >+ void didReceivePolicyDecision(uint64_t listenerID, WebCore::PolicyCheckIdentifier, WebCore::PolicyAction, uint64_t navigationID, DownloadID, Optional<WebsitePoliciesData>&&); > > uint64_t setUpWillSubmitFormListener(CompletionHandler<void()>&&); > void continueWillSubmitForm(uint64_t); >@@ -178,6 +178,7 @@ private: > WebCore::Frame* m_coreFrame { nullptr }; > > uint64_t m_policyListenerID { 0 }; >+ Optional<WebCore::PolicyCheckIdentifier> m_policyIdentifier; > WebCore::FramePolicyFunction m_policyFunction; > ForNavigationAction m_policyFunctionForNavigationAction { ForNavigationAction::No }; > HashMap<uint64_t, CompletionHandler<void()>> m_willSubmitFormCompletionHandlers; >Index: Source/WebKit/WebProcess/WebPage/WebPage.cpp >=================================================================== >--- Source/WebKit/WebProcess/WebPage/WebPage.cpp (revision 240901) >+++ Source/WebKit/WebProcess/WebPage/WebPage.cpp (working copy) >@@ -3078,12 +3078,12 @@ void WebPage::setSessionID(PAL::SessionI > m_page->setSessionID(sessionID); > } > >-void WebPage::didReceivePolicyDecision(uint64_t frameID, uint64_t listenerID, PolicyAction policyAction, uint64_t navigationID, const DownloadID& downloadID, Optional<WebsitePoliciesData>&& websitePolicies) >+void WebPage::didReceivePolicyDecision(uint64_t frameID, uint64_t listenerID, PolicyCheckIdentifier identifier, PolicyAction policyAction, uint64_t navigationID, const DownloadID& downloadID, Optional<WebsitePoliciesData>&& websitePolicies) > { > WebFrame* frame = WebProcess::singleton().webFrame(frameID); > if (!frame) > return; >- frame->didReceivePolicyDecision(listenerID, policyAction, navigationID, downloadID, WTFMove(websitePolicies)); >+ frame->didReceivePolicyDecision(listenerID, identifier, policyAction, navigationID, downloadID, WTFMove(websitePolicies)); > } > > void WebPage::continueWillSubmitForm(uint64_t frameID, uint64_t listenerID) >Index: Source/WebKit/WebProcess/WebPage/WebPage.h >=================================================================== >--- Source/WebKit/WebProcess/WebPage/WebPage.h (revision 240901) >+++ Source/WebKit/WebProcess/WebPage/WebPage.h (working copy) >@@ -1314,7 +1314,7 @@ private: > bool parentProcessHasServiceWorkerEntitlement() const { return true; } > #endif > >- void didReceivePolicyDecision(uint64_t frameID, uint64_t listenerID, WebCore::PolicyAction, uint64_t navigationID, const DownloadID&, Optional<WebsitePoliciesData>&&); >+ void didReceivePolicyDecision(uint64_t frameID, uint64_t listenerID, WebCore::PolicyCheckIdentifier, WebCore::PolicyAction, uint64_t navigationID, const DownloadID&, Optional<WebsitePoliciesData>&&); > void continueWillSubmitForm(uint64_t frameID, uint64_t listenerID); > void setUserAgent(const String&); > void setCustomTextEncodingName(const String&); >Index: Source/WebKit/WebProcess/WebPage/WebPage.messages.in >=================================================================== >--- Source/WebKit/WebProcess/WebPage/WebPage.messages.in (revision 240901) >+++ Source/WebKit/WebProcess/WebPage/WebPage.messages.in (working copy) >@@ -171,7 +171,7 @@ messages -> WebPage LegacyReceiver { > DidRemoveBackForwardItem(struct WebCore::BackForwardItemIdentifier backForwardItemID) > > UpdateWebsitePolicies(struct WebKit::WebsitePoliciesData websitePolicies) >- DidReceivePolicyDecision(uint64_t frameID, uint64_t listenerID, enum:uint8_t WebCore::PolicyAction policyAction, uint64_t navigationID, WebKit::DownloadID downloadID, Optional<WebKit::WebsitePoliciesData> websitePolicies) >+ DidReceivePolicyDecision(uint64_t frameID, uint64_t listenerID, WebCore::PolicyCheckIdentifier policyCheckIdentifier, enum:uint8_t WebCore::PolicyAction policyAction, uint64_t navigationID, WebKit::DownloadID downloadID, Optional<WebKit::WebsitePoliciesData> websitePolicies) > ContinueWillSubmitForm(uint64_t frameID, uint64_t listenerID) > > ClearSelection() >Index: Source/WebKitLegacy/mac/ChangeLog >=================================================================== >--- Source/WebKitLegacy/mac/ChangeLog (revision 240901) >+++ Source/WebKitLegacy/mac/ChangeLog (working copy) >@@ -1,3 +1,28 @@ >+2019-02-02 Ryosuke Niwa <rniwa@webkit.org> >+ >+ Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy >+ https://bugs.webkit.org/show_bug.cgi?id=194189 >+ >+ Reviewed by Geoffrey Garen. >+ >+ Pass the policy check identifier around functions and store it in WebFramePolicyListener >+ so that we can send it back to WebCore with the navigation policy decision. >+ >+ * WebCoreSupport/WebFrameLoaderClient.h: >+ * WebCoreSupport/WebFrameLoaderClient.mm: >+ (WebFrameLoaderClient::dispatchDecidePolicyForResponse): >+ (WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction): >+ (WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): >+ (WebFrameLoaderClient::dispatchWillSubmitForm): >+ (WebFrameLoaderClient::setUpPolicyListener): >+ (-[WebFramePolicyListener initWithFrame:identifier:policyFunction:defaultPolicy:]): >+ (-[WebFramePolicyListener initWithFrame:identifier:policyFunction:defaultPolicy:appLinkURL:]): >+ (-[WebFramePolicyListener invalidate]): >+ (-[WebFramePolicyListener dealloc]): >+ (-[WebFramePolicyListener receivedPolicyDecision:]): >+ (-[WebFramePolicyListener initWithFrame:policyFunction:defaultPolicy:]): Deleted. >+ (-[WebFramePolicyListener initWithFrame:policyFunction:defaultPolicy:appLinkURL:]): Deleted. >+ > 2019-01-31 Alex Christensen <achristensen@webkit.org> > > Revert r238819 which is unneeded and caused a performance regression. >Index: Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h >=================================================================== >--- Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h (revision 240901) >+++ Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h (working copy) >@@ -126,9 +126,9 @@ private: > WebCore::Frame* dispatchCreatePage(const WebCore::NavigationAction&) final; > void dispatchShow() final; > >- void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) final; >- void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::FramePolicyFunction&&) final; >- void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) final; >+ void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) final; > void cancelPolicyCheck() final; > > void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) final; >@@ -233,7 +233,7 @@ private: > > RemoteAXObjectRef accessibilityRemoteObject() final { return 0; } > >- RetainPtr<WebFramePolicyListener> setUpPolicyListener(WebCore::FramePolicyFunction&&, WebCore::PolicyAction defaultPolicy, NSURL *appLinkURL = nil); >+ RetainPtr<WebFramePolicyListener> setUpPolicyListener(WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&, WebCore::PolicyAction defaultPolicy, NSURL *appLinkURL = nil); > > NSDictionary *actionDictionary(const WebCore::NavigationAction&, WebCore::FormState*) const; > >Index: Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm >=================================================================== >--- Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm (revision 240901) >+++ Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm (working copy) >@@ -182,6 +182,7 @@ NSString *WebPluginContainerKey = @"WebP > > @interface WebFramePolicyListener : NSObject <WebPolicyDecisionListener, WebFormSubmissionListener> { > RefPtr<Frame> _frame; >+ PolicyCheckIdentifier _identifier; > FramePolicyFunction _policyFunction; > #if HAVE(APP_LINKS) > RetainPtr<NSURL> _appLinkURL; >@@ -189,9 +190,9 @@ @interface WebFramePolicyListener : NSOb > PolicyAction _defaultPolicy; > } > >-- (id)initWithFrame:(Frame*)frame policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy; >+- (id)initWithFrame:(Frame*)frame identifier:(PolicyCheckIdentifier)identifier policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy; > #if HAVE(APP_LINKS) >-- (id)initWithFrame:(Frame*)frame policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy appLinkURL:(NSURL *)url; >+- (id)initWithFrame:(Frame*)frame identifier:(PolicyCheckIdentifier)identifier policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy appLinkURL:(NSURL *)url; > #endif > > - (void)invalidate; >@@ -865,7 +866,7 @@ void WebFrameLoaderClient::dispatchShow( > [[webView _UIDelegateForwarder] webViewShow:webView]; > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceResponse& response, const ResourceRequest& request, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceResponse& response, const ResourceRequest& request, WebCore::PolicyCheckIdentifier identifier, FramePolicyFunction&& function) > { > WebView *webView = getWebView(m_webFrame.get()); > >@@ -873,7 +874,7 @@ void WebFrameLoaderClient::dispatchDecid > decidePolicyForMIMEType:response.mimeType() > request:request.nsURLRequest(HTTPBodyUpdatePolicy::UpdateHTTPBody) > frame:m_webFrame.get() >- decisionListener:setUpPolicyListener(WTFMove(function), PolicyAction::Use).get()]; >+ decisionListener:setUpPolicyListener(identifier, WTFMove(function), PolicyAction::Use).get()]; > } > > >@@ -896,7 +897,7 @@ static BOOL shouldTryAppLink(WebView *we > #endif > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& action, const ResourceRequest& request, FormState* formState, const String& frameName, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& action, const ResourceRequest& request, FormState* formState, const String& frameName, WebCore::PolicyCheckIdentifier identifier, FramePolicyFunction&& function) > { > WebView *webView = getWebView(m_webFrame.get()); > BOOL tryAppLink = shouldTryAppLink(webView, action, nullptr); >@@ -905,10 +906,10 @@ void WebFrameLoaderClient::dispatchDecid > decidePolicyForNewWindowAction:actionDictionary(action, formState) > request:request.nsURLRequest(HTTPBodyUpdatePolicy::UpdateHTTPBody) > newFrameName:frameName >- decisionListener:setUpPolicyListener(WTFMove(function), PolicyAction::Ignore, tryAppLink ? (NSURL *)request.url() : nil).get()]; >+ decisionListener:setUpPolicyListener(identifier, WTFMove(function), PolicyAction::Ignore, tryAppLink ? (NSURL *)request.url() : nil).get()]; > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, const ResourceResponse&, FormState* formState, PolicyDecisionMode, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, const ResourceResponse&, FormState* formState, PolicyDecisionMode, WebCore::PolicyCheckIdentifier identifier, FramePolicyFunction&& function) > { > WebView *webView = getWebView(m_webFrame.get()); > BOOL tryAppLink = shouldTryAppLink(webView, action, core(m_webFrame.get())); >@@ -917,7 +918,7 @@ void WebFrameLoaderClient::dispatchDecid > decidePolicyForNavigationAction:actionDictionary(action, formState) > request:request.nsURLRequest(HTTPBodyUpdatePolicy::UpdateHTTPBody) > frame:m_webFrame.get() >- decisionListener:setUpPolicyListener(WTFMove(function), PolicyAction::Ignore, tryAppLink ? (NSURL *)request.url() : nil).get()]; >+ decisionListener:setUpPolicyListener(identifier, WTFMove(function), PolicyAction::Ignore, tryAppLink ? (NSURL *)request.url() : nil).get()]; > } > > void WebFrameLoaderClient::cancelPolicyCheck() >@@ -965,7 +966,9 @@ void WebFrameLoaderClient::dispatchWillS > } > > NSDictionary *values = makeFormFieldValuesDictionary(formState); >- CallFormDelegate(getWebView(m_webFrame.get()), @selector(frame:sourceFrame:willSubmitForm:withValues:submissionListener:), m_webFrame.get(), kit(formState.sourceDocument().frame()), kit(&formState.form()), values, setUpPolicyListener([completionHandler = WTFMove(completionHandler)](PolicyAction) mutable { completionHandler(); }, PolicyAction::Ignore).get()); >+ CallFormDelegate(getWebView(m_webFrame.get()), @selector(frame:sourceFrame:willSubmitForm:withValues:submissionListener:), m_webFrame.get(), kit(formState.sourceDocument().frame()), kit(&formState.form()), values, setUpPolicyListener(PolicyCheckIdentifier { }, >+ [completionHandler = WTFMove(completionHandler)](PolicyAction, PolicyCheckIdentifier) mutable { completionHandler(); }, >+ PolicyAction::Ignore).get()); > } > > void WebFrameLoaderClient::revertToProvisionalState(DocumentLoader* loader) >@@ -1518,7 +1521,7 @@ void WebFrameLoaderClient::dispatchDidBe > { > } > >-RetainPtr<WebFramePolicyListener> WebFrameLoaderClient::setUpPolicyListener(FramePolicyFunction&& function, PolicyAction defaultPolicy, NSURL *appLinkURL) >+RetainPtr<WebFramePolicyListener> WebFrameLoaderClient::setUpPolicyListener(PolicyCheckIdentifier identifier, FramePolicyFunction&& function, PolicyAction defaultPolicy, NSURL *appLinkURL) > { > // FIXME: <rdar://5634381> We need to support multiple active policy listeners. > [m_policyListener invalidate]; >@@ -1526,10 +1529,10 @@ RetainPtr<WebFramePolicyListener> WebFra > RetainPtr<WebFramePolicyListener> policyListener; > #if HAVE(APP_LINKS) > if (appLinkURL) >- policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) policyFunction:WTFMove(function) defaultPolicy:defaultPolicy appLinkURL:appLinkURL]); >+ policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) identifier:identifier policyFunction:WTFMove(function) defaultPolicy:defaultPolicy appLinkURL:appLinkURL]); > else > #endif >- policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) policyFunction:WTFMove(function) defaultPolicy:defaultPolicy]); >+ policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) identifier:identifier policyFunction:WTFMove(function) defaultPolicy:defaultPolicy]); > > m_policyListener = policyListener.get(); > >@@ -2383,12 +2386,13 @@ + (void)initialize > #endif > } > >-- (id)initWithFrame:(Frame*)frame policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy >+- (id)initWithFrame:(Frame*)frame identifier:(PolicyCheckIdentifier)identifier policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy > { > self = [self init]; > if (!self) > return nil; > >+ _identifier = identifier; > _frame = frame; > _policyFunction = WTFMove(policyFunction); > _defaultPolicy = defaultPolicy; >@@ -2397,9 +2401,9 @@ - (id)initWithFrame:(Frame*)frame policy > } > > #if HAVE(APP_LINKS) >-- (id)initWithFrame:(Frame*)frame policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy appLinkURL:(NSURL *)appLinkURL >+- (id)initWithFrame:(Frame*)frame identifier:(PolicyCheckIdentifier)identifier policyFunction:(FramePolicyFunction&&)policyFunction defaultPolicy:(PolicyAction)defaultPolicy appLinkURL:(NSURL *)appLinkURL > { >- self = [self initWithFrame:frame policyFunction:WTFMove(policyFunction) defaultPolicy:defaultPolicy]; >+ self = [self initWithFrame:frame identifier:identifier policyFunction:WTFMove(policyFunction) defaultPolicy:defaultPolicy]; > if (!self) > return nil; > >@@ -2413,7 +2417,7 @@ - (void)invalidate > { > _frame = nullptr; > if (auto policyFunction = std::exchange(_policyFunction, nullptr)) >- policyFunction(PolicyAction::Ignore); >+ policyFunction(PolicyAction::Ignore, _identifier); > } > > - (void)dealloc >@@ -2426,7 +2430,7 @@ - (void)dealloc > _frame = nullptr; > if (auto policyFunction = std::exchange(_policyFunction, nullptr)) { > RELEASE_LOG_ERROR(Loading, "Client application failed to make a policy decision via WebPolicyDecisionListener, using defaultPolicy %hhu", _defaultPolicy); >- policyFunction(_defaultPolicy); >+ policyFunction(_defaultPolicy, _identifier); > } > > [super dealloc]; >@@ -2439,7 +2443,7 @@ - (void)receivedPolicyDecision:(PolicyAc > return; > > if (auto policyFunction = std::exchange(_policyFunction, nullptr)) >- policyFunction(action); >+ policyFunction(action, _identifier); > } > > - (void)ignore >Index: Source/WebKitLegacy/win/ChangeLog >=================================================================== >--- Source/WebKitLegacy/win/ChangeLog (revision 240901) >+++ Source/WebKitLegacy/win/ChangeLog (working copy) >@@ -1,3 +1,21 @@ >+2019-02-02 Ryosuke Niwa <rniwa@webkit.org> >+ >+ Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy >+ https://bugs.webkit.org/show_bug.cgi?id=194189 >+ >+ Reviewed by Geoffrey Garen. >+ >+ Pass the policy check identifier around functions and store it in WebFramePolicyListener >+ so that we can send it back to WebCore with the navigation policy decision. >+ >+ * WebCoreSupport/WebFrameLoaderClient.cpp: >+ (WebFrameLoaderClient::dispatchDecidePolicyForResponse): >+ (WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction): >+ (WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): >+ (WebFrameLoaderClient::dispatchWillSubmitForm): >+ (WebFrameLoaderClient::setUpPolicyListener): >+ * WebCoreSupport/WebFrameLoaderClient.h: >+ > 2019-01-31 Alex Christensen <achristensen@webkit.org> > > Revert r238819 which is unneeded and caused a performance regression. >Index: Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp >=================================================================== >--- Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp (revision 240901) >+++ Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp (working copy) >@@ -101,6 +101,7 @@ public: > > ~WebFramePolicyListenerPrivate() { } > >+ PolicyCheckIdentifier m_policyCheckIdentifier; > FramePolicyFunction m_policyFunction; > COMPtr<WebFramePolicyListener> m_policyListener; > }; >@@ -528,7 +529,7 @@ void WebFrameLoaderClient::dispatchShow( > ui->webViewShow(webView); > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceResponse& response, const ResourceRequest& request, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceResponse& response, const ResourceRequest& request, WebCore::PolicyCheckIdentifier identifier, FramePolicyFunction&& function) > { > WebView* webView = m_webFrame->webView(); > Frame* coreFrame = core(m_webFrame); >@@ -540,13 +541,13 @@ void WebFrameLoaderClient::dispatchDecid > > COMPtr<IWebURLRequest> urlRequest(AdoptCOM, WebMutableURLRequest::createInstance(request)); > >- if (SUCCEEDED(policyDelegate->decidePolicyForMIMEType(webView, BString(response.mimeType()), urlRequest.get(), m_webFrame, setUpPolicyListener(WTFMove(function)).get()))) >+ if (SUCCEEDED(policyDelegate->decidePolicyForMIMEType(webView, BString(response.mimeType()), urlRequest.get(), m_webFrame, setUpPolicyListener(identifier, WTFMove(function)).get()))) > return; > >- m_policyListenerPrivate->m_policyFunction(PolicyAction::Use); >+ m_policyListenerPrivate->m_policyFunction(identifier, PolicyAction::Use); > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& action, const ResourceRequest& request, FormState* formState, const String& frameName, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& action, const ResourceRequest& request, FormState* formState, const String& frameName, WebCore::PolicyCheckIdentifier, FramePolicyFunction&& function) > { > WebView* webView = m_webFrame->webView(); > Frame* coreFrame = core(m_webFrame); >@@ -559,13 +560,13 @@ void WebFrameLoaderClient::dispatchDecid > COMPtr<IWebURLRequest> urlRequest(AdoptCOM, WebMutableURLRequest::createInstance(request)); > COMPtr<WebActionPropertyBag> actionInformation(AdoptCOM, WebActionPropertyBag::createInstance(action, formState ? &formState->form() : nullptr, coreFrame)); > >- if (SUCCEEDED(policyDelegate->decidePolicyForNewWindowAction(webView, actionInformation.get(), urlRequest.get(), BString(frameName), setUpPolicyListener(WTFMove(function)).get()))) >+ if (SUCCEEDED(policyDelegate->decidePolicyForNewWindowAction(webView, actionInformation.get(), urlRequest.get(), BString(frameName), setUpPolicyListener(identifier, WTFMove(function)).get()))) > return; > >- m_policyListenerPrivate->m_policyFunction(PolicyAction::Use); >+ m_policyListenerPrivate->m_policyFunction(identifier, PolicyAction::Use); > } > >-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, const ResourceResponse&, FormState* formState, WebCore::PolicyDecisionMode, FramePolicyFunction&& function) >+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, const ResourceResponse&, FormState* formState, WebCore::PolicyDecisionMode, WebCore::PolicyCheckIdentifier, FramePolicyFunction&& function) > { > WebView* webView = m_webFrame->webView(); > Frame* coreFrame = core(m_webFrame); >@@ -578,10 +579,10 @@ void WebFrameLoaderClient::dispatchDecid > COMPtr<IWebURLRequest> urlRequest(AdoptCOM, WebMutableURLRequest::createInstance(request)); > COMPtr<WebActionPropertyBag> actionInformation(AdoptCOM, WebActionPropertyBag::createInstance(action, formState ? &formState->form() : nullptr, coreFrame)); > >- if (SUCCEEDED(policyDelegate->decidePolicyForNavigationAction(webView, actionInformation.get(), urlRequest.get(), m_webFrame, setUpPolicyListener(WTFMove(function)).get()))) >+ if (SUCCEEDED(policyDelegate->decidePolicyForNavigationAction(webView, actionInformation.get(), urlRequest.get(), m_webFrame, setUpPolicyListener(identifier, WTFMove(function)).get()))) > return; > >- m_policyListenerPrivate->m_policyFunction(PolicyAction::Use); >+ m_policyListenerPrivate->m_policyFunction(identifier, PolicyAction::Use); > } > > void WebFrameLoaderClient::dispatchUnableToImplementPolicy(const ResourceError& error) >@@ -623,7 +624,8 @@ void WebFrameLoaderClient::dispatchWillS > COMPtr<IPropertyBag> formValuesPropertyBag(AdoptCOM, COMPropertyBag<String>::createInstance(formValuesMap)); > > COMPtr<WebFrame> sourceFrame(kit(formState.sourceDocument().frame())); >- if (SUCCEEDED(formDelegate->willSubmitForm(m_webFrame, sourceFrame.get(), formElement.get(), formValuesPropertyBag.get(), setUpPolicyListener([completionHandler = WTFMove(completionHandler)] (PolicyAction) mutable { completionHandler(); }).get()))) >+ if (SUCCEEDED(formDelegate->willSubmitForm(m_webFrame, sourceFrame.get(), formElement.get(), formValuesPropertyBag.get(), >+ setUpPolicyListener(PolicyCheckIdentifier { }, [completionHandler = WTFMove(completionHandler)] (PolicyAction, PolicyCheckIdentifier) mutable { completionHandler(); }).get()))) > return; > > // FIXME: Add a sane default implementation >@@ -1282,7 +1284,7 @@ void WebFrameLoaderClient::cancelPolicyC > m_policyListenerPrivate->m_policyFunction = nullptr; > } > >-COMPtr<WebFramePolicyListener> WebFrameLoaderClient::setUpPolicyListener(WebCore::FramePolicyFunction&& function) >+COMPtr<WebFramePolicyListener> WebFrameLoaderClient::setUpPolicyListener(WebCore::PolicyCheckIdentifier identifier, WebCore::FramePolicyFunction&& function) > { > // FIXME: <rdar://5634381> We need to support multiple active policy listeners. > >@@ -1293,6 +1295,7 @@ COMPtr<WebFramePolicyListener> WebFrameL > ASSERT(coreFrame); > > m_policyListenerPrivate->m_policyListener.adoptRef(WebFramePolicyListener::createInstance(coreFrame)); >+ m_policyListenerPrivate->m_policyCheckIdentifier = identifier; > m_policyListenerPrivate->m_policyFunction = WTFMove(function); > > return m_policyListenerPrivate->m_policyListener; >Index: Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h >=================================================================== >--- Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h (revision 240901) >+++ Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h (working copy) >@@ -100,9 +100,9 @@ public: > void dispatchDidFinishLoad() override; > void dispatchDidReachLayoutMilestone(OptionSet<WebCore::LayoutMilestone>) override; > >- void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) override; >- void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::FramePolicyFunction&&) override; >- void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) override; >+ void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) override; >+ void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) override; >+ void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&) override; > void cancelPolicyCheck() override; > > void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) override; >@@ -197,7 +197,7 @@ public: > > void dispatchDidClearWindowObjectInWorld(WebCore::DOMWrapperWorld&) override; > >- COMPtr<WebFramePolicyListener> setUpPolicyListener(WebCore::FramePolicyFunction&&); >+ COMPtr<WebFramePolicyListener> setUpPolicyListener(WebCore::PolicyCheckIdentifier, WebCore::FramePolicyFunction&&); > void receivedPolicyDecision(WebCore::PolicyAction); > > bool shouldAlwaysUsePluginDocument(const WTF::String& mimeType) const override;
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194189
:
360967
|
360970
|
360993
|
360997
|
360998