Reduce getters/setters in WebFramePolicyListenerProxy
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jul 2018 17:27:15 +0000 (17:27 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jul 2018 17:27:15 +0000 (17:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187830

Reviewed by Dean Jackson.

This is a step towards making it a lambda, which has no getters or setters.
No change in behavior.

setApplyPolicyInNewProcessIfPossible can be replaced by passing another parameter.
This bit was just piggy-backing on the WebFramePolicyListenerProxy.

isMainFrame was only used in an assert, which has a corresponding ObjC exception in
NavigationState::NavigationClient::decidePolicyForNavigationAction for the one relevant client.

* UIProcess/API/C/WKFramePolicyListener.cpp:
(WKFramePolicyListenerUseInNewProcess):
(useWithPolicies):
(WKFramePolicyListenerUseWithPolicies):
(WKFramePolicyListenerUseInNewProcessWithPolicies):
* UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
* UIProcess/WebFramePolicyListenerProxy.cpp:
(WebKit::WebFramePolicyListenerProxy::receivedPolicyDecision):
(WebKit::WebFramePolicyListenerProxy::use):
(WebKit::WebFramePolicyListenerProxy::download):
(WebKit::WebFramePolicyListenerProxy::ignore):
(WebKit::WebFramePolicyListenerProxy::isMainFrame const): Deleted.
* UIProcess/WebFramePolicyListenerProxy.h:
(WebKit::WebFramePolicyListenerProxy::setApplyPolicyInNewProcessIfPossible): Deleted.
(WebKit::WebFramePolicyListenerProxy::applyPolicyInNewProcessIfPossible const): Deleted.
* UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::receivedPolicyDecision):
* UIProcess/WebFrameProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::receivedPolicyDecision):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
* UIProcess/WebPageProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigation):
(WebKit::WebProcessPool::processForNavigationInternal):
* UIProcess/WebProcessPool.h:

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/C/WKFramePolicyListener.cpp
Source/WebKit/UIProcess/Cocoa/NavigationState.mm
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
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h

index 47f0a70..c41be21 100644 (file)
@@ -1,5 +1,49 @@
 2018-07-24  Alex Christensen  <achristensen@webkit.org>
 
+        Reduce getters/setters in WebFramePolicyListenerProxy
+        https://bugs.webkit.org/show_bug.cgi?id=187830
+
+        Reviewed by Dean Jackson.
+
+        This is a step towards making it a lambda, which has no getters or setters.
+        No change in behavior.
+
+        setApplyPolicyInNewProcessIfPossible can be replaced by passing another parameter.
+        This bit was just piggy-backing on the WebFramePolicyListenerProxy.
+
+        isMainFrame was only used in an assert, which has a corresponding ObjC exception in
+        NavigationState::NavigationClient::decidePolicyForNavigationAction for the one relevant client.
+
+        * UIProcess/API/C/WKFramePolicyListener.cpp:
+        (WKFramePolicyListenerUseInNewProcess):
+        (useWithPolicies):
+        (WKFramePolicyListenerUseWithPolicies):
+        (WKFramePolicyListenerUseInNewProcessWithPolicies):
+        * UIProcess/Cocoa/NavigationState.mm:
+        (WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
+        * UIProcess/WebFramePolicyListenerProxy.cpp:
+        (WebKit::WebFramePolicyListenerProxy::receivedPolicyDecision):
+        (WebKit::WebFramePolicyListenerProxy::use):
+        (WebKit::WebFramePolicyListenerProxy::download):
+        (WebKit::WebFramePolicyListenerProxy::ignore):
+        (WebKit::WebFramePolicyListenerProxy::isMainFrame const): Deleted.
+        * UIProcess/WebFramePolicyListenerProxy.h:
+        (WebKit::WebFramePolicyListenerProxy::setApplyPolicyInNewProcessIfPossible): Deleted.
+        (WebKit::WebFramePolicyListenerProxy::applyPolicyInNewProcessIfPossible const): Deleted.
+        * UIProcess/WebFrameProxy.cpp:
+        (WebKit::WebFrameProxy::receivedPolicyDecision):
+        * UIProcess/WebFrameProxy.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::receivedPolicyDecision):
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::processForNavigation):
+        (WebKit::WebProcessPool::processForNavigationInternal):
+        * UIProcess/WebProcessPool.h:
+
+2018-07-24  Alex Christensen  <achristensen@webkit.org>
+
         Remove WebFramePolicyListenerProxy::invalidate
         https://bugs.webkit.org/show_bug.cgi?id=187833
 
index ff97e98..8338899 100644 (file)
@@ -31,6 +31,7 @@
 #include "WKAPICast.h"
 #include "WebFramePolicyListenerProxy.h"
 #include "WebFrameProxy.h"
