Title: [209212] trunk/Source/WebKit2
Revision
209212
Author
timothy_hor...@apple.com
Date
2016-12-01 14:31:01 -0800 (Thu, 01 Dec 2016)

Log Message

Every WKWebView initialization spends a few milliseconds hitting the disk
https://bugs.webkit.org/show_bug.cgi?id=165268
<rdar://problem/29010113>

Reviewed by Brady Eidson.

Every WKWebView init currently involves doing directory creation and
symlink resolution for a number of paths (to create sandbox extensions
for all of our storage directories), which means touching the disk a
lot during init. All of these paths are immutable per-WebProcessPool,
so, instead, cache them on WebProcessPool and create sandbox extensions
from the already-resolved paths. This is a sizable (~4x) speedup when
initializing subsequent web views.

* Shared/SandboxExtension.h:
(WebKit::SandboxExtension::createHandleWithoutResolvingPath):
Add createHandleWithoutResolvingPath, which makes a sandbox extension
handle without doing symlink resolution.

(WebKit::resolvePathForSandboxExtension):
(WebKit::resolveAndCreateReadWriteDirectoryForSandboxExtension):
Add two functions that do the same resolution that SandboxExtension::createHandle
and ::createHandleForReadWriteDirectory do, but just return the paths,
for later use with createHandleWithoutResolvingPath.

* Shared/mac/SandboxExtensionMac.mm:
(WebKit::resolveAndCreateReadWriteDirectoryForSandboxExtension):
(WebKit::resolvePathForSandboxExtension):
(WebKit::SandboxExtension::createHandleWithoutResolvingPath):
(WebKit::SandboxExtension::createHandle):
(WebKit::SandboxExtension::createHandleForReadWriteDirectory):
Implement the aforementioned functions, and reimplement the existing
createHandle{ForReadWriteDirectory} functions in terms of them.

* UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::platformDefaultIconDatabasePath):
Stop wasting time generating and resolving a platform default icon
database path, since we don't actually use it for anything anymore except
to determine whether the icon database is enabled, and we only want to
enable it if the client has provided a path.

(WebKit::WebProcessPool::platformResolvePathsForSandboxExtensions):
(WebKit::WebProcessPool::platformInitializeWebProcess):
* UIProcess/WebProcessPool.cpp:
(WebKit::m_hiddenPageThrottlingTimer):
(WebKit::WebProcessPool::resolvePathsForSandboxExtensions):
(WebKit::WebProcessPool::createNewWebProcess):
* UIProcess/WebProcessPool.h:
Resolve paths in resolvePathsForSandboxExtensions, and use the resolved
paths along with createHandleWithoutResolvingPath in order to effectively
cache the resolution operation.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (209211 => 209212)


