Create a way to signal if the WKAppBoundDomains list is empty
authorkatherine_cheney@apple.com <katherine_cheney@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Apr 2020 00:11:43 +0000 (00:11 +0000)
committerkatherine_cheney@apple.com <katherine_cheney@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Apr 2020 00:11:43 +0000 (00:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=210074
<rdar://problem/61359228>

Reviewed by Brent Fulgham.

Updates the WebFramePolicyListener to return an Optional<NavigatingToAppBoundDomain>
to signal if the WKAppBoundDomains list is empty. If so, we don't want to update
any app-bound domain parameters in WebPageProxy.

* UIProcess/WebFramePolicyListenerProxy.cpp:
(WebKit::WebFramePolicyListenerProxy::didReceiveAppBoundDomainResult):
* UIProcess/WebFramePolicyListenerProxy.h:
* UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::setUpPolicyListenerProxy):
* UIProcess/WebFrameProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::decidePolicyForNewWindowAction):
(WebKit::WebPageProxy::decidePolicyForResponseShared):
* UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
(WebKit::WebsiteDataStore::beginAppBoundDomainCheck):
Changed the WebFramePolicyListener to take a NavigatingToAppBoundDomain
type as opposed to a boolean to allow it to handle the empty value.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@259615 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/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm

index 505c1c5..bd46c2b 100644 (file)
@@ -1,3 +1,30 @@
+2020-04-06  Kate Cheney  <katherine_cheney@apple.com>
+
+        Create a way to signal if the WKAppBoundDomains list is empty
+        https://bugs.webkit.org/show_bug.cgi?id=210074
+        <rdar://problem/61359228>
+
+        Reviewed by Brent Fulgham.
+
+        Updates the WebFramePolicyListener to return an Optional<NavigatingToAppBoundDomain>
+        to signal if the WKAppBoundDomains list is empty. If so, we don't want to update
+        any app-bound domain parameters in WebPageProxy.
+
+        * UIProcess/WebFramePolicyListenerProxy.cpp:
+        (WebKit::WebFramePolicyListenerProxy::didReceiveAppBoundDomainResult):
+        * UIProcess/WebFramePolicyListenerProxy.h:
+        * UIProcess/WebFrameProxy.cpp:
+        (WebKit::WebFrameProxy::setUpPolicyListenerProxy):
+        * UIProcess/WebFrameProxy.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+        (WebKit::WebPageProxy::decidePolicyForNewWindowAction):
+        (WebKit::WebPageProxy::decidePolicyForResponseShared):
+        * UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
+        (WebKit::WebsiteDataStore::beginAppBoundDomainCheck):
+        Changed the WebFramePolicyListener to take a NavigatingToAppBoundDomain
+        type as opposed to a boolean to allow it to handle the empty value.
+
 2020-04-06  Chris Dumez  <cdumez@apple.com>
 
         [iOS] Transition most process assertions to RunningBoard
