Use CompletionHandler for policy decisions
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jul 2018 16:07:55 +0000 (16:07 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jul 2018 16:07:55 +0000 (16:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187975

Patch by Alex Christensen <achristensen@webkit.org> on 2018-07-25
Reviewed by Chris Dumez.

* UIProcess/WebFramePolicyListenerProxy.cpp:
(WebKit::WebFramePolicyListenerProxy::WebFramePolicyListenerProxy):
(WebKit::WebFramePolicyListenerProxy::use):
(WebKit::WebFramePolicyListenerProxy::download):
(WebKit::WebFramePolicyListenerProxy::ignore):
(WebKit::WebFramePolicyListenerProxy::receivedPolicyDecision): Deleted.
(WebKit::WebFramePolicyListenerProxy::setNavigation): Deleted.
* UIProcess/WebFramePolicyListenerProxy.h:
(WebKit::WebFramePolicyListenerProxy::create):
(WebKit::WebFramePolicyListenerProxy::policyListenerType const): Deleted.
(WebKit::WebFramePolicyListenerProxy::listenerID const): Deleted.
(): Deleted.
* UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::setUpPolicyListenerProxy):
(WebKit::WebFrameProxy::receivedPolicyDecision): Deleted.
(WebKit::WebFrameProxy::activePolicyListenerProxy): Deleted.
(WebKit::WebFrameProxy::changeWebsiteDataStore): Deleted.
* UIProcess/WebFrameProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::receivedPolicyDecision):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::decidePolicyForNewWindowAction):
(WebKit::WebPageProxy::decidePolicyForResponse):
* UIProcess/WebPageProxy.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234196 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp
Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h
Source/WebKit/UIProcess/WebFrameProxy.cpp
Source/WebKit/UIProcess/WebFrameProxy.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h

index 74a1808..4edb532 100644 (file)
@@ -1,3 +1,35 @@
+2018-07-25  Alex Christensen  <achristensen@webkit.org>
+
+        Use CompletionHandler for policy decisions
+        https://bugs.webkit.org/show_bug.cgi?id=187975
+
+        Reviewed by Chris Dumez.
+
+        * UIProcess/WebFramePolicyListenerProxy.cpp:
+        (WebKit::WebFramePolicyListenerProxy::WebFramePolicyListenerProxy):
+        (WebKit::WebFramePolicyListenerProxy::use):
+        (WebKit::WebFramePolicyListenerProxy::download):
+        (WebKit::WebFramePolicyListenerProxy::ignore):
+        (WebKit::WebFramePolicyListenerProxy::receivedPolicyDecision): Deleted.
+        (WebKit::WebFramePolicyListenerProxy::setNavigation): Deleted.
+        * UIProcess/WebFramePolicyListenerProxy.h:
+        (WebKit::WebFramePolicyListenerProxy::create):
+        (WebKit::WebFramePolicyListenerProxy::policyListenerType const): Deleted.
+        (WebKit::WebFramePolicyListenerProxy::listenerID const): Deleted.
+        (): Deleted.
+        * UIProcess/WebFrameProxy.cpp:
+        (WebKit::WebFrameProxy::setUpPolicyListenerProxy):
+        (WebKit::WebFrameProxy::receivedPolicyDecision): Deleted.
+        (WebKit::WebFrameProxy::activePolicyListenerProxy): Deleted.
+        (WebKit::WebFrameProxy::changeWebsiteDataStore): Deleted.
+        * UIProcess/WebFrameProxy.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::receivedPolicyDecision):
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+        (WebKit::WebPageProxy::decidePolicyForNewWindowAction):
+        (WebKit::WebPageProxy::decidePolicyForResponse):
+        * UIProcess/WebPageProxy.h:
+
 2018-07-25  Brent Fulgham  <bfulgham@apple.com>
 
         [macOS] PluginProcess needs TCC entitlements for media capture
index c494204..fe4308d 100644 (file)
 
 namespace WebKit {
 
-WebFramePolicyListenerProxy::WebFramePolicyListenerProxy(WebFrameProxy* frame, uint64_t listenerID, PolicyListenerType policyType)
-    : m_policyType(policyType)
-    , m_frame(frame)
-    , m_listenerID(listenerID)
+WebFramePolicyListenerProxy::WebFramePolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&& completionHandler)
+    : m_completionHandler(WTFMove(completionHandler))
 {
 }
 
