Attachment 333617 Details for Bug 182697 – Rebased

Rebased

180568_policyDelegates_async_wip.patch (text/plain), 18.82 KB, created by Chris Dumez on 2018-02-12 12:43:11 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-02-12 12:43:11 PST

Size: 18.82 KB

patch

obsolete

>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 3838ed0bd42..345388b7edf 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-02-12  Alex Christensen  <achristensen@webkit.org>
>+
>+        Make policy decisions asynchronous
>+        https://bugs.webkit.org/show_bug.cgi?id=180568
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::receivedPolicyDecision):
>+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
>+        (WebKit::WebPageProxy::decidePolicyForResponse):
>+        (WebKit::WebPageProxy::decidePolicyForResponseSync): Deleted.
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/WebPageProxy.messages.in:
>+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
>+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
>+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
>+
> 2018-02-11  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         [GTK] Unify builds for platform-specific files in WebKit layer
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 3b66d42be7a..79bafc34d66 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -2303,8 +2303,7 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
>     DownloadID downloadID = { };
>     if (action == PolicyAction::Download) {
>         // Create a download proxy.
>-        const ResourceRequest& downloadRequest = m_decidePolicyForResponseRequest ? *m_decidePolicyForResponseRequest : ResourceRequest();
>-        DownloadProxy* download = m_process->processPool().createDownloadProxy(downloadRequest, this);
>+        auto* download = m_process->processPool().createDownloadProxy(m_decidePolicyForResponseRequest, this);
>         if (navigation) {
>             download->setWasUserInitiated(navigation->wasUserInitiated());
>             download->setRedirectChain(navigation->takeRedirectChain());
>@@ -2312,27 +2311,9 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
> 
>         downloadID = download->downloadID();
>         handleDownloadRequest(download);
>+        m_decidePolicyForResponseRequest = { };
>     }
> 
>-    // If we received a policy decision while in decidePolicyForResponse the decision will
>-    // be sent back to the web process by decidePolicyForResponse.
>-    if (m_inDecidePolicyForResponseSync) {
>-        m_syncMimeTypePolicyActionIsValid = true;
>-        m_syncMimeTypePolicyAction = action;
>-        m_syncMimeTypePolicyDownloadID = downloadID;
>-        return;
>-    }
>-
>-    // If we received a policy decision while in decidePolicyForNavigationAction the decision will 
>-    // be sent back to the web process by decidePolicyForNavigationAction. 
>-    if (m_inDecidePolicyForNavigationAction) {
>-        m_syncNavigationActionPolicyActionIsValid = true;
>-        m_syncNavigationActionPolicyAction = action;
>-        m_syncNavigationActionPolicyDownloadID = downloadID;
>-        m_syncNavigationActionPolicyWebsitePolicies = WTFMove(websitePolicies);
>-        return;
>-    }
>-    
>     m_process->send(Messages::WebPage::DidReceivePolicyDecision(frame.frameID(), listenerID, action, navigation ? navigation->navigationID() : 0, downloadID, websitePolicies), m_pageID);
> }
> 
>@@ -3721,7 +3702,7 @@ void WebPageProxy::frameDidBecomeFrameSet(uint64_t frameID, bool value)
>         m_frameSetLargestFrame = value ? m_mainFrame : 0;
> }
> 
>-void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, ResourceRequest&& request, uint64_t listenerID, const UserData& userData, bool& receivedPolicyAction, uint64_t& newNavigationID, WebCore::PolicyAction& policyAction, DownloadID& downloadID, std::optional<WebsitePoliciesData>& websitePolicies)
>+void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, ResourceRequest&& request, uint64_t listenerID, const UserData& userData)
> {
>     PageClientProtector protector(m_pageClient);
> 
>@@ -3736,6 +3717,7 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
>     MESSAGE_CHECK_URL(request.url());
>     MESSAGE_CHECK_URL(originalRequest.url());
>     
>+    uint64_t newNavigationID { 0 };
>     Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID);
>     if (!navigationID) {
>         auto navigation = m_navigationState->createLoadRequestNavigation(ResourceRequest(request));
>@@ -3745,24 +3727,17 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
>         listener->setNavigation(WTFMove(navigation));
>     } else {
>         auto& navigation = m_navigationState->navigation(navigationID);
>+        newNavigationID = navigationID;
>         navigation.setWasUserInitiated(!!navigationActionData.userGestureTokenIdentifier);
>         navigation.setShouldForceDownload(!navigationActionData.downloadAttribute.isNull());
>         listener->setNavigation(navigation);
>     }
> 
> #if ENABLE(CONTENT_FILTERING)
>-    if (frame->didHandleContentFilterUnblockNavigation(request)) {
>-        receivedPolicyAction = true;
>-        policyAction = PolicyAction::Ignore;
>-        return;
>-    }
>+    if (frame->didHandleContentFilterUnblockNavigation(request))
>+        return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, &m_navigationState->navigation(newNavigationID), { });
> #endif
> 
>-    ASSERT(!m_inDecidePolicyForNavigationAction);
>-    
>-    m_inDecidePolicyForNavigationAction = true;
>-    m_syncNavigationActionPolicyActionIsValid = false;
>-
>     WebFrameProxy* originatingFrame = m_process->webFrame(originatingFrameInfoData.frameID);
> 
>     if (m_navigationClient) {
>@@ -3783,15 +3758,6 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
>         m_policyClient->decidePolicyForNavigationAction(*this, frame, WTFMove(navigationActionData), originatingFrame, originalRequest, WTFMove(request), WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
> 
>     m_shouldSuppressAppLinksInNextNavigationPolicyDecision = false;
>-    m_inDecidePolicyForNavigationAction = false;
>-
>-    // Check if we received a policy decision already. If we did, we can just pass it back.
>-    receivedPolicyAction = m_syncNavigationActionPolicyActionIsValid;
>-    if (m_syncNavigationActionPolicyActionIsValid) {
>-        policyAction = m_syncNavigationActionPolicyAction;
>-        downloadID = m_syncNavigationActionPolicyDownloadID;
>-        websitePolicies = WTFMove(m_syncNavigationActionPolicyWebsitePolicies);
>-    }
> }
> 
> void WebPageProxy::decidePolicyForNewWindowAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, NavigationActionData&& navigationActionData, ResourceRequest&& request, const String& frameName, uint64_t listenerID, const UserData& userData)
>@@ -3823,6 +3789,8 @@ void WebPageProxy::decidePolicyForResponse(uint64_t frameID, const SecurityOrigi
> {
>     PageClientProtector protector(m_pageClient);
> 
>+    m_decidePolicyForResponseRequest = request;
>+
>     WebFrameProxy* frame = m_process->webFrame(frameID);
>     MESSAGE_CHECK(frame);
>     MESSAGE_CHECK_URL(request.url());
>@@ -3839,29 +3807,6 @@ void WebPageProxy::decidePolicyForResponse(uint64_t frameID, const SecurityOrigi
>         m_policyClient->decidePolicyForResponse(*this, *frame, response, request, canShowMIMEType, WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
> }
> 
>-void WebPageProxy::decidePolicyForResponseSync(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const ResourceResponse& response, const ResourceRequest& request, bool canShowMIMEType, uint64_t listenerID, const UserData& userData, bool& receivedPolicyAction, WebCore::PolicyAction& policyAction, DownloadID& downloadID)
>-{
>-    PageClientProtector protector(m_pageClient);
>-
>-    ASSERT(!m_inDecidePolicyForResponseSync);
>-
>-    m_inDecidePolicyForResponseSync = true;
>-    m_decidePolicyForResponseRequest = &request;
>-    m_syncMimeTypePolicyActionIsValid = false;
>-    
>-    decidePolicyForResponse(frameID, frameSecurityOrigin, navigationID, response, request, canShowMIMEType, listenerID, userData);
>-    
>-    m_inDecidePolicyForResponseSync = false;
>-    m_decidePolicyForResponseRequest = nullptr;
>-    
>-    // Check if we received a policy decision already. If we did, we can just pass it back.
>-    receivedPolicyAction = m_syncMimeTypePolicyActionIsValid;
>-    if (m_syncMimeTypePolicyActionIsValid) {
>-        policyAction = m_syncMimeTypePolicyAction;
>-        downloadID = m_syncMimeTypePolicyDownloadID;
>-    }
>-}
>-
> void WebPageProxy::unableToImplementPolicy(uint64_t frameID, const ResourceError& error, const UserData& userData)
> {
>     PageClientProtector protector(m_pageClient);
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index cff18f57a6b..c5fdc9a0410 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1350,10 +1350,9 @@ private:
> 
>     void didDestroyNavigation(uint64_t navigationID);
> 
>-    void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, uint64_t listenerID, const UserData&, bool& receivedPolicyAction, uint64_t& newNavigationID, WebCore::PolicyAction&, DownloadID&, std::optional<WebsitePoliciesData>&);
>+    void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, uint64_t listenerID, const UserData&);
>     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 decidePolicyForResponseSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&, bool& receivedPolicyAction, WebCore::PolicyAction&, DownloadID&);
>     void unableToImplementPolicy(uint64_t frameID, const WebCore::ResourceError&, const UserData&);
> 
>     void willSubmitForm(uint64_t frameID, uint64_t sourceFrameID, const Vector<std::pair<String, String>>& textFieldValues, uint64_t listenerID, const UserData&);
>@@ -1882,17 +1881,7 @@ private:
>     bool m_isInPrintingMode { false };
>     bool m_isPerformingDOMPrintOperation { false };
> 
>-    bool m_inDecidePolicyForResponseSync { false };
>-    const WebCore::ResourceRequest* m_decidePolicyForResponseRequest { nullptr };
>-    bool m_syncMimeTypePolicyActionIsValid { false };
>-    WebCore::PolicyAction m_syncMimeTypePolicyAction { WebCore::PolicyAction::Use };
>-    DownloadID m_syncMimeTypePolicyDownloadID { 0 };
>-    bool m_inDecidePolicyForNavigationAction { false };
>-    bool m_syncNavigationActionPolicyActionIsValid { false };
>-    WebCore::PolicyAction m_syncNavigationActionPolicyAction { WebCore::PolicyAction::Use };
>-    DownloadID m_syncNavigationActionPolicyDownloadID { 0 };
>-    std::optional<WebsitePoliciesData> m_syncNavigationActionPolicyWebsitePolicies;
>-
>+    WebCore::ResourceRequest m_decidePolicyForResponseRequest;
>     bool m_shouldSuppressAppLinksInNextNavigationPolicyDecision { false };
> 
>     Deque<NativeWebKeyboardEvent> m_keyEventQueue;
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>index a2d484df95b..e6f3bd672fc 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>@@ -98,8 +98,8 @@ messages -> WebPageProxy {
> #endif
> 
>     # Policy messages
>-    DecidePolicyForResponseSync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, uint64_t listenerID, WebKit::UserData userData) -> (bool receivedPolicyAction, enum WebCore::PolicyAction policyAction, WebKit::DownloadID downloadID)
>-    DecidePolicyForNavigationAction(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, uint64_t listenerID, WebKit::UserData userData) -> (bool receivedPolicyAction, uint64_t newNavigationID, enum WebCore::PolicyAction policyAction, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies)
>+    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)
>+    DecidePolicyForNavigationAction(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, uint64_t listenerID, WebKit::UserData userData)
>     DecidePolicyForNewWindowAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, struct WebKit::NavigationActionData navigationActionData, WebCore::ResourceRequest request, String frameName, uint64_t listenerID, WebKit::UserData userData)
>     UnableToImplementPolicy(uint64_t frameID, WebCore::ResourceError error, WebKit::UserData userData)
> 
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>index 1e98b483b18..7c9f168eec6 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>@@ -748,19 +748,9 @@ void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceRespons
> 
>     Ref<WebFrame> protector(*m_frame);
>     uint64_t listenerID = m_frame->setUpPolicyListener(WTFMove(function), WebFrame::ForNavigationAction::No);
>-    bool receivedPolicyAction;
>-    PolicyAction policyAction;
>-    DownloadID downloadID;
>-
>     auto navigationID = static_cast<WebDocumentLoader&>(*policyDocumentLoader).navigationID();
>-    if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForResponseSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), navigationID, response, request, canShowMIMEType, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForResponseSync::Reply(receivedPolicyAction, policyAction, downloadID), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend)) {
>+    if (!webPage->send(Messages::WebPageProxy::DecidePolicyForResponse(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), navigationID, response, request, canShowMIMEType, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()))))
>         m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
>-        return;
>-    }
>-
>-    // We call this synchronously because CFNetwork can only convert a loading connection to a download from its didReceiveResponse callback.
>-    if (receivedPolicyAction)
>-        m_frame->didReceivePolicyDecision(listenerID, policyAction, 0, downloadID, { });
> }
> 
> void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, FramePolicyFunction&& function)
>@@ -843,10 +833,6 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
>     }
>     
>     uint64_t listenerID = m_frame->setUpPolicyListener(WTFMove(function), WebFrame::ForNavigationAction::Yes);
>-    bool receivedPolicyAction;
>-    uint64_t newNavigationID;
>-    PolicyAction policyAction;
>-    DownloadID downloadID;
> 
>     ASSERT(navigationAction.sourceDocument());
>     const Document& sourceDocument = *navigationAction.sourceDocument();
>@@ -885,15 +871,8 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
> 
>     // Notify the UIProcess.
>     Ref<WebFrame> protect(*m_frame);
>-    std::optional<WebsitePoliciesData> websitePolicies;
>-    if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingFrame && originatingFrame->page() ? originatingFrame->page()->pageID() : 0, navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationAction::Reply(receivedPolicyAction, newNavigationID, policyAction, downloadID, websitePolicies))) {
>+    if (!webPage->send(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingFrame && originatingFrame->page() ? originatingFrame->page()->pageID() : 0, navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()))))
>         m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
>-        return;
>-    }
>-
>-    // We call this synchronously because WebCore cannot gracefully handle a frame load without a synchronous navigation policy reply.
>-    if (receivedPolicyAction)
>-        m_frame->didReceivePolicyDecision(listenerID, policyAction, newNavigationID, downloadID, WTFMove(websitePolicies));
> }
> 
> void WebFrameLoaderClient::cancelPolicyCheck()

Actions: View

Attachments on bug 182697: 333600 | 333617 | 333643