Changeset 214201 in webkit
- Timestamp:
- Mar 20, 2017, 6:13:50 PM (8 years ago)
- Location:
- trunk
- Files:
-
- 8 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit2/ChangeLog
r214197 r214201 1 2017-03-20 Alex Christensen <achristensen@webkit.org> 2 3 WebPageProxy DecidePolicyForNavigationAction and DecidePolicyForResponseSync should be Delayed reply messages 4 https://bugs.webkit.org/show_bug.cgi?id=167183 5 <rdar://problem/30203539> 6 7 Reviewed by Andy Estes. 8 9 Before this patch, the WKNavigationDelegate's decidePolicyForNavigationAction must synchronously call the decisionHandler. 10 If it stores the decisionHandler and calls it after decidePolicyForNavigationAction returns, we can get incorrect behavior. 11 This can be seen when the _WKWebsitePolicies given to the decisionHandler had no effect. 12 Now, we will have the WebProcess waiting on the UIProcess to respond to the Delayed reply before continuing. 13 This will not be a regression because currently everybody is either calling the decisionHandler immediately or getting incorrect behavior, 14 and the behavior will be the same if the decisionHandler is called immediately. It is possible that we could make the WebProcess 15 not wait on the response, but we would need to make WebCore's loading truly asynchronous first 16 (getting rid of ResourceHandleClient's synchronous methods). 17 18 Covered by making an API test asynchronously call the decisionHandler. 19 20 * UIProcess/API/C/WKPage.cpp: 21 (WKPageSetPageNavigationClient): 22 * UIProcess/WebPageProxy.cpp: 23 (WebKit::WebPageProxy::receivedPolicyDecision): 24 (WebKit::WebPageProxy::decidePolicyForNavigationAction): 25 (WebKit::WebPageProxy::decidePolicyForResponseSync): 26 * UIProcess/WebPageProxy.h: 27 * UIProcess/WebPageProxy.messages.in: 28 * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp: 29 (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse): 30 (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): 31 1 32 2017-03-20 Alex Christensen <achristensen@webkit.org> 2 33 -
trunk/Source/WebKit2/UIProcess/API/C/WKPage.cpp
r214113 r214201 2266 2266 void decidePolicyForNavigationAction(WebPageProxy& page, API::NavigationAction& navigationAction, Ref<WebKit::WebFramePolicyListenerProxy>&& listener, API::Object* userData) override 2267 2267 { 2268 if (!m_client.decidePolicyForNavigationAction) 2269 return; 2268 if (!m_client.decidePolicyForNavigationAction) { 2269 listener->use({ }); 2270 return; 2271 } 2270 2272 m_client.decidePolicyForNavigationAction(toAPI(&page), toAPI(&navigationAction), toAPI(listener.ptr()), toAPI(userData), m_client.base.clientInfo); 2271 2273 } … … 2273 2275 void decidePolicyForNavigationResponse(WebPageProxy& page, API::NavigationResponse& navigationResponse, Ref<WebKit::WebFramePolicyListenerProxy>&& listener, API::Object* userData) override 2274 2276 { 2275 if (!m_client.decidePolicyForNavigationResponse) 2276 return; 2277 if (!m_client.decidePolicyForNavigationResponse) { 2278 listener->use({ }); 2279 return; 2280 } 2277 2281 m_client.decidePolicyForNavigationResponse(toAPI(&page), toAPI(&navigationResponse), toAPI(listener.ptr()), toAPI(userData), m_client.base.clientInfo); 2278 2282 } -
trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp
r214113 r214201 385 385 , m_isInPrintingMode(false) 386 386 , m_isPerformingDOMPrintOperation(false) 387 , m_inDecidePolicyForResponseSync(false)388 , m_decidePolicyForResponseRequest(0)389 , m_syncMimeTypePolicyActionIsValid(false)390 , m_syncMimeTypePolicyAction(PolicyUse)391 , m_syncMimeTypePolicyDownloadID(0)392 , m_inDecidePolicyForNavigationAction(false)393 , m_syncNavigationActionPolicyActionIsValid(false)394 , m_syncNavigationActionPolicyAction(PolicyUse)395 , m_syncNavigationActionPolicyDownloadID(0)396 387 , m_processingMouseMoveEvent(false) 397 388 , m_pageID(pageID) … … 2299 2290 if (action == PolicyDownload) { 2300 2291 // Create a download proxy. 2301 // FIXME: We should ensure that the downloadRequest is never empty. 2302 const ResourceRequest& downloadRequest = m_decidePolicyForResponseRequest ? *m_decidePolicyForResponseRequest : ResourceRequest(); 2303 DownloadProxy* download = m_process->processPool().createDownloadProxy(downloadRequest); 2292 DownloadProxy* download = m_process->processPool().createDownloadProxy(m_decidePolicyForResponseRequest); 2304 2293 downloadID = download->downloadID(); 2305 2294 handleDownloadRequest(download); 2295 m_decidePolicyForResponseRequest = { }; 2306 2296 } 2307 2297 2308 2298 // If we received a policy decision while in decidePolicyForResponse the decision will 2309 2299 // be sent back to the web process by decidePolicyForResponse. 2310 if (m_ inDecidePolicyForResponseSync) {2311 m_ syncMimeTypePolicyActionIsValid = true;2312 m_syncMimeTypePolicyAction = action;2313 m_ syncMimeTypePolicyDownloadID = downloadID;2300 if (m_responsePolicyReply) { 2301 m_responsePolicyReply->send(action, downloadID); 2302 ASSERT(!m_newNavigationID); 2303 m_responsePolicyReply = nullptr; 2314 2304 return; 2315 2305 } … … 2317 2307 // If we received a policy decision while in decidePolicyForNavigationAction the decision will 2318 2308 // be sent back to the web process by decidePolicyForNavigationAction. 2319 if (m_inDecidePolicyForNavigationAction) { 2320 m_syncNavigationActionPolicyActionIsValid = true; 2321 m_syncNavigationActionPolicyAction = action; 2322 m_syncNavigationActionPolicyDownloadID = downloadID; 2323 m_syncNavigationActionPolicyWebsitePolicies = websitePolicies; 2309 if (m_navigationActionPolicyReply) { 2310 m_navigationActionPolicyReply->send(m_newNavigationID, action, downloadID, websitePolicies); 2311 m_newNavigationID = 0; 2312 m_navigationActionPolicyReply = nullptr; 2324 2313 return; 2325 2314 } … … 3649 3638 } 3650 3639 3651 void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const NavigationActionData& navigationActionData, uint64_t originatingFrameID, const SecurityOriginData& originatingFrameSecurityOrigin, const WebCore::ResourceRequest& originalRequest, const ResourceRequest& request, uint64_t listenerID, const UserData& userData, bool& receivedPolicyAction, uint64_t& newNavigationID, uint64_t& policyAction, DownloadID& downloadID, WebsitePolicies& websitePolicies)3640 void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const NavigationActionData& navigationActionData, uint64_t originatingFrameID, const SecurityOriginData& originatingFrameSecurityOrigin, const WebCore::ResourceRequest& originalRequest, const ResourceRequest& request, uint64_t listenerID, const UserData& userData, Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&& reply) 3652 3641 { 3653 3642 PageClientProtector protector(m_pageClient); … … 3665 3654 WebFrameProxy* originatingFrame = m_process->webFrame(originatingFrameID); 3666 3655 3656 m_newNavigationID = 0; 3667 3657 Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID); 3668 3658 if (!navigationID && frame->isMainFrame()) { 3669 3659 auto navigation = m_navigationState->createLoadRequestNavigation(request); 3670 newNavigationID = navigation->navigationID();3660 m_newNavigationID = navigation->navigationID(); 3671 3661 listener->setNavigation(WTFMove(navigation)); 3672 3662 } … … 3674 3664 #if ENABLE(CONTENT_FILTERING) 3675 3665 if (frame->didHandleContentFilterUnblockNavigation(request)) { 3676 receivedPolicyAction = true; 3677 policyAction = PolicyIgnore; 3678 return; 3679 } 3680 #endif 3681 3682 ASSERT(!m_inDecidePolicyForNavigationAction); 3683 3684 m_inDecidePolicyForNavigationAction = true; 3685 m_syncNavigationActionPolicyActionIsValid = false; 3666 reply->send(m_newNavigationID, PolicyIgnore, { }, { }); 3667 m_newNavigationID = 0; 3668 return; 3669 } 3670 #endif 3671 3686 3672 #if ENABLE(DOWNLOAD_ATTRIBUTE) 3687 3673 m_syncNavigationActionHasDownloadAttribute = !navigationActionData.downloadAttribute.isNull(); 3688 3674 #endif 3675 m_navigationActionPolicyReply = WTFMove(reply); 3689 3676 3690 3677 if (m_navigationClient) { … … 3710 3697 3711 3698 m_shouldSuppressAppLinksInNextNavigationPolicyDecision = false; 3712 m_inDecidePolicyForNavigationAction = false;3713 3714 // Check if we received a policy decision already. If we did, we can just pass it back.3715 receivedPolicyAction = m_syncNavigationActionPolicyActionIsValid;3716 if (m_syncNavigationActionPolicyActionIsValid) {3717 policyAction = m_syncNavigationActionPolicyAction;3718 downloadID = m_syncNavigationActionPolicyDownloadID;3719 websitePolicies = m_syncNavigationActionPolicyWebsitePolicies;3720 }3721 3699 } 3722 3700 … … 3764 3742 } 3765 3743 3766 void WebPageProxy::decidePolicyForResponseSync(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData, bool& receivedPolicyAction, uint64_t& policyAction, DownloadID& downloadID)3744 void WebPageProxy::decidePolicyForResponseSync(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData, Ref<Messages::WebPageProxy::DecidePolicyForResponseSync::DelayedReply>&& reply) 3767 3745 { 3768 3746 PageClientProtector protector(m_pageClient); 3769 3747 3770 ASSERT(!m_inDecidePolicyForResponseSync); 3771 3772 m_inDecidePolicyForResponseSync = true; 3773 m_decidePolicyForResponseRequest = &request; 3774 m_syncMimeTypePolicyActionIsValid = false; 3748 m_decidePolicyForResponseRequest = request; 3749 m_responsePolicyReply = WTFMove(reply); 3775 3750 3776 3751 decidePolicyForResponse(frameID, frameSecurityOrigin, response, request, canShowMIMEType, listenerID, userData); 3777 3778 m_inDecidePolicyForResponseSync = false;3779 m_decidePolicyForResponseRequest = 0;3780 3781 // Check if we received a policy decision already. If we did, we can just pass it back.3782 receivedPolicyAction = m_syncMimeTypePolicyActionIsValid;3783 if (m_syncMimeTypePolicyActionIsValid) {3784 policyAction = m_syncMimeTypePolicyAction;3785 downloadID = m_syncMimeTypePolicyDownloadID;3786 }3787 3752 } 3788 3753 -
trunk/Source/WebKit2/UIProcess/WebPageProxy.h
r214113 r214201 1262 1262 void didDestroyNavigation(uint64_t navigationID); 1263 1263 1264 void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const NavigationActionData&, uint64_t originatingFrameID, const WebCore::SecurityOriginData& originatingFrameSecurityOrigin, const WebCore::ResourceRequest& originalRequest, const WebCore::ResourceRequest&, uint64_t listenerID, const UserData&, bool& receivedPolicyAction, uint64_t& newNavigationID, uint64_t& policyAction, DownloadID&, WebsitePolicies&);1264 void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const NavigationActionData&, uint64_t originatingFrameID, const WebCore::SecurityOriginData& originatingFrameSecurityOrigin, const WebCore::ResourceRequest& originalRequest, const WebCore::ResourceRequest&, uint64_t listenerID, const UserData&, Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&&); 1265 1265 void decidePolicyForNewWindowAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, const NavigationActionData&, const WebCore::ResourceRequest&, const String& frameName, uint64_t listenerID, const UserData&); 1266 1266 void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&); 1267 void decidePolicyForResponseSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&, bool& receivedPolicyAction, uint64_t& policyAction, DownloadID&);1267 void decidePolicyForResponseSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&, Ref<Messages::WebPageProxy::DecidePolicyForResponseSync::DelayedReply>&&); 1268 1268 void unableToImplementPolicy(uint64_t frameID, const WebCore::ResourceError&, const UserData&); 1269 1269 … … 1776 1776 bool m_isPerformingDOMPrintOperation; 1777 1777 1778 bool m_inDecidePolicyForResponseSync; 1779 const WebCore::ResourceRequest* m_decidePolicyForResponseRequest; 1780 bool m_syncMimeTypePolicyActionIsValid; 1781 WebCore::PolicyAction m_syncMimeTypePolicyAction; 1782 DownloadID m_syncMimeTypePolicyDownloadID; 1783 1784 bool m_inDecidePolicyForNavigationAction; 1785 bool m_syncNavigationActionPolicyActionIsValid; 1786 WebCore::PolicyAction m_syncNavigationActionPolicyAction; 1787 DownloadID m_syncNavigationActionPolicyDownloadID; 1788 WebsitePolicies m_syncNavigationActionPolicyWebsitePolicies; 1778 RefPtr<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply> m_navigationActionPolicyReply; 1779 uint64_t m_newNavigationID { 0 }; 1780 RefPtr<Messages::WebPageProxy::DecidePolicyForResponseSync::DelayedReply> m_responsePolicyReply; 1781 WebCore::ResourceRequest m_decidePolicyForResponseRequest; 1782 1789 1783 bool m_shouldSuppressAppLinksInNextNavigationPolicyDecision { false }; 1790 1784 -
trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in
r213903 r214201 101 101 102 102 # Policy messages 103 DecidePolicyForResponseSync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, uint64_t listenerID, WebKit::UserData userData) -> ( bool receivedPolicyAction, uint64_t policyAction, WebKit::DownloadID downloadID)104 DecidePolicyForNavigationAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, uint64_t originatingFrameID, struct WebCore::SecurityOriginData originatingFrameSecurityOrigin, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, uint64_t listenerID, WebKit::UserData userData) -> ( bool receivedPolicyAction, uint64_t newNavigationID, uint64_t policyAction, WebKit::DownloadID downloadID, struct WebKit::WebsitePolicies websitePolicies)103 DecidePolicyForResponseSync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, uint64_t listenerID, WebKit::UserData userData) -> (uint64_t policyAction, WebKit::DownloadID downloadID) Delayed 104 DecidePolicyForNavigationAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, uint64_t originatingFrameID, struct WebCore::SecurityOriginData originatingFrameSecurityOrigin, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, uint64_t listenerID, WebKit::UserData userData) -> (uint64_t newNavigationID, uint64_t policyAction, WebKit::DownloadID downloadID, struct WebKit::WebsitePolicies websitePolicies) Delayed 105 105 DecidePolicyForNewWindowAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, struct WebKit::NavigationActionData navigationActionData, WebCore::ResourceRequest request, String frameName, uint64_t listenerID, WebKit::UserData userData) 106 106 UnableToImplementPolicy(uint64_t frameID, WebCore::ResourceError error, WebKit::UserData userData) -
trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
r213656 r214201 686 686 687 687 uint64_t listenerID = m_frame->setUpPolicyListener(WTFMove(function)); 688 bool receivedPolicyAction;689 688 uint64_t policyAction; 690 689 DownloadID downloadID; … … 692 691 Ref<WebFrame> protect(*m_frame); 693 692 WebCore::Frame* coreFrame = m_frame->coreFrame(); 694 if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForResponseSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), response, request, canShowMIMEType, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForResponseSync::Reply( receivedPolicyAction,policyAction, downloadID), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend)) {693 if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForResponseSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), response, request, canShowMIMEType, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForResponseSync::Reply(policyAction, downloadID), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend)) { 695 694 m_frame->didReceivePolicyDecision(listenerID, PolicyIgnore, 0, { }); 696 695 return; … … 698 697 699 698 // We call this synchronously because CFNetwork can only convert a loading connection to a download from its didReceiveResponse callback. 700 if (receivedPolicyAction) 701 m_frame->didReceivePolicyDecision(listenerID, static_cast<PolicyAction>(policyAction), 0, downloadID); 699 m_frame->didReceivePolicyDecision(listenerID, static_cast<PolicyAction>(policyAction), 0, downloadID); 702 700 } 703 701 … … 764 762 765 763 uint64_t listenerID = m_frame->setUpPolicyListener(WTFMove(function)); 766 bool receivedPolicyAction;767 764 uint64_t newNavigationID; 768 765 uint64_t policyAction; … … 809 806 WebCore::Frame* originatingCoreFrame = originatingFrame ? originatingFrame->coreFrame() : nullptr; 810 807 WebsitePolicies websitePolicies; 811 if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrame ? originatingFrame->frameID() : 0, SecurityOriginData::fromFrame(originatingCoreFrame), navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationAction::Reply( receivedPolicyAction,newNavigationID, policyAction, downloadID, websitePolicies))) {808 if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrame ? originatingFrame->frameID() : 0, SecurityOriginData::fromFrame(originatingCoreFrame), navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationAction::Reply(newNavigationID, policyAction, downloadID, websitePolicies))) { 812 809 m_frame->didReceivePolicyDecision(listenerID, PolicyIgnore, 0, { }); 813 810 return; … … 836 833 837 834 // We call this synchronously because WebCore cannot gracefully handle a frame load without a synchronous navigation policy reply. 838 if (receivedPolicyAction) 839 m_frame->didReceivePolicyDecision(listenerID, static_cast<PolicyAction>(policyAction), newNavigationID, downloadID); 835 m_frame->didReceivePolicyDecision(listenerID, static_cast<PolicyAction>(policyAction), newNavigationID, downloadID); 840 836 } 841 837 -
trunk/Tools/ChangeLog
r214192 r214201 1 2017-03-20 Alex Christensen <achristensen@webkit.org> 2 3 WebPageProxy DecidePolicyForNavigationAction and DecidePolicyForResponseSync should be Delayed reply messages 4 https://bugs.webkit.org/show_bug.cgi?id=167183 5 <rdar://problem/30203539> 6 7 Reviewed by Andy Estes. 8 9 * TestWebKitAPI/Tests/WebKit2Cocoa/WebsitePolicies.mm: 10 (-[WebsitePoliciesDelegate _webView:decidePolicyForNavigationAction:decisionHandler:]): 11 1 12 2017-03-20 Jonathan Bedard <jbedard@apple.com> 2 13 -
trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WebsitePolicies.mm
r213656 r214201 34 34 #import <WebKit/_WKUserContentExtensionStorePrivate.h> 35 35 #import <WebKit/_WKWebsitePolicies.h> 36 #import <wtf/MainThread.h> 36 37 #import <wtf/RetainPtr.h> 37 38 … … 108 109 break; 109 110 case 2: 110 // Verify disabling content blockers works correctly. 111 websitePolicies.contentBlockersEnabled = false; 112 break; 111 { 112 // Verify disabling content blockers works correctly. 113 websitePolicies.contentBlockersEnabled = false; 114 115 // Verify calling the decisionHandler asynchronously works correctly. 116 auto decisionHandlerCopy = Block_copy(decisionHandler); 117 callOnMainThread([decisionHandlerCopy, websitePolicies = RetainPtr<_WKWebsitePolicies>(websitePolicies)] { 118 decisionHandlerCopy(WKNavigationActionPolicyAllow, websitePolicies.get()); 119 Block_release(decisionHandlerCopy); 120 }); 121 } 122 return; 113 123 case 3: 114 124 // Verify enabling content blockers has no effect when reloading without content blockers.
Note:
See TracChangeset
for help on using the changeset viewer.