Attachment 345767 Details for Bug 188011 – ROLLOUT of r234196

[patch] ROLLOUT of r234196

bug-188011-20180725140153.patch (text/plain), 17.01 KB, created by WebKit Commit Bot on 2018-07-25 11:01:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: WebKit Commit Bot

Created: 2018-07-25 11:01:53 PDT

Size: 17.01 KB

patch

obsolete

>Subversion Revision: 234207
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4edb532c1472ac873b1f1189446623b686926925..9823a748c5edf1eb22718c775ed4c865b1e2cb9a 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -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
>diff --git a/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp b/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp
>index fe4308d0e57bc5e4b76517dac412d7ed23e255bf..c4942046de13dc9860b73fd27eff155ff89a352d 100644
>--- a/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp
>@@ -35,24 +35,47 @@
> 
> 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
>diff --git a/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h b/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h
>index 4ce5e1df5d030a68c9b585aecb6450d37f7f2aec..319a8be6233b5125fe0a1ef1e23830732b905a97 100644
>--- a/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h
>+++ b/Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h
>@@ -26,9 +26,13 @@
> #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
>diff --git a/Source/WebKit/UIProcess/WebFrameProxy.cpp b/Source/WebKit/UIProcess/WebFrameProxy.cpp
>index 596b4cb9145987a2acd7dea26c9a259e59505bbb..9dc07cf7a5c52795fc97358cb91ba01bcc359c4b 100644
>--- a/Source/WebKit/UIProcess/WebFrameProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebFrameProxy.cpp
>@@ -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)
>diff --git a/Source/WebKit/UIProcess/WebFrameProxy.h b/Source/WebKit/UIProcess/WebFrameProxy.h
>index 261896fd25a14b92ca9b5d5a183528a0d8c2b74d..fb4421233c08b946de4b66c086a0dcd08ffeb946 100644
>--- a/Source/WebKit/UIProcess/WebFrameProxy.h
>+++ b/Source/WebKit/UIProcess/WebFrameProxy.h
>@@ -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); }
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index f7f877c200550877ae106f4f9120c7708ff726df..5df37447f533346225894a55b33f00c073e918c6 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -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);
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 186522d2f39114b2f7304a159b73ef98890d7813..68a2d09719ad97be6fa5b5c78495fd3a6ee975eb 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -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&);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 188011: 345767