Make HTTP/3 experimental feature work on iOS and only create storage directory if...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jun 2020 06:04:25 +0000 (06:04 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jun 2020 06:04:25 +0000 (06:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213527

Patch by Alex Christensen <achristensen@webkit.org> on 2020-06-23
Reviewed by Geoffrey Garen.

Source/WebKit:

Reading the experimental feature wasn't working on iOS because the user defaults prefix is different.
Also, only resolve and create the directory if it is enabled.  This should help with startup time, which
has to wait for all directories to be resolved and created.  See rdar://problem/64263228 which this should help with.
We also respect the system default unless explicitly turned on, which should make it easier for CFNetwork to change things.

I added an API test that verifies that the default alt-svc storage directory is not created when we don't want it to be.

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::ensureNetworkProcess):
* UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
(WebKit::WebsiteDataStore::platformSetNetworkParameters):
(WebKit::WebsiteDataStore::http3Enabled):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
(TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm
Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
Source/WebKit/UIProcess/WebsiteData/playstation/WebsiteDataStorePlayStation.cpp
Source/WebKit/UIProcess/WebsiteData/win/WebsiteDataStoreWin.cpp
Source/WebKit/UIProcess/glib/WebsiteDataStoreGLib.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm

index 4fbe62d..2e975d3 100644 (file)
@@ -1,3 +1,23 @@
+2020-06-23  Alex Christensen  <achristensen@webkit.org>
+
+        Make HTTP/3 experimental feature work on iOS and only create storage directory if enabled
+        https://bugs.webkit.org/show_bug.cgi?id=213527
+
+        Reviewed by Geoffrey Garen.
+
+        Reading the experimental feature wasn't working on iOS because the user defaults prefix is different.
+        Also, only resolve and create the directory if it is enabled.  This should help with startup time, which
+        has to wait for all directories to be resolved and created.  See rdar://problem/64263228 which this should help with.
+        We also respect the system default unless explicitly turned on, which should make it easier for CFNetwork to change things.
+
+        I added an API test that verifies that the default alt-svc storage directory is not created when we don't want it to be.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::ensureNetworkProcess):
+        * UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
+        (WebKit::WebsiteDataStore::platformSetNetworkParameters):
+        (WebKit::WebsiteDataStore::http3Enabled):
+
 2020-06-23  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         REGRESSION (r244633): Mail flashes when copying text in an email
index a2bb74d..a77e171 100644 (file)
@@ -1202,7 +1202,8 @@ NetworkSessionCocoa::NetworkSessionCocoa(NetworkProcess& networkProcess, Network
         configuration._alternativeServicesStorage = [[[_NSHTTPAlternativeServicesStorage alloc] initPersistentStoreWithURL:[[NSURL fileURLWithPath:parameters.alternativeServiceDirectory isDirectory:YES] URLByAppendingPathComponent:@"AlternativeService.sqlite"]] autorelease];
     } else
         ASSERT(m_sessionID.isEphemeral());
-    configuration._allowsHTTP3 = parameters.http3Enabled;
+    if (parameters.http3Enabled)
+        configuration._allowsHTTP3 = YES;
 #endif
 
     configuration._preventsSystemHTTPProxyAuthentication = parameters.preventsSystemHTTPProxyAuthentication;
index cd7e708..2b27694 100644 (file)
@@ -673,10 +673,12 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
     }
 
 #if HAVE(CFNETWORK_ALTERNATIVE_SERVICE)
-    parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectory = WebsiteDataStore::defaultAlternativeServicesDirectory();
-    if (!parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectory, parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectoryExtensionHandle);
-    parameters.defaultDataStoreParameters.networkSessionParameters.http3Enabled = WebsiteDataStore::http3Enabled();
+    if (WebsiteDataStore::http3Enabled()) {
+        parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectory = WebsiteDataStore::defaultAlternativeServicesDirectory();
+        if (!parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectory.isEmpty())
+            SandboxExtension::createHandleForReadWriteDirectory(parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectory, parameters.defaultDataStoreParameters.networkSessionParameters.alternativeServiceDirectoryExtensionHandle);
+        parameters.defaultDataStoreParameters.networkSessionParameters.http3Enabled = true;
+    }
 #endif
     bool isItpStateExplicitlySet = false;
     parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsParameters = ResourceLoadStatisticsParameters {
index a9504d3..a001f2e 100644 (file)
@@ -156,11 +156,14 @@ void WebsiteDataStore::platformSetNetworkParameters(WebsiteDataStoreParameters&
         httpsProxy = URL(URL(), [defaults stringForKey:(NSString *)WebKit2HTTPSProxyDefaultsKey]);
 
 #if HAVE(CFNETWORK_ALTERNATIVE_SERVICE)
-    String alternativeServiceStorageDirectory = resolvedAlternativeServicesStorageDirectory();
-    SandboxExtension::Handle alternativeServiceStorageDirectoryExtensionHandle;
-    if (!alternativeServiceStorageDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(alternativeServiceStorageDirectory, alternativeServiceStorageDirectoryExtensionHandle);
     bool http3Enabled = WebsiteDataStore::http3Enabled();
+    String alternativeServiceStorageDirectory;
+    SandboxExtension::Handle alternativeServiceStorageDirectoryExtensionHandle;
+    if (http3Enabled) {
+        alternativeServiceStorageDirectory = resolvedAlternativeServicesStorageDirectory();
+        if (!alternativeServiceStorageDirectory.isEmpty())
+            SandboxExtension::createHandleForReadWriteDirectory(alternativeServiceStorageDirectory, alternativeServiceStorageDirectoryExtensionHandle);
+    }
 #endif
 
     bool shouldIncludeLocalhostInResourceLoadStatistics = isSafari;
@@ -202,7 +205,12 @@ void WebsiteDataStore::platformSetNetworkParameters(WebsiteDataStoreParameters&
 bool WebsiteDataStore::http3Enabled()
 {
 #if HAVE(CFNETWORK_ALTERNATIVE_SERVICE)
-    return [[NSUserDefaults standardUserDefaults] boolForKey:[NSString stringWithFormat:@"Experimental%@", (NSString *)WebPreferencesKey::http3EnabledKey()]];
+#if PLATFORM(MAC)
+    NSString *format = @"Experimental%@";
+#else
+    NSString *format = @"WebKitExperimental%@";
+#endif
+    return [[NSUserDefaults standardUserDefaults] boolForKey:[NSString stringWithFormat:format, (NSString *)WebPreferencesKey::http3EnabledKey()]];
 #else
     return false;
 #endif
@@ -261,7 +269,7 @@ WTF::String WebsiteDataStore::defaultNetworkCacheDirectory()
 
 WTF::String WebsiteDataStore::defaultAlternativeServicesDirectory()
 {
-    return cacheDirectoryFileSystemRepresentation("AlternativeServices");
+    return cacheDirectoryFileSystemRepresentation("AlternativeServices", ShouldCreateDirectory::No);
 }
 
 WTF::String WebsiteDataStore::defaultMediaCacheDirectory()
@@ -333,7 +341,7 @@ WTF::String WebsiteDataStore::tempDirectoryFileSystemRepresentation(const WTF::S
     return url.absoluteURL.path.fileSystemRepresentation;
 }
 
-WTF::String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const WTF::String& directoryName)
+WTF::String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const WTF::String& directoryName, ShouldCreateDirectory shouldCreateDirectory)
 {
     static dispatch_once_t onceToken;
     static NSURL *cacheURL;
@@ -354,7 +362,8 @@ WTF::String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const WTF::
     });
 
     NSURL *url = [cacheURL URLByAppendingPathComponent:directoryName isDirectory:YES];
-    if (![[NSFileManager defaultManager] createDirectoryAtURL:url withIntermediateDirectories:YES attributes:nil error:nullptr])
+    if (shouldCreateDirectory == ShouldCreateDirectory::Yes
+        && ![[NSFileManager defaultManager] createDirectoryAtURL:url withIntermediateDirectories:YES attributes:nil error:nullptr])
         LOG_ERROR("Failed to create directory %@", url);
 
     return url.absoluteURL.path.fileSystemRepresentation;
index a0fcc99..15fe4c1 100644 (file)
@@ -217,7 +217,7 @@ void WebsiteDataStore::resolveDirectoriesIfNecessary()
         m_resolvedConfiguration->setWebSQLDatabaseDirectory(resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->webSQLDatabaseDirectory()));
     if (!m_configuration->indexedDBDatabaseDirectory().isEmpty())
         m_resolvedConfiguration->setIndexedDBDatabaseDirectory(resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->indexedDBDatabaseDirectory()));
-    if (!m_configuration->alternativeServicesDirectory().isEmpty())
+    if (!m_configuration->alternativeServicesDirectory().isEmpty() && WebsiteDataStore::http3Enabled())
         m_resolvedConfiguration->setAlternativeServicesDirectory(resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->alternativeServicesDirectory()));
     if (!m_configuration->deviceIdHashSaltsStorageDirectory().isEmpty())
         m_resolvedConfiguration->setDeviceIdHashSaltsStorageDirectory(resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->deviceIdHashSaltsStorageDirectory()));