--- trunk/Source/WebKit2/ChangeLog	2016-12-01 22:24:07 UTC (rev 209211)
+++ trunk/Source/WebKit2/ChangeLog	2016-12-01 22:31:01 UTC (rev 209212)
@@ -1,3 +1,57 @@
+2016-12-01  Tim Horton  <timothy_hor...@apple.com>
+
+        Every WKWebView initialization spends a few milliseconds hitting the disk
+        https://bugs.webkit.org/show_bug.cgi?id=165268
+        <rdar://problem/29010113>
+
+        Reviewed by Brady Eidson.
+
+        Every WKWebView init currently involves doing directory creation and
+        symlink resolution for a number of paths (to create sandbox extensions
+        for all of our storage directories), which means touching the disk a
+        lot during init. All of these paths are immutable per-WebProcessPool,
+        so, instead, cache them on WebProcessPool and create sandbox extensions
+        from the already-resolved paths. This is a sizable (~4x) speedup when
+        initializing subsequent web views.
+
+        * Shared/SandboxExtension.h:
+        (WebKit::SandboxExtension::createHandleWithoutResolvingPath):
+        Add createHandleWithoutResolvingPath, which makes a sandbox extension
+        handle without doing symlink resolution.
+
+        (WebKit::resolvePathForSandboxExtension):
+        (WebKit::resolveAndCreateReadWriteDirectoryForSandboxExtension):
+        Add two functions that do the same resolution that SandboxExtension::createHandle
+        and ::createHandleForReadWriteDirectory do, but just return the paths,
+        for later use with createHandleWithoutResolvingPath.
+
+        * Shared/mac/SandboxExtensionMac.mm:
+        (WebKit::resolveAndCreateReadWriteDirectoryForSandboxExtension):
+        (WebKit::resolvePathForSandboxExtension):
+        (WebKit::SandboxExtension::createHandleWithoutResolvingPath):
+        (WebKit::SandboxExtension::createHandle):
+        (WebKit::SandboxExtension::createHandleForReadWriteDirectory):
+        Implement the aforementioned functions, and reimplement the existing
+        createHandle{ForReadWriteDirectory} functions in terms of them.
+
+        * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
+        (WebKit::WebProcessPool::platformDefaultIconDatabasePath):
+        Stop wasting time generating and resolving a platform default icon
+        database path, since we don't actually use it for anything anymore except
+        to determine whether the icon database is enabled, and we only want to
+        enable it if the client has provided a path.
+
+        (WebKit::WebProcessPool::platformResolvePathsForSandboxExtensions):
+        (WebKit::WebProcessPool::platformInitializeWebProcess):
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::m_hiddenPageThrottlingTimer):
+        (WebKit::WebProcessPool::resolvePathsForSandboxExtensions):
+        (WebKit::WebProcessPool::createNewWebProcess):
+        * UIProcess/WebProcessPool.h:
+        Resolve paths in resolvePathsForSandboxExtensions, and use the resolved
+        paths along with createHandleWithoutResolvingPath in order to effectively
+        cache the resolution operation.
+
 2016-12-01  Antti Koivisto  <an...@apple.com>
 
         Revert r208931

Modified: trunk/Source/WebKit2/Shared/SandboxExtension.h (209211 => 209212)


--- trunk/Source/WebKit2/Shared/SandboxExtension.h	2016-12-01 22:24:07 UTC (rev 209211)
+++ trunk/Source/WebKit2/Shared/SandboxExtension.h	2016-12-01 22:31:01 UTC (rev 209212)
@@ -91,9 +91,10 @@
     };
     
     static RefPtr<SandboxExtension> create(const Handle&);
-    static bool createHandle(const String& path, Type type, Handle&);
+    static bool createHandle(const String& path, Type, Handle&);
+    static bool createHandleWithoutResolvingPath(const String& path, Type, Handle&);
     static bool createHandleForReadWriteDirectory(const String& path, Handle&); // Will attempt to create the directory.
-    static String createHandleForTemporaryFile(const String& prefix, Type type, Handle&);
+    static String createHandleForTemporaryFile(const String& prefix, Type, Handle&);
     static bool createHandleForGenericExtension(const String& extensionClass, Handle&);
     ~SandboxExtension();
 
@@ -127,6 +128,7 @@
 inline bool SandboxExtension::HandleArray::decode(IPC::Decoder&, HandleArray&) { return true; }
 inline RefPtr<SandboxExtension> SandboxExtension::create(const Handle&) { return nullptr; }
 inline bool SandboxExtension::createHandle(const String&, Type, Handle&) { return true; }
+inline bool SandboxExtension::createHandleWithoutResolvingPath(const String&, Type, Handle&) { return true; }
 inline bool SandboxExtension::createHandleForReadWriteDirectory(const String&, Handle&) { return true; }
 inline String SandboxExtension::createHandleForTemporaryFile(const String& /*prefix*/, Type, Handle&) {return String();}
 inline bool SandboxExtension::createHandleForGenericExtension(const String& /*extensionClass*/, Handle&) { return true; }