+#include "WebProcessPool.h"
 #include "WebsitePoliciesData.h"
 
 using namespace WebKit;
@@ -47,29 +48,31 @@ void WKFramePolicyListenerUse(WKFramePolicyListenerRef policyListenerRef)
 
 void WKFramePolicyListenerUseInNewProcess(WKFramePolicyListenerRef policyListenerRef)
 {
-    toImpl(policyListenerRef)->setApplyPolicyInNewProcessIfPossible(true);
-    toImpl(policyListenerRef)->use(std::nullopt);
+    toImpl(policyListenerRef)->use(std::nullopt, ShouldProcessSwapIfPossible::Yes);
 }
 
-void WKFramePolicyListenerUseWithPolicies(WKFramePolicyListenerRef policyListenerRef, WKWebsitePoliciesRef websitePolicies)
+static void useWithPolicies(WKFramePolicyListenerRef policyListenerRef, WKWebsitePoliciesRef websitePolicies, ShouldProcessSwapIfPossible shouldProcessSwapIfPossible)
 {
     auto data = toImpl(websitePolicies)->data();
 
     if (data.websiteDataStoreParameters) {
         auto& sessionID = data.websiteDataStoreParameters->networkSessionParameters.sessionID;
         RELEASE_ASSERT_WITH_MESSAGE(sessionID.isEphemeral() || sessionID == PAL::SessionID::defaultSessionID(), "If WebsitePolicies specifies a WebsiteDataStore, the data store's session must be default or non-persistent.");
-        RELEASE_ASSERT_WITH_MESSAGE(toImpl(policyListenerRef)->isMainFrame(), "WebsitePolicies cannot specify a WebsiteDataStore for subframe navigations.");
 
         toImpl(policyListenerRef)->changeWebsiteDataStore(toImpl(websitePolicies)->websiteDataStore()->websiteDataStore());
     }
 
-    toImpl(policyListenerRef)->use(WTFMove(data));
+    toImpl(policyListenerRef)->use(WTFMove(data), shouldProcessSwapIfPossible);
+}
+
+void WKFramePolicyListenerUseWithPolicies(WKFramePolicyListenerRef policyListenerRef, WKWebsitePoliciesRef websitePolicies)
+{
+    useWithPolicies(policyListenerRef, websitePolicies, ShouldProcessSwapIfPossible::No);
 }
 
 void WKFramePolicyListenerUseInNewProcessWithPolicies(WKFramePolicyListenerRef policyListenerRef, WKWebsitePoliciesRef websitePolicies)
 {
-    toImpl(policyListenerRef)->setApplyPolicyInNewProcessIfPossible(true);
-    WKFramePolicyListenerUseWithPolicies(policyListenerRef, websitePolicies);
+    useWithPolicies(policyListenerRef, websitePolicies, ShouldProcessSwapIfPossible::Yes);
 }
 
 void WKFramePolicyListenerDownload(WKFramePolicyListenerRef policyListenerRef)
index 2387d62..6948673 100644 (file)
@@ -551,13 +551,12 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP
         case _WKNavigationActionPolicyAllowInNewProcess:
 #pragma clang diagnostic pop
             tryAppLink(WTFMove(navigationAction), mainFrameURLString, [actionPolicy, localListener = WTFMove(localListener), data = WTFMove(data)](bool followedLinkToApp) mutable {
-                localListener->setApplyPolicyInNewProcessIfPossible(actionPolicy == _WKNavigationActionPolicyAllowInNewProcess);
                 if (followedLinkToApp) {
                     localListener->ignore();
                     return;
                 }
 
-                localListener->use(WTFMove(data));
+                localListener->use(WTFMove(data), actionPolicy == _WKNavigationActionPolicyAllowInNewProcess ? ShouldProcessSwapIfPossible::Yes : ShouldProcessSwapIfPossible::No);
             });
         
             break;
index ef50472..fea2368 100644 (file)
@@ -40,12 +40,12 @@ WebFramePolicyListenerProxy::WebFramePolicyListenerProxy(WebFrameProxy* frame, u
 {
 }
 
-void WebFramePolicyListenerProxy::receivedPolicyDecision(WebCore::PolicyAction action, std::optional<WebsitePoliciesData>&& data)
+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));
+    m_frame->receivedPolicyDecision(action, m_listenerID, m_navigation.get(), WTFMove(data), swap);
     m_frame = nullptr;
 }
 
@@ -57,32 +57,24 @@ void WebFramePolicyListenerProxy::changeWebsiteDataStore(WebsiteDataStore& websi
     m_frame->changeWebsiteDataStore(websiteDataStore);
 }
 
