REGRESSION (r249923): ASSERTION FAILED: sessionID == WebProcess::singleton().sessionI...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Sep 2019 17:12:04 +0000 (17:12 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Sep 2019 17:12:04 +0000 (17:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201859
<rdar://problem/55426742>

Reviewed by Alex Christensen.

Source/WebKit:

Drop support for the WKPreferencesSetPrivateBrowsingEnabled() C API (Mark as deprecated and make it a no-op) as it
was changing a WebContent process's sessionID, which is no longer supported. This was also using the legacy private
browsing session, which we're trying to get rid of. There is suitable C API to do private browsing (WKWebsiteDataStoreRef).

* Shared/WebPreferences.yaml:
* UIProcess/API/C/WKPreferences.cpp:
(WKPreferencesSetPrivateBrowsingEnabled):
(WKPreferencesGetPrivateBrowsingEnabled):
* UIProcess/API/C/WKPreferencesRef.h:
* UIProcess/API/C/mac/WKPagePrivateMac.mm:
(WKPageIsURLKnownHSTSHost):
* UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::isURLKnownHSTSHost const):
* UIProcess/WebPreferences.cpp:
(WebKit::WebPreferences::addPage):
(WebKit::WebPreferences::removePage):
(WebKit::WebPreferences::updateBoolValueForKey):
* UIProcess/WebPreferences.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::ensureNetworkProcess):
* UIProcess/WebProcessPool.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::updatePreferences):

Tools:

Update API test to use WKWebsiteDataStoreRef C API to do private browsing instead of using
the legacy WKPreferencesSetPrivateBrowsingEnabled() C API.

* TestWebKitAPI/Tests/WebKit/PrivateBrowsingPushStateNoHistoryCallback.cpp:
(TestWebKitAPI::TEST):

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

15 files changed:
Source/WebKit/ChangeLog
Source/WebKit/Shared/WebPreferences.yaml
Source/WebKit/UIProcess/API/C/WKPreferences.cpp
Source/WebKit/UIProcess/API/C/WKPreferencesRef.h
Source/WebKit/UIProcess/API/C/mac/WKPagePrivateMac.mm
Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp
Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp
Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm
Source/WebKit/UIProcess/WebPreferences.cpp
Source/WebKit/UIProcess/WebPreferences.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit/PrivateBrowsingPushStateNoHistoryCallback.cpp

index 2b7cbd9..bac305a 100644 (file)
@@ -1,5 +1,37 @@
 2019-09-17  Chris Dumez  <cdumez@apple.com>
 
+        REGRESSION (r249923): ASSERTION FAILED: sessionID == WebProcess::singleton().sessionID() in WebCore::SWClientConnection *WebKit::WebServiceWorkerProvider::existingServiceWorkerConnectionForSession(PAL::SessionID)
+        https://bugs.webkit.org/show_bug.cgi?id=201859
+        <rdar://problem/55426742>
+
+        Reviewed by Alex Christensen.
+
+        Drop support for the WKPreferencesSetPrivateBrowsingEnabled() C API (Mark as deprecated and make it a no-op) as it
+        was changing a WebContent process's sessionID, which is no longer supported. This was also using the legacy private
+        browsing session, which we're trying to get rid of. There is suitable C API to do private browsing (WKWebsiteDataStoreRef).
+
+        * Shared/WebPreferences.yaml:
+        * UIProcess/API/C/WKPreferences.cpp:
+        (WKPreferencesSetPrivateBrowsingEnabled):
+        (WKPreferencesGetPrivateBrowsingEnabled):
+        * UIProcess/API/C/WKPreferencesRef.h:
+        * UIProcess/API/C/mac/WKPagePrivateMac.mm:
+        (WKPageIsURLKnownHSTSHost):
+        * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
+        (WebKit::WebProcessPool::isURLKnownHSTSHost const):
+        * UIProcess/WebPreferences.cpp:
+        (WebKit::WebPreferences::addPage):
+        (WebKit::WebPreferences::removePage):
+        (WebKit::WebPreferences::updateBoolValueForKey):
+        * UIProcess/WebPreferences.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::ensureNetworkProcess):
+        * UIProcess/WebProcessPool.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::updatePreferences):
+
+2019-09-17  Chris Dumez  <cdumez@apple.com>
+
         REGRESSION (iOS 13): rAF stops firing when navigating away cross-origin and then back
         https://bugs.webkit.org/show_bug.cgi?id=201767
         <rdar://problem/55350854>
