Return app-bound sessions for instances where WKAppBoundDomains is
authorkatherine_cheney@apple.com <katherine_cheney@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Apr 2020 18:43:45 +0000 (18:43 +0000)
committerkatherine_cheney@apple.com <katherine_cheney@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Apr 2020 18:43:45 +0000 (18:43 +0000)
empty
https://bugs.webkit.org/show_bug.cgi?id=210124
<rdar://problem/61276630>

Reviewed by Brent Fulgham.

Source/WebKit:

No new tests. Behavior confirmed by existing In-App Browser Privacy
tests.

* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(WebKit::NetworkSessionCocoa::sessionWrapperForTask):
Remove the flag checking if In-App Browser Privacy is enabled. We
should return an app-bound session if WKAppBoundDomains is empty so
we no longer need to check the flag here.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::setIsNavigatingToAppBoundDomain):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
* UIProcess/WebPageProxy.h:
As described above, we no longer need to check the flag in this
instance as we are determining behavior based on the WKAppBoundDomains
list. Also moved the logic for checking an empty list to setIsNavigatingToAppBoundDomain,
so it should take an Optional (WTF::nullopt indicates an empty list).

* UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
(WebKit::WebsiteDataStore::initializeAppBoundDomains):
Use the flag to enable internal debugging for testing purposes.

* UIProcess/API/APIHTTPCookieStore.cpp:
(API::HTTPCookieStore::filterAppBoundCookies):
Flag no longer needed. This should be gated by whether the domains
list is empty or not.

Tools:

Cleaned up tests to turn the flag on at the start of each In-App
Browser Privacy test.

* TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:
(cleanUpInAppBrowserPrivacyTestSettings):
(initializeInAppBrowserPrivacyTestSettings):
(TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm

index 6cd44c9..964568a 100644 (file)
@@ -1,3 +1,39 @@
+2020-04-07  Kate Cheney  <katherine_cheney@apple.com>
+
+        Return app-bound sessions for instances where WKAppBoundDomains is
+        empty
+        https://bugs.webkit.org/show_bug.cgi?id=210124
+        <rdar://problem/61276630>
+
+        Reviewed by Brent Fulgham.
+
+        No new tests. Behavior confirmed by existing In-App Browser Privacy
+        tests.
+
+        * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+        (WebKit::NetworkSessionCocoa::sessionWrapperForTask):
+        Remove the flag checking if In-App Browser Privacy is enabled. We
+        should return an app-bound session if WKAppBoundDomains is empty so
+        we no longer need to check the flag here.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::setIsNavigatingToAppBoundDomain):
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+        * UIProcess/WebPageProxy.h:
+        As described above, we no longer need to check the flag in this
+        instance as we are determining behavior based on the WKAppBoundDomains
+        list. Also moved the logic for checking an empty list to setIsNavigatingToAppBoundDomain,
+        so it should take an Optional (WTF::nullopt indicates an empty list).
+
+        * UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
+        (WebKit::WebsiteDataStore::initializeAppBoundDomains):
+        Use the flag to enable internal debugging for testing purposes.
+
+        * UIProcess/API/APIHTTPCookieStore.cpp:
+        (API::HTTPCookieStore::filterAppBoundCookies):
+        Flag no longer needed. This should be gated by whether the domains
+        list is empty or not.
+
 2020-04-07  Per Arne Vollan  <pvollan@apple.com>
 
         [iOS] Add message to message filter in the WebContent sandbox
index 9b0104d..43df52d 100644 (file)
@@ -63,7 +63,6 @@
 #include <WebKitAdditions/NetworkSessionCocoaAdditions.h>
 #else
 #define NETWORK_SESSION_COCOA_ADDITIONS_1
-#define NETWORK_SESSION_COCOA_ADDITIONS_2 false
 #endif
 
 #import "DeviceManagementSoftLink.h"