index 61c15b2..713f5ed 100644 (file)
@@ -318,7 +318,7 @@ private:
 
     enum class ShouldCreateDirectory { No, Yes };
     static WTF::String tempDirectoryFileSystemRepresentation(const WTF::String& directoryName, ShouldCreateDirectory = ShouldCreateDirectory::Yes);
-    static WTF::String cacheDirectoryFileSystemRepresentation(const WTF::String& directoryName);
+    static WTF::String cacheDirectoryFileSystemRepresentation(const WTF::String& directoryName, ShouldCreateDirectory = ShouldCreateDirectory::Yes);
     static WTF::String websiteDataDirectoryFileSystemRepresentation(const WTF::String& directoryName);
 
     HashSet<RefPtr<WebProcessPool>> processPools(size_t limit = std::numeric_limits<size_t>::max()) const;
index 31bdcad..c198974 100644 (file)
@@ -91,7 +91,7 @@ String WebsiteDataStore::defaultResourceLoadStatisticsDirectory()
     return { };
 }
 
-String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const String& directoryName)
+String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const String& directoryName, ShouldCreateDirectory)
 {
     return { };
 }
index 35a4b24..b87c6fd 100644 (file)
@@ -91,7 +91,7 @@ String WebsiteDataStore::defaultResourceLoadStatisticsDirectory()
     return FileSystem::pathByAppendingComponent(FileSystem::localUserSpecificStorageDirectory(), "ResourceLoadStatistics");
 }
 