index cb0444e..01a1acf 100644 (file)
@@ -46,16 +46,15 @@ WebFramePolicyListenerProxy::WebFramePolicyListenerProxy(Reply&& reply, ShouldEx
 
 WebFramePolicyListenerProxy::~WebFramePolicyListenerProxy() = default;
 
-void WebFramePolicyListenerProxy::didReceiveAppBoundDomainResult(bool isNavigatingToAppBoundDomain)
+void WebFramePolicyListenerProxy::didReceiveAppBoundDomainResult(Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain)
 {
     ASSERT(RunLoop::isMain());
 
-    auto isAppBound = isNavigatingToAppBoundDomain ? NavigatingToAppBoundDomain::Yes : NavigatingToAppBoundDomain::No;
     if (m_policyResult && m_safeBrowsingWarning) {
         if (m_reply)
-            m_reply(WebCore::PolicyAction::Use, m_policyResult->first.get(), m_policyResult->second, WTFMove(*m_safeBrowsingWarning), isAppBound);
+            m_reply(WebCore::PolicyAction::Use, m_policyResult->first.get(), m_policyResult->second, WTFMove(*m_safeBrowsingWarning), isNavigatingToAppBoundDomain);
     } else
-        m_isNavigatingToAppBoundDomain = isAppBound;
+        m_isNavigatingToAppBoundDomain = isNavigatingToAppBoundDomain;
 }
 
 void WebFramePolicyListenerProxy::didReceiveSafeBrowsingResults(RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning)
index 5bfae9f..895830c 100644 (file)
@@ -46,7 +46,7 @@ enum class ShouldExpectAppBoundDomainResult : bool { No, Yes };
 class WebFramePolicyListenerProxy : public API::ObjectImpl<API::Object::Type::FramePolicyListener> {
 public:
 
-    using Reply = CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&&, NavigatingToAppBoundDomain)>;
+    using Reply = CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&&, Optional<NavigatingToAppBoundDomain>)>;
     static Ref<WebFramePolicyListenerProxy> create(Reply&& reply, ShouldExpectSafeBrowsingResult expectSafeBrowsingResult, ShouldExpectAppBoundDomainResult expectAppBoundDomainResult)
     {
         return adoptRef(*new WebFramePolicyListenerProxy(WTFMove(reply), expectSafeBrowsingResult, expectAppBoundDomainResult));
@@ -58,14 +58,14 @@ public:
     void ignore();
     
     void didReceiveSafeBrowsingResults(RefPtr<SafeBrowsingWarning>&&);
-    void didReceiveAppBoundDomainResult(bool);
+    void didReceiveAppBoundDomainResult(Optional<NavigatingToAppBoundDomain>);
 
 private:
     WebFramePolicyListenerProxy(Reply&&, ShouldExpectSafeBrowsingResult, ShouldExpectAppBoundDomainResult);
 
     Optional<std::pair<RefPtr<API::WebsitePolicies>, ProcessSwapRequestedByClient>> m_policyResult;
     Optional<RefPtr<SafeBrowsingWarning>> m_safeBrowsingWarning;
-    Optional<NavigatingToAppBoundDomain> m_isNavigatingToAppBoundDomain;
+    Optional<Optional<NavigatingToAppBoundDomain>> m_isNavigatingToAppBoundDomain;
     Reply m_reply;
 };
 
index 346c080..a5e3462 100644 (file)
@@ -193,11 +193,11 @@ void WebFrameProxy::didChangeTitle(const String& title)
     m_title = title;
 }
 
-WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(CompletionHandler<void(PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&&, NavigatingToAppBoundDomain)>&& completionHandler, ShouldExpectSafeBrowsingResult expectSafeBrowsingResult, ShouldExpectAppBoundDomainResult expectAppBoundDomainResult)
+WebFramePolicyListenerProxy& WebFrameProxy::setUpPolicyListenerProxy(CompletionHandler<void(PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&&, Optional<NavigatingToAppBoundDomain>)>&& completionHandler, ShouldExpectSafeBrowsingResult expectSafeBrowsingResult, ShouldExpectAppBoundDomainResult expectAppBoundDomainResult)
 {
     if (m_activeListener)
         m_activeListener->ignore();
-    m_activeListener = WebFramePolicyListenerProxy::create([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (PolicyAction action, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, NavigatingToAppBoundDomain isNavigatingToAppBoundDomain) mutable {
+    m_activeListener = WebFramePolicyListenerProxy::create([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (PolicyAction action, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain) mutable {
         completionHandler(action, policies, processSwapRequestedByClient, WTFMove(safeBrowsingWarning), isNavigatingToAppBoundDomain);
         m_activeListener = nullptr;
     }, expectSafeBrowsingResult, expectAppBoundDomainResult);
index 601f404..5eae22b 100644 (file)
@@ -120,7 +120,7 @@ public:
     void didSameDocumentNavigation(const URL&); // eg. anchor navigation, session state change.
     void didChangeTitle(const String&);
 
-    WebFramePolicyListenerProxy& setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&&, NavigatingToAppBoundDomain)>&&, ShouldExpectSafeBrowsingResult, ShouldExpectAppBoundDomainResult);
+    WebFramePolicyListenerProxy& setUpPolicyListenerProxy(CompletionHandler<void(WebCore::PolicyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&&, Optional<NavigatingToAppBoundDomain>)>&&, ShouldExpectSafeBrowsingResult, ShouldExpectAppBoundDomainResult);
 
 #if ENABLE(CONTENT_FILTERING)
     void contentFilterDidBlockLoad(WebCore::ContentFilterUnblockHandler contentFilterUnblockHandler) { m_contentFilterUnblockHandler = WTFMove(contentFilterUnblockHandler); }
index 7f32e34..d1bb07e 100644 (file)
@@ -5103,10 +5103,10 @@ void WebPageProxy::decidePolicyForNavigationAction(Ref<WebProcessProxy>&& proces
     shouldExpectAppBoundDomainResult = ShouldExpectAppBoundDomainResult::Yes;
 #endif
     
-    auto listener = makeRef(frame.setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(frame), sender = WTFMove(sender), navigation] (PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, NavigatingToAppBoundDomain isAppBoundDomain) mutable {
+    auto listener = makeRef(frame.setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(frame), sender = WTFMove(sender), navigation] (PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, Optional<NavigatingToAppBoundDomain> isAppBoundDomain) mutable {
 
-        if (policyAction != PolicyAction::Ignore)
-            setIsNavigatingToAppBoundDomain(frame->isMainFrame(), navigation->currentRequest().url(), isAppBoundDomain);
+        if (policyAction != PolicyAction::Ignore && isAppBoundDomain)
+            setIsNavigatingToAppBoundDomain(frame->isMainFrame(), navigation->currentRequest().url(), *isAppBoundDomain);
 
         auto completionHandler = [this, protectedThis = protectedThis.copyRef(), frame = frame.copyRef(), sender = WTFMove(sender), navigation, processSwapRequestedByClient, policies = makeRefPtr(policies)] (PolicyAction policyAction) mutable {
             if (frame->isMainFrame()) {
@@ -5290,7 +5290,7 @@ void WebPageProxy::decidePolicyForNewWindowAction(FrameIdentifier frameID, Frame
     MESSAGE_CHECK(m_process, frame);
     MESSAGE_CHECK_URL(m_process, request.url());
 
-    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), identifier, listenerID, frameID] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, NavigatingToAppBoundDomain isNavigatingToAppBoundDomain) mutable {
+    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), identifier, listenerID, frameID] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain) mutable {
         // FIXME: Assert the API::WebsitePolicies* is nullptr here once clients of WKFramePolicyListenerUseWithPolicies go away.
         RELEASE_ASSERT(processSwapRequestedByClient == ProcessSwapRequestedByClient::No);
         ASSERT_UNUSED(safeBrowsingWarning, !safeBrowsingWarning);
@@ -5338,7 +5338,7 @@ void WebPageProxy::decidePolicyForResponseShared(Ref<WebProcessProxy>&& process,
     MESSAGE_CHECK_URL(process, response.url());
     RefPtr<API::Navigation> navigation = navigationID ? m_navigationState->navigation(navigationID) : nullptr;
     auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), webPageID, frameID, identifier, listenerID, navigation = WTFMove(navigation),
-        process = process.copyRef()] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, NavigatingToAppBoundDomain isNavigatingToAppBoundDomain) mutable {
+        process = process.copyRef()] (PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain) mutable {
         // FIXME: Assert the API::WebsitePolicies* is nullptr here once clients of WKFramePolicyListenerUseWithPolicies go away.
         RELEASE_ASSERT(processSwapRequestedByClient == ProcessSwapRequestedByClient::No);
         ASSERT_UNUSED(safeBrowsingWarning, !safeBrowsingWarning);
index 22c98ef..7b6d8cb 100644 (file)
@@ -457,12 +457,16 @@ void WebsiteDataStore::beginAppBoundDomainCheck(const URL& requestURL, WebFrameP
     ASSERT(RunLoop::isMain());
 
     if (shouldTreatURLProtocolAsAppBound(requestURL)) {
-        listener.didReceiveAppBoundDomainResult(true);
+        listener.didReceiveAppBoundDomainResult(NavigatingToAppBoundDomain::Yes);
         return;
     }
 
     ensureAppBoundDomains([domain = WebCore::RegistrableDomain(requestURL), listener = makeRef(listener)] (auto& domains) mutable {
-        listener->didReceiveAppBoundDomainResult(domains.contains(domain));
+        if (domains.isEmpty()) {
+            listener->didReceiveAppBoundDomainResult(WTF::nullopt);
+            return;
+        }
+        listener->didReceiveAppBoundDomainResult(domains.contains(domain) ? NavigatingToAppBoundDomain::Yes : NavigatingToAppBoundDomain::No);
     });
 }