-void WebFramePolicyListenerProxy::receivedPolicyDecision(WebCore::PolicyAction action, std::optional<WebsitePoliciesData>&& data, ShouldProcessSwapIfPossible swap)
-{
-    if (!m_frame)
-        return;
-    
-    m_frame->receivedPolicyDecision(action, m_listenerID, m_navigation.get(), WTFMove(data), swap);
-    m_frame = nullptr;
-}
-
-void WebFramePolicyListenerProxy::setNavigation(Ref<API::Navigation>&& navigation)
-{
-    m_navigation = WTFMove(navigation);
-}
-    
 void WebFramePolicyListenerProxy::use(API::WebsitePolicies* policies, ShouldProcessSwapIfPossible swap)
 {
-    std::optional<WebsitePoliciesData> data;
-    if (policies) {
-        data = policies->data();
-        if (m_frame && policies->websiteDataStore())
-            m_frame->changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
-    }
-
-    receivedPolicyDecision(WebCore::PolicyAction::Use, WTFMove(data), swap);
+    m_completionHandler(WebCore::PolicyAction::Use, policies, swap);
 }
 
 void WebFramePolicyListenerProxy::download()
 {
-    receivedPolicyDecision(WebCore::PolicyAction::Download, std::nullopt, ShouldProcessSwapIfPossible::No);
+    m_completionHandler(WebCore::PolicyAction::Download, nullptr, ShouldProcessSwapIfPossible::No);
 }
 
 void WebFramePolicyListenerProxy::ignore()
 {
-    receivedPolicyDecision(WebCore::PolicyAction::Ignore, std::nullopt, ShouldProcessSwapIfPossible::No);
+    m_completionHandler(WebCore::PolicyAction::Ignore, nullptr, ShouldProcessSwapIfPossible::No);
 }
 
 } // namespace WebKit
index 319a8be..4ce5e1d 100644 (file)
 #pragma once
 
 #include "APIObject.h"
-
-#if PLATFORM(COCOA)
-#include "WKFoundation.h"
-#endif
+#include <wtf/CompletionHandler.h>
 
 namespace API {
-class Navigation;
 class WebsitePolicies;
 }
 
@@ -42,45 +38,24 @@ enum class PolicyAction;
 
 namespace WebKit {
 
-class WebFrameProxy;
-class WebsiteDataStore;
-struct WebsitePoliciesData;
-
-enum class PolicyListenerType {
-    NavigationAction,
-    NewWindowAction,
-    Response,
-};
-
 enum class ShouldProcessSwapIfPossible { No, Yes };
 
 class WebFramePolicyListenerProxy : public API::ObjectImpl<API::Object::Type::FramePolicyListener> {
 public:
 
-    static Ref<WebFramePolicyListenerProxy> create(WebFrameProxy* frame, uint64_t listenerID, PolicyListenerType policyType)
+    static Ref<WebFramePolicyListenerProxy> create(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&& completionHandler)
     {
-        return adoptRef(*new WebFramePolicyListenerProxy(frame, listenerID, policyType));
+        return adoptRef(*new WebFramePolicyListenerProxy(WTFMove(completionHandler)));
     }
 
     void use(API::WebsitePolicies* = nullptr, ShouldProcessSwapIfPossible = ShouldProcessSwapIfPossible::No);
     void download();
     void ignore();
 
-    PolicyListenerType policyListenerType() const { return m_policyType; }
-
-    uint64_t listenerID() const { return m_listenerID; }
-    
-    void setNavigation(Ref<API::Navigation>&&);
-
 private:
-    WebFramePolicyListenerProxy(WebFrameProxy*, uint64_t listenerID, PolicyListenerType);
-
-    void receivedPolicyDecision(WebCore::PolicyAction, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
+    WebFramePolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&&);
 
-    PolicyListenerType m_policyType;
-    RefPtr<WebFrameProxy> m_frame;
-    uint64_t m_listenerID { 0 };
-    RefPtr<API::Navigation> m_navigation;
+    CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)> m_completionHandler;
 };
 
 } // namespace WebKit
index 9dc07cf..596b4cb 100644 (file)
@@ -178,35 +178,15 @@ void WebFrameProxy::didChangeTitle(const String& title)
     m_title = title;
 }
 