-String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const String& directoryName)
+String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const String& directoryName, ShouldCreateDirectory)
 {
     return FileSystem::pathByAppendingComponent(FileSystem::localUserSpecificStorageDirectory(), directoryName);
 }
index 9349af5..d020266 100644 (file)
@@ -94,7 +94,7 @@ WTF::String WebsiteDataStore::defaultResourceLoadStatisticsDirectory()
     return websiteDataDirectoryFileSystemRepresentation(BASE_DIRECTORY G_DIR_SEPARATOR_S "itp");
 }
 
-WTF::String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const WTF::String& directoryName)
+WTF::String WebsiteDataStore::cacheDirectoryFileSystemRepresentation(const WTF::String& directoryName, ShouldCreateDirectory)
 {
     return FileSystem::pathByAppendingComponent(FileSystem::stringFromFileSystemRepresentation(g_get_user_cache_dir()), directoryName);
 }
index 3590a6a..ef3bc91 100644 (file)
@@ -1,3 +1,13 @@
+2020-06-23  Alex Christensen  <achristensen@webkit.org>
+
+        Make HTTP/3 experimental feature work on iOS and only create storage directory if enabled
+        https://bugs.webkit.org/show_bug.cgi?id=213527
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
+        (TEST):
+
 2020-06-23  Jonathan Bedard  <jbedard@apple.com>
 
         [Big Sur] Add 11.0 version to scripts
index 4d673ee..4bae59d 100644 (file)
@@ -436,6 +436,37 @@ TEST(WebKit, WebsiteDataStoreEphemeral)
     [[NSFileManager defaultManager] removeItemAtURL:defaultResourceLoadStatisticsPath error:nil];
 }
 
+TEST(WebKit, AlternativeServicesDefaultDirectoryCreation)
+{
+    NSURL *defaultDirectory = [NSURL fileURLWithPath:[@"~/Library/Caches/com.apple.WebKit.TestWebKitAPI/WebKit/AlternativeServices" stringByExpandingTildeInPath] isDirectory:YES];
+    [[NSFileManager defaultManager] removeItemAtURL:defaultDirectory error:nil];
+    
+    TestWKWebView *webView1 = [[[TestWKWebView alloc] init] autorelease];
+    [webView1 synchronouslyLoadHTMLString:@"start auxiliary processes" baseURL:nil];
+
+    EXPECT_FALSE([[NSFileManager defaultManager] fileExistsAtPath:defaultDirectory.path]);
+    
+#if HAVE(CFNETWORK_ALTERNATIVE_SERVICE)
+
+#if PLATFORM(MAC)
+    NSString *key = @"ExperimentalHTTP3Enabled";
+#else
+    NSString *key = @"WebKitExperimentalHTTP3Enabled";
+#endif
+    
+    [[NSUserDefaults standardUserDefaults] setObject:[NSNumber numberWithBool:YES] forKey:key];
+
+    TestWKWebView *webView2 = [[[TestWKWebView alloc] init] autorelease];
+    [webView2 synchronouslyLoadHTMLString:@"start auxiliary processes" baseURL:nil];
+
+    EXPECT_TRUE([[NSFileManager defaultManager] fileExistsAtPath:defaultDirectory.path]);
+
+    [[NSUserDefaults standardUserDefaults] removeObjectForKey:key];
+    [[NSFileManager defaultManager] removeItemAtURL:defaultDirectory error:nil];
+
+#endif // HAVE(CFNETWORK_ALTERNATIVE_SERVICE)
+}
+
 TEST(WebKit, WebsiteDataStoreEphemeralViaConfiguration)
 {
     RetainPtr<WebsiteDataStoreCustomPathsMessageHandler> handler = adoptNS([[WebsiteDataStoreCustomPathsMessageHandler alloc] init]);