index c810ce4..65f9de7 100644 (file)
@@ -92,11 +92,6 @@ SafeBrowsingEnabled:
   defaultValue: true
   webcoreBinding: none
 
-PrivateBrowsingEnabled:
-  type: bool
-  defaultValue: false
-  webcoreBinding: none
-
 TextAreasAreResizable:
   type: bool
   defaultValue: DEFAULT_TEXT_AREAS_ARE_RESIZABLE
index d8d9ba8..5e7f523 100644 (file)
@@ -351,12 +351,11 @@ WKStringRef WKPreferencesCopyDefaultTextEncodingName(WKPreferencesRef preference
 
 void WKPreferencesSetPrivateBrowsingEnabled(WKPreferencesRef preferencesRef, bool enabled)
 {
-    toImpl(preferencesRef)->setPrivateBrowsingEnabled(enabled);
 }
 
 bool WKPreferencesGetPrivateBrowsingEnabled(WKPreferencesRef preferencesRef)
 {
-    return toImpl(preferencesRef)->privateBrowsingEnabled();
+    return false;
 }
 
 void WKPreferencesSetDeveloperExtrasEnabled(WKPreferencesRef preferencesRef, bool enabled)
index ca871ff..27553f9 100644 (file)
@@ -149,8 +149,8 @@ WK_EXPORT void WKPreferencesSetDefaultTextEncodingName(WKPreferencesRef preferen
 WK_EXPORT WKStringRef WKPreferencesCopyDefaultTextEncodingName(WKPreferencesRef preferencesRef);
 
 // Defaults to false.
-WK_EXPORT void WKPreferencesSetPrivateBrowsingEnabled(WKPreferencesRef preferencesRef, bool enabled);
-WK_EXPORT bool WKPreferencesGetPrivateBrowsingEnabled(WKPreferencesRef preferencesRef);
+WK_EXPORT void WKPreferencesSetPrivateBrowsingEnabled(WKPreferencesRef preferencesRef, bool enabled) WK_C_API_DEPRECATED;
+WK_EXPORT bool WKPreferencesGetPrivateBrowsingEnabled(WKPreferencesRef preferencesRef) WK_C_API_DEPRECATED;
 
 // Defaults to false.
 WK_EXPORT void WKPreferencesSetDeveloperExtrasEnabled(WKPreferencesRef preferencesRef, bool enabled);
index 35ef922..17398ec 100644 (file)
@@ -132,9 +132,8 @@ _WKRemoteObjectRegistry *WKPageGetObjectRegistry(WKPageRef pageRef)
 bool WKPageIsURLKnownHSTSHost(WKPageRef page, WKURLRef url)
 {
     WebKit::WebPageProxy* webPageProxy = WebKit::toImpl(page);
-    bool privateBrowsingEnabled = webPageProxy->pageGroup().preferences().privateBrowsingEnabled();
 
-    return webPageProxy->process().processPool().isURLKnownHSTSHost(WebKit::toImpl(url)->string(), privateBrowsingEnabled);
+    return webPageProxy->process().processPool().isURLKnownHSTSHost(WebKit::toImpl(url)->string());
 }
 
 WKNavigation *WKPageLoadURLRequestReturningNavigation(WKPageRef pageRef, WKURLRequestRef urlRequestRef)
index 9b5818a..28e6620 100644 (file)
@@ -271,7 +271,6 @@ void webkitFaviconDatabaseOpen(WebKitFaviconDatabase* database, const String& pa
     priv->iconDatabase->setClient(makeUnique<WebKitIconDatabaseClient>(database));
     IconDatabase::delayDatabaseCleanup();
     priv->iconDatabase->setEnabled(true);
-    priv->iconDatabase->setPrivateBrowsingEnabled(WebPreferences::anyPagesAreUsingPrivateBrowsing());
 
     if (!priv->iconDatabase->open(FileSystem::directoryName(path), FileSystem::pathGetFileName(path))) {
         priv->iconDatabase = nullptr;
index 6189747..3c832e5 100644 (file)
@@ -2391,7 +2391,7 @@ gboolean webkit_settings_get_enable_private_browsing(WebKitSettings* settings)
 {
     g_return_val_if_fail(WEBKIT_IS_SETTINGS(settings), FALSE);
 
-    return settings->priv->preferences->privateBrowsingEnabled();
+    return FALSE;
 }
 
 /**
@@ -2406,14 +2406,6 @@ gboolean webkit_settings_get_enable_private_browsing(WebKitSettings* settings)
 void webkit_settings_set_enable_private_browsing(WebKitSettings* settings, gboolean enabled)
 {
     g_return_if_fail(WEBKIT_IS_SETTINGS(settings));
-
-    WebKitSettingsPrivate* priv = settings->priv;
-    bool currentValue = priv->preferences->privateBrowsingEnabled();
-    if (currentValue == enabled)
-        return;
-
-    priv->preferences->setPrivateBrowsingEnabled(enabled);
-    g_object_notify(G_OBJECT(settings), "enable-private-browsing");
 }
 #endif
 
index 4fc5092..510ecb0 100644 (file)
@@ -489,11 +489,11 @@ static CFURLStorageSessionRef privateBrowsingSession()
     return session;
 }
 
-bool WebProcessPool::isURLKnownHSTSHost(const String& urlString, bool privateBrowsingEnabled) const
+bool WebProcessPool::isURLKnownHSTSHost(const String& urlString) const
 {
     RetainPtr<CFURLRef> url = URL(URL(), urlString).createCFURL();
 
-    return _CFNetworkIsKnownHSTSHostWithSession(url.get(), privateBrowsingEnabled ? privateBrowsingSession() : nullptr);
+    return _CFNetworkIsKnownHSTSHostWithSession(url.get(), nullptr);
 }
 
 void WebProcessPool::resetHSTSHosts()
index 2664ba0..5d81047 100644 (file)
 
 namespace WebKit {
 
-// FIXME: Manipulating this variable is not thread safe.
-// Instead of tracking private browsing state as a boolean preference, we should let the client provide storage sessions explicitly.
-static unsigned privateBrowsingPageCount;
-
 Ref<WebPreferences> WebPreferences::create(const String& identifier, const String& keyPrefix, const String& globalDebugKeyPrefix)
 {
     return adoptRef(*new WebPreferences(identifier, keyPrefix, globalDebugKeyPrefix));
@@ -88,25 +84,12 @@ void WebPreferences::addPage(WebPageProxy& webPageProxy)
 {
     ASSERT(!m_pages.contains(&webPageProxy));
     m_pages.add(&webPageProxy);
-
-    if (privateBrowsingEnabled()) {
-        if (!privateBrowsingPageCount)
-            WebProcessPool::willStartUsingPrivateBrowsing();
-
-        ++privateBrowsingPageCount;
-    }
 }
 
 void WebPreferences::removePage(WebPageProxy& webPageProxy)
 {
     ASSERT(m_pages.contains(&webPageProxy));
     m_pages.remove(&webPageProxy);
-
-    if (privateBrowsingEnabled()) {
-        --privateBrowsingPageCount;
-        if (!privateBrowsingPageCount)
-            WebProcessPool::willStopUsingPrivateBrowsing();
-    }
 }
 
 void WebPreferences::update()
@@ -123,11 +106,6 @@ void WebPreferences::updateStringValueForKey(const String& key, const String& va
 
 void WebPreferences::updateBoolValueForKey(const String& key, bool value)
 {
-    if (key == WebPreferencesKey::privateBrowsingEnabledKey()) {
-        updatePrivateBrowsingValue(value);
-        return;
-    }
-
     platformUpdateBoolValueForKey(key, value);
     update(); // FIXME: Only send over the changed key and value.
 }
@@ -186,30 +164,6 @@ void WebPreferences::deleteKey(const String& key)
     update(); // FIXME: Only send over the changed key and value.
 }
 
-void WebPreferences::updatePrivateBrowsingValue(bool value)
-{
-    platformUpdateBoolValueForKey(WebPreferencesKey::privateBrowsingEnabledKey(), value);
-
-    unsigned pagesChanged = m_pages.size();
-    if (!pagesChanged)
-        return;
-
-    if (value) {
-        if (!privateBrowsingPageCount)
-            WebProcessPool::willStartUsingPrivateBrowsing();
-        privateBrowsingPageCount += pagesChanged;
-    }
-
-    update(); // FIXME: Only send over the changed key and value.
-
-    if (!value) {
-        ASSERT(privateBrowsingPageCount >= pagesChanged);
-        privateBrowsingPageCount -= pagesChanged;
-        if (!privateBrowsingPageCount)
-            WebProcessPool::willStopUsingPrivateBrowsing();
-    }
-}
-
 #define DEFINE_PREFERENCE_GETTER_AND_SETTERS(KeyUpper, KeyLower, TypeName, Type, DefaultValue, HumanReadableName, HumanReadableDescription) \
     void WebPreferences::set##KeyUpper(const Type& value) \
     { \
@@ -234,11 +188,6 @@ FOR_EACH_WEBKIT_DEBUG_PREFERENCE(DEFINE_PREFERENCE_GETTER_AND_SETTERS)
 #undef DEFINE_PREFERENCE_GETTER_AND_SETTERS
 
 
-bool WebPreferences::anyPagesAreUsingPrivateBrowsing()
-{
-    return privateBrowsingPageCount;
-}
-
 void WebPreferences::registerDefaultBoolValueForKey(const String& key, bool value)
 {
     m_store.setOverrideDefaultsBoolValueForKey(key, value);
index 12061f6..effc691 100644 (file)
@@ -79,8 +79,6 @@ public:
     // Exposed for WebKitTestRunner use only.
     void forceUpdate() { update(); }
 
-    static bool anyPagesAreUsingPrivateBrowsing();
-
 private:
     void platformInitializeStore();
 
@@ -102,8 +100,6 @@ private:
     void deleteKey(const String& key);
     void platformDeleteKey(const String& key);
 
-    void updatePrivateBrowsingValue(bool value);
-
     void registerDefaultBoolValueForKey(const String&, bool);
     void registerDefaultUInt32ValueForKey(const String&, uint32_t);
 
index 80e5f99..78e8c79 100644 (file)
@@ -626,9 +626,6 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
     // Initialize the network process.
     networkProcess->send(Messages::NetworkProcess::InitializeNetworkProcess(parameters), 0);
 
-    if (WebPreferences::anyPagesAreUsingPrivateBrowsing())
-        networkProcess->send(Messages::NetworkProcess::AddWebsiteDataStore(WebsiteDataStoreParameters::legacyPrivateSessionParameters()), 0);
-
 #if PLATFORM(COCOA)
     networkProcess->send(Messages::NetworkProcess::SetQOS(networkProcessLatencyQOS(), networkProcessThroughputQOS()), 0);
 #endif
@@ -735,18 +732,6 @@ void WebProcessPool::disableServiceWorkerProcessTerminationDelay()
 #endif
 }
 
-void WebProcessPool::willStartUsingPrivateBrowsing()
-{
-    for (auto* processPool : allProcessPools())
-        processPool->setAnyPageGroupMightHavePrivateBrowsingEnabled(true);
-}
-
-void WebProcessPool::willStopUsingPrivateBrowsing()
-{
-    for (auto* processPool : allProcessPools())
-        processPool->setAnyPageGroupMightHavePrivateBrowsingEnabled(false);
-}
-
 void WebProcessPool::windowServerConnectionStateChanged()
 {
     size_t processCount = m_processes.size();
@@ -754,16 +739,6 @@ void WebProcessPool::windowServerConnectionStateChanged()
         m_processes[i]->windowServerConnectionStateChanged();
 }
 
-void WebProcessPool::setAnyPageGroupMightHavePrivateBrowsingEnabled(bool privateBrowsingEnabled)
-{
-    if (privateBrowsingEnabled) {
-        ensureNetworkProcess().send(Messages::NetworkProcess::AddWebsiteDataStore(WebsiteDataStoreParameters::legacyPrivateSessionParameters()), 0);
-    } else {
-        if (auto* networkProcess = this->networkProcess())
-            networkProcess->removeSession(PAL::SessionID::legacyPrivateSessionID());
-    }
-}
-
 void (*s_invalidMessageCallback)(WKStringRef messageName);
 
 void WebProcessPool::setInvalidMessageCallback(void (*invalidMessageCallback)(WKStringRef messageName))
index 643c40d..6b7c761 100644 (file)
@@ -395,9 +395,6 @@ public:
 
     void windowServerConnectionStateChanged();
 
-    static void willStartUsingPrivateBrowsing();
-    static void willStopUsingPrivateBrowsing();
-
 #if USE(SOUP)
     void setIgnoreTLSErrors(bool);
     bool ignoreTLSErrors() const { return m_ignoreTLSErrors; }
@@ -408,7 +405,7 @@ public:
 
     void processDidCachePage(WebProcessProxy*);
 
-    bool isURLKnownHSTSHost(const String& urlString, bool privateBrowsingEnabled) const;
+    bool isURLKnownHSTSHost(const String& urlString) const;
     void resetHSTSHosts();
     void resetHSTSHostsAddedAfterDate(double startDateIntervalSince1970);
 
@@ -594,8 +591,6 @@ private:
     void addPlugInAutoStartOriginHash(const String& pageOrigin, unsigned plugInOriginHash, PAL::SessionID);
     void plugInDidReceiveUserInteraction(unsigned plugInOriginHash, PAL::SessionID);
 
-    void setAnyPageGroupMightHavePrivateBrowsingEnabled(bool);
-
     void resolvePathsForSandboxExtensions();
     void platformResolvePathsForSandboxExtensions();
 
index adedb53..02e9d5c 100644 (file)
@@ -3546,11 +3546,6 @@ void WebPage::updatePreferences(const WebPreferencesStore& store)
     m_scrollingPerformanceLoggingEnabled = store.getBoolValueForKey(WebPreferencesKey::scrollingPerformanceLoggingEnabledKey());
     settings.setScrollingPerformanceLoggingEnabled(m_scrollingPerformanceLoggingEnabled);
 
-    if (store.getBoolValueForKey(WebPreferencesKey::privateBrowsingEnabledKey()) && !usesEphemeralSession())
-        setSessionID(PAL::SessionID::legacyPrivateSessionID());
-    else if (!store.getBoolValueForKey(WebPreferencesKey::privateBrowsingEnabledKey()) && sessionID() == PAL::SessionID::legacyPrivateSessionID())
-        setSessionID(PAL::SessionID::defaultSessionID());
-
     bool isAppNapEnabled = store.getBoolValueForKey(WebPreferencesKey::pageVisibilityBasedProcessSuppressionEnabledKey());
     if (m_isAppNapEnabled != isAppNapEnabled) {
         m_isAppNapEnabled = isAppNapEnabled;
index 7282d36..577faab 100644 (file)
@@ -1,3 +1,17 @@
+2019-09-17  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r249923): ASSERTION FAILED: sessionID == WebProcess::singleton().sessionID() in WebCore::SWClientConnection *WebKit::WebServiceWorkerProvider::existingServiceWorkerConnectionForSession(PAL::SessionID)
+        https://bugs.webkit.org/show_bug.cgi?id=201859
+        <rdar://problem/55426742>
+
+        Reviewed by Alex Christensen.
+
+        Update API test to use WKWebsiteDataStoreRef C API to do private browsing instead of using
+        the legacy WKPreferencesSetPrivateBrowsingEnabled() C API.
+
+        * TestWebKitAPI/Tests/WebKit/PrivateBrowsingPushStateNoHistoryCallback.cpp:
+        (TestWebKitAPI::TEST):
+
 2019-09-17  Ryan Haddad  <ryanhaddad@apple.com>
 
         Bring up queues for iOS 13
index bfb08fd..635253e 100644 (file)
@@ -32,6 +32,7 @@
 #include "Test.h"
 #include <WebKit/WKPreferencesRefPrivate.h>
 #include <WebKit/WKRetainPtr.h>
+#include <WebKit/WKWebsiteDataStoreRef.h>
 
 namespace TestWebKitAPI {
 
@@ -52,6 +53,12 @@ static void didNavigateWithNavigationData(WKContextRef context, WKPageRef page,
 TEST(WebKit, PrivateBrowsingPushStateNoHistoryCallback)
 {
     WKRetainPtr<WKContextRef> context = adoptWK(WKContextCreateWithConfiguration(nullptr));
+    
+    auto pageConfiguration = adoptWK(WKPageConfigurationCreate());
+    WKPageConfigurationSetContext(pageConfiguration.get(), context.get());
+    
+    auto ephemeralStore = adoptWK(WKWebsiteDataStoreCreateNonPersistentDataStore());
+    WKPageConfigurationSetWebsiteDataStore(pageConfiguration.get(), ephemeralStore.get());
 
     WKContextHistoryClientV0 historyClient;
     memset(&historyClient, 0, sizeof(historyClient));
@@ -61,7 +68,7 @@ TEST(WebKit, PrivateBrowsingPushStateNoHistoryCallback)
 
     WKContextSetHistoryClient(context.get(), &historyClient.base);
 
-    PlatformWebView webView(context.get());
+    PlatformWebView webView(pageConfiguration.get());
 
     WKPageNavigationClientV0 pageLoaderClient;
     memset(&pageLoaderClient, 0, sizeof(pageLoaderClient));
@@ -74,7 +81,6 @@ TEST(WebKit, PrivateBrowsingPushStateNoHistoryCallback)
     WKPageSetPageNavigationClient(webView.page(), &pageLoaderClient.base);
 
     WKRetainPtr<WKPreferencesRef> preferences = adoptWK(WKPreferencesCreate());
-    WKPreferencesSetPrivateBrowsingEnabled(preferences.get(), true);
     WKPreferencesSetUniversalAccessFromFileURLsAllowed(preferences.get(), true);
 
     WKPageGroupRef pageGroup = WKPageGetPageGroup(webView.page());
@@ -85,12 +91,13 @@ TEST(WebKit, PrivateBrowsingPushStateNoHistoryCallback)
 
     Util::run(&didSameDocumentNavigation);
 
-    WKPreferencesSetPrivateBrowsingEnabled(preferences.get(), false);
+    WKPageConfigurationSetWebsiteDataStore(pageConfiguration.get(), WKWebsiteDataStoreGetDefaultDataStore());
+    PlatformWebView webView2(pageConfiguration.get());
 
     historyClient.didNavigateWithNavigationData = didNavigateWithNavigationData;
     WKContextSetHistoryClient(context.get(), &historyClient.base);
 
-    WKPageLoadURL(webView.page(), url.get());
+    WKPageLoadURL(webView2.page(), url.get());
 
     Util::run(&didNavigate);
 }