Unreviewed, rolling out r234196.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jul 2018 18:04:51 +0000 (18:04 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jul 2018 18:04:51 +0000 (18:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188011

broke API tests (Requested by alexchristensen on #webkit).

Reverted changeset:

"Use CompletionHandler for policy decisions"
https://bugs.webkit.org/show_bug.cgi?id=187975
https://trac.webkit.org/changeset/234196

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234209 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 4edb532..9823a74 100644 (file)
@@ -1,3 +1,16 @@
+2018-07-25  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r234196.
+        https://bugs.webkit.org/show_bug.cgi?id=188011
+
+        broke API tests (Requested by alexchristensen on #webkit).
+
+        Reverted changeset:
+
+        "Use CompletionHandler for policy decisions"
+        https://bugs.webkit.org/show_bug.cgi?id=187975
+        https://trac.webkit.org/changeset/234196
+
 2018-07-25  Alex Christensen  <achristensen@webkit.org>
 
         Use CompletionHandler for policy decisions
index fe4308d..c494204 100644 (file)
 
 namespace WebKit {
 
-WebFramePolicyListenerProxy::WebFramePolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&& completionHandler)
-    : m_completionHandler(WTFMove(completionHandler))
+WebFramePolicyListenerProxy::WebFramePolicyListenerProxy(WebFrameProxy* frame, uint64_t listenerID, PolicyListenerType policyType)
+    : m_policyType(policyType)
+    , m_frame(frame)
+    , m_listenerID(listenerID)
 {
 }
 
+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)
 {
-    m_completionHandler(WebCore::PolicyAction::Use, policies, 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);
 }
 
 void WebFramePolicyListenerProxy::download()
 {
-    m_completionHandler(WebCore::PolicyAction::Download, nullptr, ShouldProcessSwapIfPossible::No);
+    receivedPolicyDecision(WebCore::PolicyAction::Download, std::nullopt, ShouldProcessSwapIfPossible::No);
 }
 
 void WebFramePolicyListenerProxy::ignore()
 {
-    m_completionHandler(WebCore::PolicyAction::Ignore, nullptr, ShouldProcessSwapIfPossible::No);
+    receivedPolicyDecision(WebCore::PolicyAction::Ignore, std::nullopt, ShouldProcessSwapIfPossible::No);
 }
 
 } // namespace WebKit
index 4ce5e1d..319a8be 100644 (file)
 #pragma once
 
 #include "APIObject.h"
-#include <wtf/CompletionHandler.h>
+
+#if PLATFORM(COCOA)
+#include "WKFoundation.h"
+#endif
 
 namespace API {
+class Navigation;
 class WebsitePolicies;
 }
 
@@ -38,24 +42,45 @@ 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(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&& completionHandler)
+    static Ref<WebFramePolicyListenerProxy> create(WebFrameProxy* frame, uint64_t listenerID, PolicyListenerType policyType)
     {
-        return adoptRef(*new WebFramePolicyListenerProxy(WTFMove(completionHandler)));
+        return adoptRef(*new WebFramePolicyListenerProxy(frame, listenerID, policyType));
     }
 
     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(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&&);
+    WebFramePolicyListenerProxy(WebFrameProxy*, uint64_t listenerID, PolicyListenerType);
+
+    void receivedPolicyDecision(WebCore::PolicyAction, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
 
-    CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)> m_completionHandler;
+    PolicyListenerType m_policyType;
+    RefPtr<WebFrameProxy> m_frame;
+    uint64_t m_listenerID { 0 };
+    RefPtr<API::Navigation> m_navigation;
 };
 
 } // namespace WebKit
index 596b4cb..9dc07cf 100644 (file)
@@ -178,15 +178,35 @@ void WebFrameProxy::didChangeTitle(const String& title)
     m_title = title;
 }
 
-WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&& completionHandler)
+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)
 {
     if (m_activeListener)
         m_activeListener->ignore();
-    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;
+    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);
 }
 
 void WebFrameProxy::getWebArchive(Function<void (API::Data*, CallbackBase::Error)>&& callbackFunction)
index 261896f..fb44212 100644 (file)
@@ -53,6 +53,7 @@ class WebFramePolicyListenerProxy;
 class WebPageProxy;
 class WebsiteDataStore;
 enum class ShouldProcessSwapIfPossible;
+enum class PolicyListenerType;
 struct WebsitePoliciesData;
 
 typedef GenericCallback<API::Data*> DataCallback;
@@ -115,7 +116,13 @@ public:
     void didSameDocumentNavigation(const WebCore::URL&); // eg. anchor navigation, session state change.
     void didChangeTitle(const String&);
 
-    WebFramePolicyListenerProxy& setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ShouldProcessSwapIfPossible)>&&);
+    // 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&);
 
 #if ENABLE(CONTENT_FILTERING)
     void contentFilterDidBlockLoad(WebCore::ContentFilterUnblockHandler contentFilterUnblockHandler) { m_contentFilterUnblockHandler = WTFMove(contentFilterUnblockHandler); }
index f7f877c..5df3744 100644 (file)
@@ -51,7 +51,6 @@
 #include "APISecurityOrigin.h"
 #include "APIUIClient.h"
 #include "APIURLRequest.h"
-#include "APIWebsitePolicies.h"
 #include "AuthenticationChallengeProxy.h"
 #include "AuthenticationDecisionListener.h"
 #include "DataReference.h"
@@ -2407,7 +2406,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)
+void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& frame, uint64_t listenerID, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& websitePolicies, ShouldProcessSwapIfPossible shouldProcessSwapIfPossible)
 {
     if (!isValid())
         return;
@@ -2434,6 +2433,23 @@ 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;
@@ -4005,34 +4021,14 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
     navigation->setHasOpenedFrames(navigationActionData.hasOpenedFrames);
     navigation->setOpener(navigationActionData.opener);
 
-    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));
-    }));
+    auto listener = makeRef(frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::NavigationAction));
 
     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);
+        return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, &m_navigationState->navigation(newNavigationID), std::nullopt, ShouldProcessSwapIfPossible::No);
 #else
     UNUSED_PARAM(newNavigationID);
 #endif
@@ -4082,11 +4078,7 @@ void WebPageProxy::decidePolicyForNewWindowAction(uint64_t frameID, const Securi
     MESSAGE_CHECK(frame);
     MESSAGE_CHECK_URL(request.url());
 
-    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);
-    }));
+    Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::NewWindowAction);
 
     if (m_navigationClient) {
         RefPtr<API::FrameInfo> sourceFrameInfo;
@@ -4114,12 +4106,11 @@ void WebPageProxy::decidePolicyForResponse(uint64_t frameID, const SecurityOrigi
     MESSAGE_CHECK_URL(request.url());
     MESSAGE_CHECK_URL(response.url());
 
-    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);
-    }));
+    Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::Response);
+    if (navigationID) {
+        auto& navigation = m_navigationState->navigation(navigationID);
+        listener->setNavigation(navigation);
+    }
 
     if (m_navigationClient) {
         auto navigationResponse = API::NavigationResponse::create(API::FrameInfo::create(*frame, frameSecurityOrigin.securityOrigin()).get(), request, response, canShowMIMEType);
index 186522d..68a2d09 100644 (file)
@@ -908,7 +908,7 @@ public:
     void performDictionaryLookupOfCurrentSelection();
 #endif
 
-    void receivedPolicyDecision(WebCore::PolicyAction, WebFrameProxy&, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&);
+    void receivedPolicyDecision(WebCore::PolicyAction, WebFrameProxy&, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
 
     void backForwardRemovedItem(const WebCore::BackForwardItemIdentifier&);