Attachment 349807 Details for Bug 189631 – Patch

[patch] Patch

bug-189631-20180914142936.patch (text/plain), 17.57 KB, created by Alex Christensen on 2018-09-14 14:29:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-09-14 14:29:37 PDT

Size: 17.57 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 236018)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,38 @@
>+2018-09-14  Alex Christensen  <achristensen@webkit.org>
>+
>+        Refactoring related to Safe Browsing
>+        https://bugs.webkit.org/show_bug.cgi?id=189631
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make SafeBrowsingResult RefCounted.
>+        Move logic from an unnamed lambda to WebPageProxy::receivedNavigationPolicyDecision.
>+
>+        * UIProcess/Cocoa/WebPageProxyCocoa.mm:
>+        (WebKit::WebPageProxy::beginSafeBrowsingCheck):
>+        (WebKit::WebPageProxy::contentFilterDidBlockLoadForFrame): Deleted.
>+        (WebKit::WebPageProxy::addPlatformLoadParameters): Deleted.
>+        (WebKit::WebPageProxy::createSandboxExtensionsIfNeeded): Deleted.
>+        (WebKit::WebPageProxy::startDrag): Deleted.
>+        (WebKit::WebPageProxy::setPromisedDataForImage): Deleted.
>+        (WebKit::WebPageProxy::setDragCaretRect): Deleted.
>+        (WebKit::WebPageProxy::platformRegisterAttachment): Deleted.
>+        (WebKit::WebPageProxy::platformCloneAttachment): Deleted.
>+        * UIProcess/SafeBrowsingResult.h:
>+        (WebKit::SafeBrowsingResult::create):
>+        * UIProcess/WebFramePolicyListenerProxy.cpp:
>+        (WebKit::WebFramePolicyListenerProxy::didReceiveSafeBrowsingResults):
>+        * UIProcess/WebFramePolicyListenerProxy.h:
>+        * UIProcess/WebFrameProxy.cpp:
>+        (WebKit::WebFrameProxy::setUpPolicyListenerProxy):
>+        * UIProcess/WebFrameProxy.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
>+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
>+        (WebKit::WebPageProxy::decidePolicyForNewWindowAction):
>+        (WebKit::WebPageProxy::decidePolicyForResponse):
>+        * UIProcess/WebPageProxy.h:
>+
> 2018-09-11  Simon Fraser  <simon.fraser@apple.com>
> 
>         Make GraphicsLayers ref-counted, so their tree can persist when disconnected from RenderLayerBackings
>Index: Source/WebKit/UIProcess/SafeBrowsingResult.h
>===================================================================
>--- Source/WebKit/UIProcess/SafeBrowsingResult.h	(revision 236017)
>+++ Source/WebKit/UIProcess/SafeBrowsingResult.h	(working copy)
>@@ -26,19 +26,21 @@
> #pragma once
> 
> #include <WebCore/URL.h>
>+#include <wtf/RefCounted.h>
> #include <wtf/text/WTFString.h>
> 
> OBJC_CLASS SSBServiceLookupResult;
> 
> namespace WebKit {
> 
>-class SafeBrowsingResult {
>+class SafeBrowsingResult : public RefCounted<SafeBrowsingResult> {
> public:
> #if HAVE(SAFE_BROWSING)
>-    SafeBrowsingResult(WebCore::URL&&, SSBServiceLookupResult *);
>+    static Ref<SafeBrowsingResult> create(WebCore::URL&& url, SSBServiceLookupResult *result)
>+    {
>+        return adoptRef(*new SafeBrowsingResult(WTFMove(url), result));
>+    }
> #endif
>-    SafeBrowsingResult() = default;
>-
>     const WebCore::URL& url() const { return m_url; }
>     const String& provider() const { return m_provider; }
>     bool isPhishing() const { return m_isPhishing; }
>@@ -47,6 +49,9 @@ public:
>     bool isKnownToBeUnsafe() const { return m_isKnownToBeUnsafe; }
> 
> private:
>+#if HAVE(SAFE_BROWSING)
>+    SafeBrowsingResult(WebCore::URL&&, SSBServiceLookupResult *);
>+#endif
>     WebCore::URL m_url;
>     String m_provider;
>     bool m_isPhishing { false };
>Index: Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp	(revision 236017)
>+++ Source/WebKit/UIProcess/WebFramePolicyListenerProxy.cpp	(working copy)
>@@ -45,7 +45,7 @@ WebFramePolicyListenerProxy::WebFramePol
> 
> WebFramePolicyListenerProxy::~WebFramePolicyListenerProxy() = default;
> 
>-void WebFramePolicyListenerProxy::didReceiveSafeBrowsingResults(Vector<SafeBrowsingResult>&& safeBrowsingResults)
>+void WebFramePolicyListenerProxy::didReceiveSafeBrowsingResults(Vector<Ref<SafeBrowsingResult>>&& safeBrowsingResults)
> {
>     ASSERT(!m_safeBrowsingResults);
>     if (m_policyResult) {
>Index: Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h	(revision 236017)
>+++ Source/WebKit/UIProcess/WebFramePolicyListenerProxy.h	(working copy)
>@@ -47,7 +47,7 @@ enum class ShouldExpectSafeBrowsingResul
> class WebFramePolicyListenerProxy : public API::ObjectImpl<API::Object::Type::FramePolicyListener> {
> public:
> 
>-    using Reply = CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, Vector<SafeBrowsingResult>&&)>;
>+    using Reply = CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&)>;
>     static Ref<WebFramePolicyListenerProxy> create(Reply&& reply, ShouldExpectSafeBrowsingResult expect)
>     {
>         return adoptRef(*new WebFramePolicyListenerProxy(WTFMove(reply), expect));
>@@ -58,13 +58,13 @@ public:
>     void download();
>     void ignore();
>     
>-    void didReceiveSafeBrowsingResults(Vector<SafeBrowsingResult>&&);
>+    void didReceiveSafeBrowsingResults(Vector<Ref<SafeBrowsingResult>>&&);
> 
> private:
>     WebFramePolicyListenerProxy(Reply&&, ShouldExpectSafeBrowsingResult);
> 
>     std::optional<std::pair<RefPtr<API::WebsitePolicies>, ProcessSwapRequestedByClient>> m_policyResult;
>-    std::optional<Vector<SafeBrowsingResult>> m_safeBrowsingResults;
>+    std::optional<Vector<Ref<SafeBrowsingResult>>> m_safeBrowsingResults;
>     Reply m_reply;
> };
> 
>Index: Source/WebKit/UIProcess/WebFrameProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebFrameProxy.cpp	(revision 236017)
>+++ Source/WebKit/UIProcess/WebFrameProxy.cpp	(working copy)
>@@ -177,11 +177,11 @@ void WebFrameProxy::didChangeTitle(const
>     m_title = title;
> }
> 
>-WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, Vector<SafeBrowsingResult>&&)>&& completionHandler, ShouldExpectSafeBrowsingResult expect)
>+WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&)>&& completionHandler, ShouldExpectSafeBrowsingResult expect)
> {
>     if (m_activeListener)
>         m_activeListener->ignore();
>-    m_activeListener = WebFramePolicyListenerProxy::create([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (WebCore::PolicyAction action, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<SafeBrowsingResult>&& safeBrowsingResults) mutable {
>+    m_activeListener = WebFramePolicyListenerProxy::create([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (WebCore::PolicyAction action, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&& safeBrowsingResults) mutable {
>         completionHandler(action, policies, processSwapRequestedByClient, WTFMove(safeBrowsingResults));
>         m_activeListener = nullptr;
>     }, expect);
>Index: Source/WebKit/UIProcess/WebFrameProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/WebFrameProxy.h	(revision 236017)
>+++ Source/WebKit/UIProcess/WebFrameProxy.h	(working copy)
>@@ -117,7 +117,7 @@ public:
>     void didSameDocumentNavigation(const WebCore::URL&); // eg. anchor navigation, session state change.
>     void didChangeTitle(const String&);
> 
>-    WebFramePolicyListenerProxy& setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, Vector<SafeBrowsingResult>&&)>&&, ShouldExpectSafeBrowsingResult);
>+    WebFramePolicyListenerProxy& setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&)>&&, ShouldExpectSafeBrowsingResult);
> 
> #if ENABLE(CONTENT_FILTERING)
>     void contentFilterDidBlockLoad(WebCore::ContentFilterUnblockHandler contentFilterUnblockHandler) { m_contentFilterUnblockHandler = WTFMove(contentFilterUnblockHandler); }
>Index: Source/WebKit/UIProcess/WebPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.cpp	(revision 236017)
>+++ Source/WebKit/UIProcess/WebPageProxy.cpp	(working copy)
>@@ -2431,7 +2431,36 @@ private:
> 
>     SendFunction m_sendFunction;
> };
>+
>+void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, WebFrameProxy& frame, API::WebsitePolicies* policies, Ref<PolicyDecisionSender>&& sender)
>+{
>+    std::optional<WebsitePoliciesData> data;
>+    if (policies) {
>+        data = policies->data();
>+        if (policies->websiteDataStore())
>+            changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
>+    }
>+    
>+    if (policyAction == PolicyAction::Use && frame.isMainFrame()) {
>+        String reason;
>+        auto proposedProcess = process().processPool().processForNavigation(*this, navigation, processSwapRequestedByClient, policyAction, reason);
>+        ASSERT(!reason.isNull());
>+        
>+        if (proposedProcess.ptr() != &process()) {
>+            RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), proposedProcess->processIdentifier(), reason.utf8().data());
>+            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, WTFMove(proposedProcess));
>+            });
>+        } else
>+            RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, keep using process %i for navigation, reason: %{public}s", this, processIdentifier(), reason.utf8().data());
>+    }
>     
>+    receivedPolicyDecision(policyAction, &navigation, WTFMove(data), WTFMove(sender));
>+
>+}
>+
> void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& websitePolicies, Ref<PolicyDecisionSender>&& sender)
> {
>     if (!isValid()) {
>@@ -4033,33 +4062,9 @@ void WebPageProxy::decidePolicyForNaviga
>     UNUSED_PARAM(newNavigationID);
> #endif
> 
>-    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(*frame), sender = sender.copyRef(), navigation] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<SafeBrowsingResult>&&) mutable {
>+    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(*frame), sender = WTFMove(sender), navigation = navigation.releaseNonNull()] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&) mutable {
>         // FIXME: do something with the SafeBrowsingResults.
>-
>-        std::optional<WebsitePoliciesData> data;
>-        if (policies) {
>-            data = policies->data();
>-            if (policies->websiteDataStore())
>-                changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
>-        }
>-
>-        if (policyAction == PolicyAction::Use && frame->isMainFrame()) {
>-            String reason;
>-            auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, policyAction, reason);
>-            ASSERT(!reason.isNull());
>-            
>-            if (proposedProcess.ptr() != &process()) {
>-                RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), proposedProcess->processIdentifier(), reason.utf8().data());
>-                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 = WTFMove(protectedThis), navigation = makeRef(*navigation), proposedProcess = WTFMove(proposedProcess)]() mutable {
>-                    continueNavigationInNewProcess(navigation.get(), WTFMove(proposedProcess));
>-                });
>-            } else
>-                RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, keep using process %i for navigation, reason: %{public}s", this, processIdentifier(), reason.utf8().data());
>-        }
>-
>-        receivedPolicyDecision(policyAction, navigation.get(), WTFMove(data), WTFMove(sender));
>+        receivedNavigationPolicyDecision(policyAction, navigation, processSwapRequestedByClient, frame, policies, WTFMove(sender));
>     }, shouldSkipSafeBrowsingCheck == ShouldSkipSafeBrowsingCheck::Yes ? ShouldExpectSafeBrowsingResult::No : ShouldExpectSafeBrowsingResult::Yes));
>     if (shouldSkipSafeBrowsingCheck == ShouldSkipSafeBrowsingCheck::No)
>         beginSafeBrowsingCheck(request.url(), listener);
>@@ -4108,7 +4113,7 @@ void WebPageProxy::decidePolicyForNewWin
>     MESSAGE_CHECK(frame);
>     MESSAGE_CHECK_URL(request.url());
> 
>-    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), listenerID, frameID] (WebCore::PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<SafeBrowsingResult>&& safeBrowsingResults) mutable {
>+    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), listenerID, frameID] (WebCore::PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&& safeBrowsingResults) mutable {
>         // FIXME: Assert the API::WebsitePolicies* is nullptr here once clients of WKFramePolicyListenerUseWithPolicies go away.
>         RELEASE_ASSERT(processSwapRequestedByClient == ProcessSwapRequestedByClient::No);
>         ASSERT_UNUSED(safeBrowsingResults, safeBrowsingResults.isEmpty());
>@@ -4144,7 +4149,7 @@ void WebPageProxy::decidePolicyForRespon
>     MESSAGE_CHECK_URL(response.url());
> 
>     RefPtr<API::Navigation> navigation = navigationID ? &m_navigationState->navigation(navigationID) : nullptr;
>-    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frameID, listenerID, navigation = WTFMove(navigation)] (WebCore::PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<SafeBrowsingResult>&& safeBrowsingResults) mutable {
>+    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frameID, listenerID, navigation = WTFMove(navigation)] (WebCore::PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&& safeBrowsingResults) mutable {
>         // FIXME: Assert the API::WebsitePolicies* is nullptr here once clients of WKFramePolicyListenerUseWithPolicies go away.
>         RELEASE_ASSERT(processSwapRequestedByClient == ProcessSwapRequestedByClient::No);
>         ASSERT_UNUSED(safeBrowsingResults, safeBrowsingResults.isEmpty());
>Index: Source/WebKit/UIProcess/WebPageProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.h	(revision 236017)
>+++ Source/WebKit/UIProcess/WebPageProxy.h	(working copy)
>@@ -913,6 +913,7 @@ public:
> 
>     class PolicyDecisionSender;
>     void receivedPolicyDecision(WebCore::PolicyAction, API::Navigation*, std::optional<WebsitePoliciesData>&&, Ref<PolicyDecisionSender>&&);
>+    void receivedNavigationPolicyDecision(WebCore::PolicyAction, API::Navigation&, ProcessSwapRequestedByClient, WebFrameProxy&, API::WebsitePolicies*, Ref<PolicyDecisionSender>&&);
> 
>     void backForwardRemovedItem(const WebCore::BackForwardItemIdentifier&);
> 
>Index: Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
>===================================================================
>--- Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm	(revision 236017)
>+++ Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm	(working copy)
>@@ -85,10 +85,10 @@ void WebPageProxy::beginSafeBrowsingChec
>             }
> 
>             NSArray<SSBServiceLookupResult *> *results = [result serviceLookupResults];
>-            Vector<SafeBrowsingResult> resultsVector;
>+            Vector<Ref<SafeBrowsingResult>> resultsVector;
>             resultsVector.reserveInitialCapacity([results count]);
>             for (SSBServiceLookupResult *result in results)
>-                resultsVector.uncheckedAppend({ URL(url), result });
>+                resultsVector.uncheckedAppend(SafeBrowsingResult::create(URL(url), result));
>             listener->didReceiveSafeBrowsingResults(WTFMove(resultsVector));
>         });
>     }).get()];

Actions: View | Formatted Diff | Diff

Attachments on bug 189631: 349803 | 349807 | 349892