@@ -136,8 +138,12 @@
 inline bool SandboxExtension::consumePermanently() { return true; }
 inline bool SandboxExtension::consumePermanently(const Handle&) { return true; }
 inline String stringByResolvingSymlinksInPath(const String& path) { return path; }
+inline String resolvePathForSandboxExtension(const String& path) { return path; }
+inline String resolveAndCreateReadWriteDirectoryForSandboxExtension(const String& path) { return path; }
 #else
 String stringByResolvingSymlinksInPath(const String& path);
+String resolvePathForSandboxExtension(const String& path);
+String resolveAndCreateReadWriteDirectoryForSandboxExtension(const String& path);
 #endif
 
 } // namespace WebKit

Modified: trunk/Source/WebKit2/Shared/mac/SandboxExtensionMac.mm (209211 => 209212)


--- trunk/Source/WebKit2/Shared/mac/SandboxExtensionMac.mm	2016-12-01 22:24:07 UTC (rev 209211)
+++ trunk/Source/WebKit2/Shared/mac/SandboxExtensionMac.mm	2016-12-01 22:31:01 UTC (rev 209212)
@@ -215,19 +215,37 @@
     return String::fromUTF8(resolveSymlinksInPath(path.utf8()));
 }
 
-bool SandboxExtension::createHandle(const String& path, Type type, Handle& handle)
+String resolveAndCreateReadWriteDirectoryForSandboxExtension(const String& path)
 {
-    ASSERT(!handle.m_sandboxExtension);
+    NSError *error = nil;
+    NSString *nsPath = path;
 
+    if (![[NSFileManager defaultManager] createDirectoryAtPath:nsPath withIntermediateDirectories:YES attributes:nil error:&error]) {
+        NSLog(@"could not create \"%@\", error %@", nsPath, error);
+        return { };
+    }
+
+    return resolvePathForSandboxExtension(path);
+}
+
+String resolvePathForSandboxExtension(const String& path)
+{
     // FIXME: Do we need both resolveSymlinksInPath() and -stringByStandardizingPath?
     CString fileSystemPath = fileSystemRepresentation([(NSString *)path stringByStandardizingPath]);
     if (fileSystemPath.isNull()) {
         LOG_ERROR("Could not create a valid file system representation for the string '%s' of length %lu", fileSystemPath.data(), fileSystemPath.length());
-        return false;
+        return { };
     }
 
     CString standardizedPath = resolveSymlinksInPath(fileSystemPath);
-    handle.m_sandboxExtension = WKSandboxExtensionCreate(standardizedPath.data(), wkSandboxExtensionType(type));
+    return String::fromUTF8(standardizedPath);
+}
+
+bool SandboxExtension::createHandleWithoutResolvingPath(const String& path, Type type, Handle& handle)
+{
+    ASSERT(!handle.m_sandboxExtension);
+
+    handle.m_sandboxExtension = WKSandboxExtensionCreate(path.utf8().data(), wkSandboxExtensionType(type));
     if (!handle.m_sandboxExtension) {
         LOG_ERROR("Could not create a sandbox extension for '%s'", path.utf8().data());
         return false;
@@ -235,17 +253,20 @@
     return true;
 }
 
+bool SandboxExtension::createHandle(const String& path, Type type, Handle& handle)
+{
+    ASSERT(!handle.m_sandboxExtension);
+
+    return createHandleWithoutResolvingPath(resolvePathForSandboxExtension(path), type, handle);
+}
+
 bool SandboxExtension::createHandleForReadWriteDirectory(const String& path, SandboxExtension::Handle& handle)
 {
-    NSError *error = nil;
-    NSString *nsPath = path;
-
-    if (![[NSFileManager defaultManager] createDirectoryAtPath:nsPath withIntermediateDirectories:YES attributes:nil error:&error]) {
-        NSLog(@"could not create \"%@\", error %@", nsPath, error);
+    String resolvedPath = resolveAndCreateReadWriteDirectoryForSandboxExtension(path);
+    if (resolvedPath.isNull())
         return false;
-    }
 
-    return SandboxExtension::createHandle(path, SandboxExtension::ReadWrite, handle);
+    return SandboxExtension::createHandleWithoutResolvingPath(resolvedPath, SandboxExtension::ReadWrite, handle);
 }
 
 String SandboxExtension::createHandleForTemporaryFile(const String& prefix, Type type, Handle& handle)

Modified: trunk/Source/WebKit2/UIProcess/Cocoa/WebProcessPoolCocoa.mm (209211 => 209212)


--- trunk/Source/WebKit2/UIProcess/Cocoa/WebProcessPoolCocoa.mm	2016-12-01 22:24:07 UTC (rev 209211)
+++ trunk/Source/WebKit2/UIProcess/Cocoa/WebProcessPoolCocoa.mm	2016-12-01 22:31:01 UTC (rev 209212)
@@ -159,6 +159,17 @@
 }
 #endif
 