-void WebFrameProxy::receivedPolicyDecision(PolicyAction action, uint64_t listenerID, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& data, ShouldProcessSwapIfPossible swap)
-{
-    if (!m_page)
-        return;
-
-    ASSERT(m_activeListener);
-    ASSERT(m_activeListener->listenerID() == listenerID);
-    m_page->receivedPolicyDecision(action, *this, listenerID, navigation, WTFMove(data), swap);
-}
-
-WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(uint64_t listenerID, PolicyListenerType policyListenerType)
+WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&& completionHandler)
 {
     if (m_activeListener)
         m_activeListener->ignore();
-    m_activeListener = WebFramePolicyListenerProxy::create(this, listenerID, policyListenerType);
-    return *static_cast<WebFramePolicyListenerProxy*>(m_activeListener.get());
-}
-
-WebFramePolicyListenerProxy* WebFrameProxy::activePolicyListenerProxy()
-{
-    return m_activeListener.get();
-}
-
-void WebFrameProxy::changeWebsiteDataStore(WebsiteDataStore& websiteDataStore)
-{
-    if (!m_page)
-        return;
-
-    m_page->changeWebsiteDataStore(websiteDataStore);
+    m_activeListener = WebFramePolicyListenerProxy::create([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (WebCore::PolicyAction action, API::WebsitePolicies* policies, ShouldProcessSwapIfPossible swap) {
+        completionHandler(action, policies, swap);
+        m_activeListener = nullptr;
+    });
+    return *m_activeListener;
 }
 
 void WebFrameProxy::getWebArchive(Function<void (API::Data*, CallbackBase::Error)>&& callbackFunction)
index fb44212..261896f 100644 (file)
@@ -53,7 +53,6 @@ class WebFramePolicyListenerProxy;
 class WebPageProxy;
 class WebsiteDataStore;
 enum class ShouldProcessSwapIfPossible;
-enum class PolicyListenerType;
 struct WebsitePoliciesData;
 
 typedef GenericCallback<API::Data*> DataCallback;
@@ -116,13 +115,7 @@ public:
     void didSameDocumentNavigation(const WebCore::URL&); // eg. anchor navigation, session state change.
     void didChangeTitle(const String&);
 
-    // Policy operations.
-    void receivedPolicyDecision(WebCore::PolicyAction, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
-
-    WebFramePolicyListenerProxy& setUpPolicyListenerProxy(uint64_t listenerID, PolicyListenerType);
-    WebFramePolicyListenerProxy* activePolicyListenerProxy();
-
-    void changeWebsiteDataStore(WebsiteDataStore&);
+    WebFramePolicyListenerProxy& setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&&);
 
 #if ENABLE(CONTENT_FILTERING)
     void contentFilterDidBlockLoad(WebCore::ContentFilterUnblockHandler contentFilterUnblockHandler) { m_contentFilterUnblockHandler = WTFMove(contentFilterUnblockHandler); }
index 5df3744..f7f877c 100644 (file)
@@ -51,6 +51,7 @@
 #include "APISecurityOrigin.h"
 #include "APIUIClient.h"
 #include "APIURLRequest.h"
+#include "APIWebsitePolicies.h"
 #include "AuthenticationChallengeProxy.h"
 #include "AuthenticationDecisionListener.h"
 #include "DataReference.h"
@@ -2406,7 +2407,7 @@ void WebPageProxy::centerSelectionInVisibleArea()
     m_process->send(Messages::WebPage::CenterSelectionInVisibleArea(), m_pageID);
 }
 
-void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& frame, uint64_t listenerID, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& websitePolicies, ShouldProcessSwapIfPossible shouldProcessSwapIfPossible)
+void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& frame, uint64_t listenerID, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& websitePolicies)
 {
     if (!isValid())
         return;
@@ -2433,23 +2434,6 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
         m_decidePolicyForResponseRequest = { };
     }
 