@@ -1211,10 +1210,8 @@ SessionWrapper& NetworkSessionCocoa::sessionWrapperForTask(const WebCore::Resour
         ASSERT_NOT_REACHED();
 #endif
 
-    if (isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::Yes) {
-        if (m_isInAppBrowserPrivacyEnabled || NETWORK_SESSION_COCOA_ADDITIONS_2)
-            return appBoundSession(storedCredentialsPolicy);
-    }
+    if (isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::Yes)
+        return appBoundSession(storedCredentialsPolicy);
 
     switch (storedCredentialsPolicy) {
     case WebCore::StoredCredentialsPolicy::Use:
index 97e93cb..d0c0613 100644 (file)
@@ -38,7 +38,6 @@
 #if USE(APPLE_INTERNAL_SDK)
 #include <WebKitAdditions/HTTPCookieStoreAdditions.h>
 #else
-#define IN_APP_BROWSER_PRIVACY_ENABLED false
 #define IMPLEMENT_IN_APP_BROWSER_PRIVACY_ENABLED
 #endif
 
@@ -66,8 +65,8 @@ void HTTPCookieStore::filterAppBoundCookies(const Vector<WebCore::Cookie>& cooki
 {
     Vector<WebCore::Cookie> appBoundCookies;
 #if PLATFORM(IOS_FAMILY)
-    m_owningDataStore->getAppBoundDomains([this, protectedThis = makeRef(*this), cookies, appBoundCookies = WTFMove(appBoundCookies), completionHandler = WTFMove(completionHandler)] (auto& domains) mutable {
-        if (m_owningDataStore->parameters().networkSessionParameters.isInAppBrowserPrivacyEnabled || IN_APP_BROWSER_PRIVACY_ENABLED) {
+    m_owningDataStore->getAppBoundDomains([cookies, appBoundCookies = WTFMove(appBoundCookies), completionHandler = WTFMove(completionHandler)] (auto& domains) mutable {
+        if (!domains.isEmpty()) {
             IMPLEMENT_IN_APP_BROWSER_PRIVACY_ENABLED
             for (auto& cookie : cookies) {
                 if (domains.contains(WebCore::RegistrableDomain::uncheckedCreateFromHost(cookie.domain)))
index d1bb07e..682f2ed 100644 (file)
 #include <WebKitAdditions/WebPageProxyAdditions.h>
 #else
 #define WEB_PAGE_PROXY_ADDITIONS_SETISNAVIGATINGTOAPPBOUNDDOMAIN
-#define WEB_PAGE_PROXY_ADDITIONS_SETISNAVIGATINGTOAPPBOUNDDOMAIN_2 false
 #endif
 
 // This controls what strategy we use for mouse wheel coalescing.
@@ -3119,14 +3118,18 @@ private:
     PolicyCheckIdentifier m_identifier;
 };
 
-void WebPageProxy::setIsNavigatingToAppBoundDomain(bool isMainFrame, const URL& requestURL, NavigatingToAppBoundDomain isNavigatingToAppBoundDomain)
+void WebPageProxy::setIsNavigatingToAppBoundDomain(bool isMainFrame, const URL& requestURL, Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain)
 {
 #if PLATFORM(IOS_FAMILY)
-    if (isMainFrame && (m_preferences->isInAppBrowserPrivacyEnabled() || WEB_PAGE_PROXY_ADDITIONS_SETISNAVIGATINGTOAPPBOUNDDOMAIN_2)) {
+    if (isMainFrame) {
+        WEB_PAGE_PROXY_ADDITIONS_SETISNAVIGATINGTOAPPBOUNDDOMAIN
+        if (!isNavigatingToAppBoundDomain) {
+            m_isNavigatingToAppBoundDomain = NavigatingToAppBoundDomain::Yes;
+            return;
+        }
         if (m_ignoresAppBoundDomains)
             return;
-        WEB_PAGE_PROXY_ADDITIONS_SETISNAVIGATINGTOAPPBOUNDDOMAIN
-        if (isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::No) {
+        if (*isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::No) {
             m_configuration->setWebViewCategory(WebViewCategory::InAppBrowser);
             m_isNavigatingToAppBoundDomain = NavigatingToAppBoundDomain::No;
             m_hasNavigatedAwayFromAppBoundDomain = NavigatedAwayFromAppBoundDomain::Yes;
@@ -5105,8 +5108,8 @@ void WebPageProxy::decidePolicyForNavigationAction(Ref<WebProcessProxy>&& proces
     
     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 && isAppBoundDomain)
-            setIsNavigatingToAppBoundDomain(frame->isMainFrame(), navigation->currentRequest().url(), *isAppBoundDomain);
+        if (policyAction != PolicyAction::Ignore)
+            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()) {
index a5f328b..97136d5 100644 (file)
@@ -2282,7 +2282,7 @@ private:
     void tryCloseTimedOut();
     void makeStorageSpaceRequest(WebCore::FrameIdentifier, const String& originIdentifier, const String& databaseName, const String& displayName, uint64_t currentQuota, uint64_t currentOriginUsage, uint64_t currentDatabaseUsage, uint64_t expectedUsage, CompletionHandler<void(uint64_t)>&&);
         
-    void setIsNavigatingToAppBoundDomain(bool isMainFrame, const URL&, NavigatingToAppBoundDomain);
+    void setIsNavigatingToAppBoundDomain(bool isMainFrame, const URL&, Optional<NavigatingToAppBoundDomain>);
     NavigatedAwayFromAppBoundDomain hasNavigatedAwayFromAppBoundDomain() const { return m_hasNavigatedAwayFromAppBoundDomain; }
         
     const Identifier m_identifier;
index 7b6d8cb..9ab7d97 100644 (file)
@@ -403,13 +403,13 @@ void WebsiteDataStore::initializeAppBoundDomains(ForceReinitialization forceRein
     
     static const auto maxAppBoundDomainCount = 10;
     
-    appBoundDomainQueue().dispatch([forceReinitialization] () mutable {
+    appBoundDomainQueue().dispatch([isInAppBrowserPrivacyEnabled = parameters().networkSessionParameters.isInAppBrowserPrivacyEnabled, forceReinitialization] () mutable {
         if (hasInitializedAppBoundDomains && forceReinitialization != ForceReinitialization::Yes)
             return;
         
         NSArray<NSString *> *domains = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"WKAppBoundDomains"];
         
-        RunLoop::main().dispatch([forceReinitialization , domains = retainPtr(domains)] {
+        RunLoop::main().dispatch([isInAppBrowserPrivacyEnabled, forceReinitialization, domains = retainPtr(domains)] {
             if (forceReinitialization == ForceReinitialization::Yes)
                 appBoundDomains().clear();
 
@@ -426,7 +426,8 @@ void WebsiteDataStore::initializeAppBoundDomains(ForceReinitialization forceRein
                 if (appBoundDomains().size() >= maxAppBoundDomainCount)
                     break;
             }
-            WEBSITE_DATA_STORE_ADDITIONS
+            if (isInAppBrowserPrivacyEnabled)
+                WEBSITE_DATA_STORE_ADDITIONS
             hasInitializedAppBoundDomains = true;
         });
     });
index 38d1055..4dffd99 100644 (file)
@@ -1,3 +1,20 @@
+2020-04-07  Kate Cheney  <katherine_cheney@apple.com>
+
+        Return app-bound sessions for instances where WKAppBoundDomains is
+        empty
+        https://bugs.webkit.org/show_bug.cgi?id=210124
+        <rdar://problem/61276630>
+
+        Reviewed by Brent Fulgham.
+
+        Cleaned up tests to turn the flag on at the start of each In-App
+        Browser Privacy test.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:
+        (cleanUpInAppBrowserPrivacyTestSettings):
+        (initializeInAppBrowserPrivacyTestSettings):
+        (TEST):
+
 2020-04-07  Timothy Hatcher  <timothy@apple.com>
 
         WKUserScripts deferred from injection are not injected if -[WKWebView _notifyUserScripts] is called early.
index 37f8a11..9542ab4 100644 (file)
@@ -81,11 +81,13 @@ static NSString * const userScriptSource = @"window.wkUserScriptInjected = true"
 
 static void cleanUpInAppBrowserPrivacyTestSettings()
 {
+    [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
     IN_APP_BROWSER_PRIVACY_ADDITIONS_2
 }
 
 static void initializeInAppBrowserPrivacyTestSettings()
 {
+    [[NSUserDefaults standardUserDefaults] setBool:YES forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
     RunLoop::initializeMainRunLoop();
     WebCore::clearApplicationBundleIdentifierTestingOverride();
     IN_APP_BROWSER_PRIVACY_ADDITIONS
@@ -536,7 +538,6 @@ static void setUpCookieTest()
 TEST(InAppBrowserPrivacy, SetCookieForNonAppBoundDomainFails)
 {
     initializeInAppBrowserPrivacyTestSettings();
-    [[NSUserDefaults standardUserDefaults] setBool:YES forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
 
     auto dataStore = [WKWebsiteDataStore defaultDataStore];
     auto webView = adoptNS([TestWKWebView new]);
@@ -581,7 +582,6 @@ TEST(InAppBrowserPrivacy, SetCookieForNonAppBoundDomainFails)
     TestWebKitAPI::Util::run(&gotFlag);
 
     cleanUpInAppBrowserPrivacyTestSettings();
-    [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
     gotFlag = false;
 
     // Check the cookie store to make sure only one cookie was set.
@@ -607,7 +607,7 @@ TEST(InAppBrowserPrivacy, GetCookieForNonAppBoundDomainFails)
 {
     // Since we can't set non-app-bound cookies with In-App Browser privacy protections on,
     // we can turn the protections off to set a cookie we will then try to get with protections enabled.
-    [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
+    cleanUpInAppBrowserPrivacyTestSettings();
 
     setUpCookieTest();
     globalCookieStore = [[WKWebsiteDataStore defaultDataStore] httpCookieStore];
@@ -658,7 +658,6 @@ TEST(InAppBrowserPrivacy, GetCookieForNonAppBoundDomainFails)
     ASSERT_EQ(cookies.count, 2u);
 
     // Now enable protections and ensure we can only retrieve the app-bound cookies.
-    [[NSUserDefaults standardUserDefaults] setBool:YES forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
     initializeInAppBrowserPrivacyTestSettings();
 
     gotFlag = false;
@@ -683,7 +682,6 @@ TEST(InAppBrowserPrivacy, GetCookieForNonAppBoundDomainFails)
     gotFlag = false;
     [globalCookieStore deleteCookie:appBoundCookie.get() completionHandler:[]() {
         // Reset flag.
-        [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
         cleanUpInAppBrowserPrivacyTestSettings();
         gotFlag = true;
     }];
@@ -695,7 +693,7 @@ TEST(InAppBrowserPrivacy, GetCookieForURLFails)
 {
     // Since we can't set non-app-bound cookies with In-App Browser privacy protections on,
     // we can turn the protections off to set a cookie we will then try to get with protections enabled.
-    [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
+    cleanUpInAppBrowserPrivacyTestSettings();
     setUpCookieTest();
 
     globalCookieStore = [[WKWebsiteDataStore defaultDataStore] httpCookieStore];
@@ -720,7 +718,6 @@ TEST(InAppBrowserPrivacy, GetCookieForURLFails)
         [globalCookieStore setCookie:appBoundCookie completionHandler:^{
 
             // Now enable protections and ensure we can only retrieve the app-bound cookies.
-            [[NSUserDefaults standardUserDefaults] setBool:YES forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
             initializeInAppBrowserPrivacyTestSettings();
 
             [globalCookieStore _getCookiesForURL:[NSURL URLWithString:@"https://webkit.org/"] completionHandler:^(NSArray<NSHTTPCookie *> *cookies) {
@@ -730,7 +727,6 @@ TEST(InAppBrowserPrivacy, GetCookieForURLFails)
                     EXPECT_EQ(cookies.count, 0u);
                     [globalCookieStore deleteCookie:nonAppBoundCookie completionHandler:^{
                         [globalCookieStore deleteCookie:appBoundCookie completionHandler:^{
-                            [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
                             cleanUpInAppBrowserPrivacyTestSettings();
                             done = true;
                         }];