+void WebProcessPool::platformResolvePathsForSandboxExtensions()
+{
+    m_resolvedPaths.uiProcessBundleResourcePath = resolvePathForSandboxExtension([[NSBundle mainBundle] resourcePath]);
+
+#if PLATFORM(IOS)
+    m_resolvedPaths.cookieStorageDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(cookieStorageDirectory());
+    m_resolvedPaths.containerCachesDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(webContentCachesDirectory());
+    m_resolvedPaths.containerTemporaryDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(containerTemporaryDirectory());
+#endif
+}
+
 void WebProcessPool::platformInitializeWebProcess(WebProcessCreationParameters& parameters)
 {
     parameters.presenterApplicationPid = getpid();
@@ -186,11 +197,22 @@
 #endif
 
     // FIXME: This should really be configurable; we shouldn't just blindly allow read access to the UI process bundle.
-    parameters.uiProcessBundleResourcePath = [[NSBundle mainBundle] resourcePath];
-    SandboxExtension::createHandle(parameters.uiProcessBundleResourcePath, SandboxExtension::ReadOnly, parameters.uiProcessBundleResourcePathExtensionHandle);
+    parameters.uiProcessBundleResourcePath = m_resolvedPaths.uiProcessBundleResourcePath;
+    SandboxExtension::createHandleWithoutResolvingPath(parameters.uiProcessBundleResourcePath, SandboxExtension::ReadOnly, parameters.uiProcessBundleResourcePathExtensionHandle);
 
     parameters.uiProcessBundleIdentifier = String([[NSBundle mainBundle] bundleIdentifier]);
 
+#if PLATFORM(IOS)
+    if (!m_resolvedPaths.cookieStorageDirectory.isEmpty())
+        SandboxExtension::createHandleWithoutResolvingPath(m_resolvedPaths.cookieStorageDirectory, SandboxExtension::ReadWrite, parameters.cookieStorageDirectoryExtensionHandle);
+
+    if (!m_resolvedPaths.containerCachesDirectory.isEmpty())
+        SandboxExtension::createHandleWithoutResolvingPath(m_resolvedPaths.containerCachesDirectory, SandboxExtension::ReadWrite, parameters.containerCachesDirectoryExtensionHandle);
+
+    if (!m_resolvedPaths.containerTemporaryDirectory.isEmpty())
+        SandboxExtension::createHandleWithoutResolvingPath(m_resolvedPaths.containerTemporaryDirectory, SandboxExtension::ReadWrite, parameters.containerTemporaryDirectoryExtensionHandle);
+#endif
+
     parameters.fontWhitelist = m_fontWhitelist;
 
     if (m_bundleParameters) {
@@ -437,11 +459,7 @@
 
 String WebProcessPool::platformDefaultIconDatabasePath() const
 {
-    // FIXME: <rdar://problem/9138817> - After this "backwards compatibility" radar is removed, this code should be removed to only return an empty String.
-    NSString *databasesDirectory = [[NSUserDefaults standardUserDefaults] objectForKey:WebIconDatabaseDirectoryDefaultsKey];
-    if (!databasesDirectory || ![databasesDirectory isKindOfClass:[NSString class]])
-        databasesDirectory = @"~/Library/Icons/WebpageIcons.db";
-    return stringByResolvingSymlinksInPath([databasesDirectory stringByStandardizingPath]);
+    return "";
 }
 
 bool WebProcessPool::omitPDFSupport()

Modified: trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp (209211 => 209212)


--- trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp	2016-12-01 22:24:07 UTC (rev 209211)
+++ trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp	2016-12-01 22:31:01 UTC (rev 209212)
@@ -202,6 +202,8 @@
 
     addLanguageChangeObserver(this, languageChanged);
 
+    resolvePathsForSandboxExtensions();
+
 #if !LOG_DISABLED || !RELEASE_LOG_DISABLED
     WebCore::initializeLogChannelsIfNecessary();
     WebKit::initializeLogChannelsIfNecessary();
@@ -527,6 +529,17 @@
     m_processWithPageCache = process;
 }
 
+void WebProcessPool::resolvePathsForSandboxExtensions()
+{
+    m_resolvedPaths.injectedBundlePath = resolvePathForSandboxExtension(injectedBundlePath());
+    m_resolvedPaths.applicationCacheDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->applicationCacheDirectory());
+    m_resolvedPaths.webSQLDatabaseDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->webSQLDatabaseDirectory());
+    m_resolvedPaths.mediaCacheDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->mediaCacheDirectory());
+    m_resolvedPaths.mediaKeyStorageDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(m_configuration->mediaKeysStorageDirectory());
+
+    platformResolvePathsForSandboxExtensions();
+}
+
 WebProcessProxy& WebProcessPool::createNewWebProcess()
 {
     ensureNetworkProcess();
@@ -537,41 +550,27 @@
 
     parameters.urlParserEnabled = URLParser::enabled();
     
-    parameters.injectedBundlePath = injectedBundlePath();
+    parameters.injectedBundlePath = m_resolvedPaths.injectedBundlePath;
     if (!parameters.injectedBundlePath.isEmpty())
-        SandboxExtension::createHandle(parameters.injectedBundlePath, SandboxExtension::ReadOnly, parameters.injectedBundlePathExtensionHandle);
+        SandboxExtension::createHandleWithoutResolvingPath(parameters.injectedBundlePath, SandboxExtension::ReadOnly, parameters.injectedBundlePathExtensionHandle);
 
-    parameters.applicationCacheDirectory = m_configuration->applicationCacheDirectory();
+    parameters.applicationCacheDirectory = m_resolvedPaths.applicationCacheDirectory;
+    if (!parameters.applicationCacheDirectory.isEmpty())
+        SandboxExtension::createHandleWithoutResolvingPath(parameters.applicationCacheDirectory, SandboxExtension::ReadWrite, parameters.applicationCacheDirectoryExtensionHandle);
+
     parameters.applicationCacheFlatFileSubdirectoryName = m_configuration->applicationCacheFlatFileSubdirectoryName();
 
-    if (!parameters.applicationCacheDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(parameters.applicationCacheDirectory, parameters.applicationCacheDirectoryExtensionHandle);
-
-    parameters.webSQLDatabaseDirectory = m_configuration->webSQLDatabaseDirectory();
+    parameters.webSQLDatabaseDirectory = m_resolvedPaths.webSQLDatabaseDirectory;
     if (!parameters.webSQLDatabaseDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(parameters.webSQLDatabaseDirectory, parameters.webSQLDatabaseDirectoryExtensionHandle);
+        SandboxExtension::createHandleWithoutResolvingPath(parameters.webSQLDatabaseDirectory, SandboxExtension::ReadWrite, parameters.webSQLDatabaseDirectoryExtensionHandle);
 
-    parameters.mediaCacheDirectory = m_configuration->mediaCacheDirectory();
+    parameters.mediaCacheDirectory = m_resolvedPaths.mediaCacheDirectory;
     if (!parameters.mediaCacheDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(parameters.mediaCacheDirectory, parameters.mediaCacheDirectoryExtensionHandle);
+        SandboxExtension::createHandleWithoutResolvingPath(parameters.mediaCacheDirectory, SandboxExtension::ReadWrite, parameters.mediaCacheDirectoryExtensionHandle);
 
-#if PLATFORM(IOS)
-    String cookieStorageDirectory = this->cookieStorageDirectory();
-    if (!cookieStorageDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(cookieStorageDirectory, parameters.cookieStorageDirectoryExtensionHandle);
-
-    String containerCachesDirectory = this->webContentCachesDirectory();
-    if (!containerCachesDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(containerCachesDirectory, parameters.containerCachesDirectoryExtensionHandle);
-
-    String containerTemporaryDirectory = this->containerTemporaryDirectory();
-    if (!containerTemporaryDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(containerTemporaryDirectory, parameters.containerTemporaryDirectoryExtensionHandle);
-#endif
-
-    parameters.mediaKeyStorageDirectory = m_configuration->mediaKeysStorageDirectory();
+    parameters.mediaKeyStorageDirectory = m_resolvedPaths.mediaKeyStorageDirectory;
     if (!parameters.mediaKeyStorageDirectory.isEmpty())
-        SandboxExtension::createHandleForReadWriteDirectory(parameters.mediaKeyStorageDirectory, parameters.mediaKeyStorageDirectoryExtensionHandle);
+        SandboxExtension::createHandleWithoutResolvingPath(parameters.mediaKeyStorageDirectory, SandboxExtension::ReadWrite, parameters.mediaKeyStorageDirectoryExtensionHandle);
 
     parameters.shouldUseTestingNetworkSession = m_shouldUseTestingNetworkSession;
 
@@ -594,8 +593,6 @@
     parameters.shouldAlwaysUseComplexTextCodePath = m_alwaysUsesComplexTextCodePath;
     parameters.shouldUseFontSmoothing = m_shouldUseFontSmoothing;
 
-    // FIXME: This leaves UI process and WebProcess disagreeing about the state if the client hasn't set the path.
-    // iconDatabasePath is non-empty by default, but m_iconDatabase isn't enabled in UI process unless setDatabasePath is called explicitly.
     parameters.iconDatabaseEnabled = !iconDatabasePath().isEmpty();
 
     parameters.terminationTimeout = 0;

Modified: trunk/Source/WebKit2/UIProcess/WebProcessPool.h (209211 => 209212)


--- trunk/Source/WebKit2/UIProcess/WebProcessPool.h	2016-12-01 22:24:07 UTC (rev 209211)
+++ trunk/Source/WebKit2/UIProcess/WebProcessPool.h	2016-12-01 22:31:01 UTC (rev 209212)
@@ -444,6 +444,9 @@
 
     void setAnyPageGroupMightHavePrivateBrowsingEnabled(bool);
 
+    void resolvePathsForSandboxExtensions();
+    void platformResolvePathsForSandboxExtensions();
+
     Ref<API::ProcessPoolConfiguration> m_configuration;
 
     IPC::MessageReceiverMap m_messageReceiverMap;
@@ -574,6 +577,22 @@
 #if PLATFORM(COCOA)
     bool m_cookieStoragePartitioningEnabled { false };
 #endif
+
+    struct Paths {
+        String injectedBundlePath;
+        String applicationCacheDirectory;
+        String webSQLDatabaseDirectory;
+        String mediaCacheDirectory;
+        String mediaKeyStorageDirectory;
+        String uiProcessBundleResourcePath;
+
+#if PLATFORM(IOS)
+        String cookieStorageDirectory;
+        String containerCachesDirectory;
+        String containerTemporaryDirectory;
+#endif
+    };
+    Paths m_resolvedPaths;
 };
 
 template<typename T>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to