-    auto* activePolicyListener = frame.activePolicyListenerProxy();
-    if (activePolicyListener && activePolicyListener->policyListenerType() == PolicyListenerType::NavigationAction) {
-        ASSERT(activePolicyListener->listenerID() == listenerID);
-
-        if (action == PolicyAction::Use && navigation && frame.isMainFrame()) {
-            auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, shouldProcessSwapIfPossible, action);
-
-            if (proposedProcess.ptr() != &process()) {
-                LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), proposedProcess->processIdentifier(), navigation->navigationID(), navigation->loggingString());
-
-                RunLoop::main().dispatch([this, protectedThis = makeRef(*this), navigation = makeRef(*navigation), proposedProcess = WTFMove(proposedProcess)]() mutable {
-                    continueNavigationInNewProcess(navigation.get(), WTFMove(proposedProcess));
-                });
-            }
-        }
-    }
-
     if (auto syncNavigationActionPolicyReply = WTFMove(m_syncNavigationActionPolicyReply)) {
         syncNavigationActionPolicyReply(navigation ? navigation->navigationID() : 0, action, downloadID, WTFMove(websitePolicies));
         return;
@@ -4021,14 +4005,34 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
     navigation->setHasOpenedFrames(navigationActionData.hasOpenedFrames);
     navigation->setOpener(navigationActionData.opener);
 
-    auto listener = makeRef(frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::NavigationAction));
+    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(*frame), listenerID, navigation] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ShouldProcessSwapIfPossible swap) {
+        std::optional<WebsitePoliciesData> data;
+        if (policies) {
+            data = policies->data();
+            if (policies->websiteDataStore())
+                changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
+        }
+
+        if (policyAction == PolicyAction::Use && frame->isMainFrame()) {
+            auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, swap, policyAction);
+            
+            if (proposedProcess.ptr() != &process()) {
+                LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), proposedProcess->processIdentifier(), navigation->navigationID(), navigation->loggingString());
+                
+                RunLoop::main().dispatch([this, protectedThis = makeRef(*this), navigation = makeRef(*navigation), proposedProcess = WTFMove(proposedProcess)]() mutable {
+                    continueNavigationInNewProcess(navigation.get(), WTFMove(proposedProcess));
+                });
+            }
+        }
+
+        receivedPolicyDecision(policyAction, frame.get(), listenerID, navigation.get(), WTFMove(data));
+    }));
 
     API::Navigation* mainFrameNavigation = frame->isMainFrame() ? navigation.get() : nullptr;
-    listener->setNavigation(navigation.releaseNonNull());
 
 #if ENABLE(CONTENT_FILTERING)
     if (frame->didHandleContentFilterUnblockNavigation(request))
-        return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, &m_navigationState->navigation(newNavigationID), std::nullopt, ShouldProcessSwapIfPossible::No);
+        return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, &m_navigationState->navigation(newNavigationID), std::nullopt);
 #else
     UNUSED_PARAM(newNavigationID);
 #endif
@@ -4078,7 +4082,11 @@ void WebPageProxy::decidePolicyForNewWindowAction(uint64_t frameID, const Securi
     MESSAGE_CHECK(frame);
     MESSAGE_CHECK_URL(request.url());
 
-    Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::NewWindowAction);
+    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), listenerID, frame = makeRef(*frame)] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ShouldProcessSwapIfPossible swap) {
+        ASSERT_UNUSED(policies, !policies);
+        ASSERT_UNUSED(swap, swap == ShouldProcessSwapIfPossible::No);
+        receivedPolicyDecision(policyAction, frame.get(), listenerID, nullptr, std::nullopt);
+    }));
 
     if (m_navigationClient) {
         RefPtr<API::FrameInfo> sourceFrameInfo;
@@ -4106,11 +4114,12 @@ void WebPageProxy::decidePolicyForResponse(uint64_t frameID, const SecurityOrigi
     MESSAGE_CHECK_URL(request.url());
     MESSAGE_CHECK_URL(response.url());
 
-    Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::Response);
-    if (navigationID) {
-        auto& navigation = m_navigationState->navigation(navigationID);
-        listener->setNavigation(navigation);
-    }
+    RefPtr<API::Navigation> navigation = navigationID ? &m_navigationState->navigation(navigationID) : nullptr;
+    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(*frame), listenerID, navigation = WTFMove(navigation)] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ShouldProcessSwapIfPossible swap) {
+        ASSERT_UNUSED(policies, !policies);
+        ASSERT_UNUSED(swap, swap == ShouldProcessSwapIfPossible::No);
+        receivedPolicyDecision(policyAction, frame.get(), listenerID, nullptr, std::nullopt);
+    }));
 
     if (m_navigationClient) {
         auto navigationResponse = API::NavigationResponse::create(API::FrameInfo::create(*frame, frameSecurityOrigin.securityOrigin()).get(), request, response, canShowMIMEType);
index 68a2d09..186522d 100644 (file)
@@ -908,7 +908,7 @@ public:
     void performDictionaryLookupOfCurrentSelection();
 #endif
 
-    void receivedPolicyDecision(WebCore::PolicyAction, WebFrameProxy&, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
+    void receivedPolicyDecision(WebCore::PolicyAction, WebFrameProxy&, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&);
 
     void backForwardRemovedItem(const WebCore::BackForwardItemIdentifier&);