-bool WebFramePolicyListenerProxy::isMainFrame() const
-{
-    if (!m_frame)
-        return false;
-    
-    return m_frame->isMainFrame();
-}
-
 void WebFramePolicyListenerProxy::setNavigation(Ref<API::Navigation>&& navigation)
 {
     m_navigation = WTFMove(navigation);
 }
     
-void WebFramePolicyListenerProxy::use(std::optional<WebsitePoliciesData>&& data)
+void WebFramePolicyListenerProxy::use(std::optional<WebsitePoliciesData>&& data, ShouldProcessSwapIfPossible swap)
 {
-    receivedPolicyDecision(WebCore::PolicyAction::Use, WTFMove(data));
+    receivedPolicyDecision(WebCore::PolicyAction::Use, WTFMove(data), swap);
 }
 
 void WebFramePolicyListenerProxy::download()
 {
-    receivedPolicyDecision(WebCore::PolicyAction::Download, std::nullopt);
+    receivedPolicyDecision(WebCore::PolicyAction::Download, std::nullopt, ShouldProcessSwapIfPossible::No);
 }
 
 void WebFramePolicyListenerProxy::ignore()
 {
-    receivedPolicyDecision(WebCore::PolicyAction::Ignore, std::nullopt);
+    receivedPolicyDecision(WebCore::PolicyAction::Ignore, std::nullopt, ShouldProcessSwapIfPossible::No);
 }
 
 } // namespace WebKit
index 6153c26..fc34e92 100644 (file)
@@ -51,6 +51,8 @@ enum class PolicyListenerType {
     Response,
 };
 
+enum class ShouldProcessSwapIfPossible { No, Yes };
+
 class WebFramePolicyListenerProxy : public API::ObjectImpl<API::Object::Type::FramePolicyListener> {
 public:
 
@@ -59,7 +61,7 @@ public:
         return adoptRef(*new WebFramePolicyListenerProxy(frame, listenerID, policyType));
     }
 
-    void use(std::optional<WebsitePoliciesData>&&);
+    void use(std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible = ShouldProcessSwapIfPossible::No);
     void download();
     void ignore();
 
@@ -70,21 +72,16 @@ public:
     void setNavigation(Ref<API::Navigation>&&);
     
     void changeWebsiteDataStore(WebsiteDataStore&);
-    bool isMainFrame() const;
-
-    void setApplyPolicyInNewProcessIfPossible(bool applyPolicyInNewProcessIfPossible) { m_applyPolicyInNewProcessIfPossible = applyPolicyInNewProcessIfPossible; }
-    bool applyPolicyInNewProcessIfPossible() const { return m_applyPolicyInNewProcessIfPossible; }
 
 private:
     WebFramePolicyListenerProxy(WebFrameProxy*, uint64_t listenerID, PolicyListenerType);
 
-    void receivedPolicyDecision(WebCore::PolicyAction, std::optional<WebsitePoliciesData>&&);
+    void receivedPolicyDecision(WebCore::PolicyAction, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
 
     PolicyListenerType m_policyType;
     RefPtr<WebFrameProxy> m_frame;
     uint64_t m_listenerID { 0 };
     RefPtr<API::Navigation> m_navigation;
-    bool m_applyPolicyInNewProcessIfPossible { false };
 };
 
 } // namespace WebKit
index 4a778c4..9dc07cf 100644 (file)
@@ -178,14 +178,14 @@ void WebFrameProxy::didChangeTitle(const String& title)
     m_title = title;
 }
 
-void WebFrameProxy::receivedPolicyDecision(PolicyAction action, uint64_t listenerID, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& data)
+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));
+    m_page->receivedPolicyDecision(action, *this, listenerID, navigation, WTFMove(data), swap);
 }
 
 WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(uint64_t listenerID, PolicyListenerType policyListenerType)
index 1d7d48a..fb44212 100644 (file)
@@ -52,6 +52,7 @@ class WebCertificateInfo;
 class WebFramePolicyListenerProxy;
 class WebPageProxy;
 class WebsiteDataStore;
+enum class ShouldProcessSwapIfPossible;
 enum class PolicyListenerType;
 struct WebsitePoliciesData;
 
@@ -116,7 +117,7 @@ public:
     void didChangeTitle(const String&);
 
     // Policy operations.
-    void receivedPolicyDecision(WebCore::PolicyAction, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&);
+    void receivedPolicyDecision(WebCore::PolicyAction, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
 
     WebFramePolicyListenerProxy& setUpPolicyListenerProxy(uint64_t listenerID, PolicyListenerType);
     WebFramePolicyListenerProxy* activePolicyListenerProxy();
index 78f5836..8dac6de 100644 (file)
@@ -2406,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;
@@ -2438,7 +2438,7 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
         ASSERT(activePolicyListener->listenerID() == listenerID);
 
         if (action == PolicyAction::Use && navigation && frame.isMainFrame()) {
-            auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, activePolicyListener->applyPolicyInNewProcessIfPossible(), action);
+            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());
@@ -4028,7 +4028,7 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
 
 #if ENABLE(CONTENT_FILTERING)
     if (frame->didHandleContentFilterUnblockNavigation(request))
-        return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, &m_navigationState->navigation(newNavigationID), { });
+        return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, &m_navigationState->navigation(newNavigationID), std::nullopt, ShouldProcessSwapIfPossible::No);
 #else
     UNUSED_PARAM(newNavigationID);
 #endif
index 97567b9..77090c1 100644 (file)
@@ -259,6 +259,8 @@ struct PrintInfo;
 struct WebPopupItem;
 struct URLSchemeTaskParameters;
 
+enum class ShouldProcessSwapIfPossible;
+
 #if USE(QUICK_LOOK)
 class QuickLookDocumentData;
 #endif
@@ -906,7 +908,7 @@ public:
     void performDictionaryLookupOfCurrentSelection();
 #endif
 
-    void receivedPolicyDecision(WebCore::PolicyAction, WebFrameProxy&, uint64_t listenerID, API::Navigation* navigationID, std::optional<WebsitePoliciesData>&&);
+    void receivedPolicyDecision(WebCore::PolicyAction, WebFrameProxy&, uint64_t listenerID, API::Navigation*, std::optional<WebsitePoliciesData>&&, ShouldProcessSwapIfPossible);
 
     void backForwardRemovedItem(const WebCore::BackForwardItemIdentifier&);
 
index fec2c72..f2159ce 100644 (file)
@@ -2107,7 +2107,7 @@ void WebProcessPool::removeProcessFromOriginCacheSet(WebProcessProxy& process)
         m_swappedProcesses.remove(origin);
 }
 
-Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, bool shouldProcessSwapIfPossible, PolicyAction& action)
+Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, ShouldProcessSwapIfPossible shouldProcessSwapIfPossible, PolicyAction& action)
 {
     auto process = processForNavigationInternal(page, navigation, shouldProcessSwapIfPossible, action);
 
@@ -2125,9 +2125,9 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, co
     return process;
 }
 
-Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, bool shouldProcessSwapIfPossible, PolicyAction& action)
+Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, ShouldProcessSwapIfPossible shouldProcessSwapIfPossible, PolicyAction& action)
 {
-    if (!m_configuration->processSwapsOnNavigation() && !shouldProcessSwapIfPossible)
+    if (!m_configuration->processSwapsOnNavigation() && shouldProcessSwapIfPossible == ShouldProcessSwapIfPossible::No)
         return page.process();
 
     if (page.inspectorFrontendCount() > 0)
@@ -2171,7 +2171,7 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
     }
 
     auto targetURL = navigation.currentRequest().url();
-    if (!shouldProcessSwapIfPossible) {
+    if (shouldProcessSwapIfPossible == ShouldProcessSwapIfPossible::No) {
         if (navigation.treatAsSameOriginNavigation())
             return page.process();
 
index 6676f73..9d7a99f 100644 (file)
@@ -110,6 +110,8 @@ int webProcessLatencyQOS();
 int webProcessThroughputQOS();
 #endif
 
+enum class ShouldProcessSwapIfPossible;
+
 class WebProcessPool final : public API::ObjectImpl<API::Object::Type::ProcessPool>, public CanMakeWeakPtr<WebProcessPool>, private IPC::MessageReceiver {
 public:
     static Ref<WebProcessPool> create(API::ProcessPoolConfiguration&);
@@ -452,7 +454,7 @@ public:
     BackgroundWebProcessToken backgroundWebProcessToken() const { return BackgroundWebProcessToken(m_backgroundWebProcessCounter.count()); }
 #endif
 
-    Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, bool shouldProcessSwapIfPossible, WebCore::PolicyAction&);
+    Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, ShouldProcessSwapIfPossible, WebCore::PolicyAction&);
     void registerSuspendedPageProxy(SuspendedPageProxy&);
     void unregisterSuspendedPageProxy(SuspendedPageProxy&);
     void didReachGoodTimeToPrewarm();
@@ -470,7 +472,7 @@ private:
     void platformInitializeWebProcess(WebProcessCreationParameters&);
     void platformInvalidateContext();
 
-    Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, bool shouldProcessSwapIfPossible, WebCore::PolicyAction&);
+    Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, ShouldProcessSwapIfPossible, WebCore::PolicyAction&);
 
     RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&);