Title: [241821] trunk/Tools
Revision
241821
Author
cdu...@apple.com
Date
2019-02-20 10:34:44 -0800 (Wed, 20 Feb 2019)

Log Message

[WKTR] Avoid starting new NetworkProcesses unnecessarily when running the layout tests
https://bugs.webkit.org/show_bug.cgi?id=194829
<rdar://problem/47889906>

Reviewed by Alexey Proskuryakov.

Every time the TestOptions were changing we were creating both a new Web view and
a new WKContext, which would start a new Network process. In most cases, we only
need to contruct a new Web view and we do can keep reusing the same WKContext.
This patch implements this optimization and thus avoids spinning a lot of new
Network processes while running the layout tests.

* WebKitTestRunner/TestController.cpp:
(WTR::TestController::generateContextConfiguration const):
(WTR::TestController::generatePageConfiguration):
(WTR::TestController::createWebViewWithOptions):
(WTR::TestController::resetPreferencesToConsistentValues):
(WTR::updateTestOptionsFromTestHeader):
* WebKitTestRunner/TestController.h:
* WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::ContextOptions::hasSameInitializationOptions const):
(WTR::TestOptions::ContextOptions::shouldEnableProcessSwapOnNavigation const):
(WTR::TestOptions::hasSameInitializationOptions const):
(WTR::TestOptions::shouldEnableProcessSwapOnNavigation const): Deleted.
* WebKitTestRunner/cocoa/TestControllerCocoa.mm:
(WTR::TestController::platformAddTestOptions const):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (241820 => 241821)


--- trunk/Tools/ChangeLog	2019-02-20 18:23:19 UTC (rev 241820)
+++ trunk/Tools/ChangeLog	2019-02-20 18:34:44 UTC (rev 241821)
@@ -1,3 +1,32 @@
+2019-02-20  Chris Dumez  <cdu...@apple.com>
+
+        [WKTR] Avoid starting new NetworkProcesses unnecessarily when running the layout tests
+        https://bugs.webkit.org/show_bug.cgi?id=194829
+        <rdar://problem/47889906>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Every time the TestOptions were changing we were creating both a new Web view and
+        a new WKContext, which would start a new Network process. In most cases, we only
+        need to contruct a new Web view and we do can keep reusing the same WKContext.
+        This patch implements this optimization and thus avoids spinning a lot of new
+        Network processes while running the layout tests.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::generateContextConfiguration const):
+        (WTR::TestController::generatePageConfiguration):
+        (WTR::TestController::createWebViewWithOptions):
+        (WTR::TestController::resetPreferencesToConsistentValues):
+        (WTR::updateTestOptionsFromTestHeader):
+        * WebKitTestRunner/TestController.h:
+        * WebKitTestRunner/TestOptions.h:
+        (WTR::TestOptions::ContextOptions::hasSameInitializationOptions const):
+        (WTR::TestOptions::ContextOptions::shouldEnableProcessSwapOnNavigation const):
+        (WTR::TestOptions::hasSameInitializationOptions const):
+        (WTR::TestOptions::shouldEnableProcessSwapOnNavigation const): Deleted.
+        * WebKitTestRunner/cocoa/TestControllerCocoa.mm:
+        (WTR::TestController::platformAddTestOptions const):
+
 2019-02-20  Adrian Perez de Castro  <ape...@igalia.com>
 
         [WPE][GTK] Enable support for CONTENT_EXTENSIONS

Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (241820 => 241821)


--- trunk/Tools/WebKitTestRunner/TestController.cpp	2019-02-20 18:23:19 UTC (rev 241820)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp	2019-02-20 18:34:44 UTC (rev 241821)
@@ -454,7 +454,7 @@
     m_pageGroup.adopt(WKPageGroupCreateWithIdentifier(pageGroupIdentifier.get()));
 }
 
-WKRetainPtr<WKContextConfigurationRef> TestController::generateContextConfiguration(const TestOptions& options) const
+WKRetainPtr<WKContextConfigurationRef> TestController::generateContextConfiguration(const TestOptions::ContextOptions& options) const
 {
     auto configuration = adoptWK(WKContextConfigurationCreate());
     WKContextConfigurationSetInjectedBundlePath(configuration.get(), injectedBundlePath());
@@ -461,6 +461,17 @@
     WKContextConfigurationSetFullySynchronousModeIsAllowedForTesting(configuration.get(), true);
     WKContextConfigurationSetIgnoreSynchronousMessagingTimeoutsForTesting(configuration.get(), options.ignoreSynchronousMessagingTimeouts);
 
+    WKRetainPtr<WKMutableArrayRef> overrideLanguages = adoptWK(WKMutableArrayCreate());
+    for (auto& language : options.overrideLanguages)
+        WKArrayAppendItem(overrideLanguages.get(), adoptWK(WKStringCreateWithUTF8CString(language.utf8().data())).get());
+    WKContextConfigurationSetOverrideLanguages(configuration.get(), overrideLanguages.get());
+
+    if (options.shouldEnableProcessSwapOnNavigation()) {
+        WKContextConfigurationSetProcessSwapsOnNavigation(configuration.get(), true);
+        if (options.enableProcessSwapOnWindowOpen)
+            WKContextConfigurationSetProcessSwapsOnWindowOpenWithOpener(configuration.get(), true);
+    }
+
     if (const char* dumpRenderTreeTemp = libraryPathForTesting()) {
         String temporaryFolder = String::fromUTF8(dumpRenderTreeTemp);
 
@@ -475,30 +486,34 @@
     return configuration;
 }
 
-WKRetainPtr<WKPageConfigurationRef> TestController::generatePageConfiguration(WKContextConfigurationRef configuration)
+WKRetainPtr<WKPageConfigurationRef> TestController::generatePageConfiguration(const TestOptions& options)
 {
-    m_context = platformAdjustContext(adoptWK(WKContextCreateWithConfiguration(configuration)).get(), configuration);
+    if (!m_context || !m_contextOptions->hasSameInitializationOptions(options.contextOptions)) {
+        auto contextConfiguration = generateContextConfiguration(options.contextOptions);
+        m_context = platformAdjustContext(adoptWK(WKContextCreateWithConfiguration(contextConfiguration.get())).get(), contextConfiguration.get());
+        m_contextOptions = options.contextOptions;
 
-    m_geolocationProvider = std::make_unique<GeolocationProviderMock>(m_context.get());
+        m_geolocationProvider = std::make_unique<GeolocationProviderMock>(m_context.get());
 
-    if (const char* dumpRenderTreeTemp = libraryPathForTesting()) {
-        String temporaryFolder = String::fromUTF8(dumpRenderTreeTemp);
+        if (const char* dumpRenderTreeTemp = libraryPathForTesting()) {
+            String temporaryFolder = String::fromUTF8(dumpRenderTreeTemp);
 
-        // FIXME: This should be migrated to WKContextConfigurationRef.
-        // Disable icon database to avoid fetching <http://127.0.0.1:8000/favicon.ico> and making tests flaky.
-        // Invividual tests can enable it using testRunner.setIconDatabaseEnabled, although it's not currently supported in WebKitTestRunner.
-        WKContextSetIconDatabasePath(m_context.get(), toWK(emptyString()).get());
-    }
+            // FIXME: This should be migrated to WKContextConfigurationRef.
+            // Disable icon database to avoid fetching <http://127.0.0.1:8000/favicon.ico> and making tests flaky.
+            // Invividual tests can enable it using testRunner.setIconDatabaseEnabled, although it's not currently supported in WebKitTestRunner.
+            WKContextSetIconDatabasePath(m_context.get(), toWK(emptyString()).get());
+        }
 
-    WKContextSetDiskCacheSpeculativeValidationEnabled(m_context.get(), true);
-    WKContextUseTestingNetworkSession(m_context.get());
-    WKContextSetCacheModel(m_context.get(), kWKCacheModelDocumentBrowser);
+        WKContextSetDiskCacheSpeculativeValidationEnabled(m_context.get(), true);
+        WKContextUseTestingNetworkSession(m_context.get());
+        WKContextSetCacheModel(m_context.get(), kWKCacheModelDocumentBrowser);
 
-    auto* websiteDataStore = WKContextGetWebsiteDataStore(m_context.get());
-    WKWebsiteDataStoreSetCacheStoragePerOriginQuota(websiteDataStore, 400 * 1024);
-    
-    platformInitializeContext();
+        auto* websiteDataStore = WKContextGetWebsiteDataStore(m_context.get());
+        WKWebsiteDataStoreSetCacheStoragePerOriginQuota(websiteDataStore, 400 * 1024);
 
+        platformInitializeContext();
+    }
+
     WKContextInjectedBundleClientV1 injectedBundleClient = {
         { 1, this },
         didReceiveMessageFromInjectedBundle,
@@ -547,21 +562,8 @@
 
 void TestController::createWebViewWithOptions(const TestOptions& options)
 {
-    auto contextConfiguration = generateContextConfiguration(options);
+    auto configuration = generatePageConfiguration(options);
 
-    WKRetainPtr<WKMutableArrayRef> overrideLanguages = adoptWK(WKMutableArrayCreate());
-    for (auto& language : options.overrideLanguages)
-        WKArrayAppendItem(overrideLanguages.get(), adoptWK(WKStringCreateWithUTF8CString(language.utf8().data())).get());
-    WKContextConfigurationSetOverrideLanguages(contextConfiguration.get(), overrideLanguages.get());
-
-    if (options.shouldEnableProcessSwapOnNavigation()) {
-        WKContextConfigurationSetProcessSwapsOnNavigation(contextConfiguration.get(), true);
-        if (options.enableProcessSwapOnWindowOpen)
-            WKContextConfigurationSetProcessSwapsOnWindowOpenWithOpener(contextConfiguration.get(), true);
-    }
-
-    auto configuration = generatePageConfiguration(contextConfiguration.get());
-
     // Some preferences (notably mock scroll bars setting) currently cannot be re-applied to an existing view, so we need to set them now.
     // FIXME: Migrate these preferences to WKContextConfigurationRef.
     resetPreferencesToConsistentValues(options);
@@ -736,7 +738,7 @@
     for (const auto& internalDebugFeature : options.internalDebugFeatures)
         WKPreferencesSetInternalDebugFeatureForKey(preferences, internalDebugFeature.value, toWK(internalDebugFeature.key).get());
 
-    WKPreferencesSetProcessSwapOnNavigationEnabled(preferences, options.shouldEnableProcessSwapOnNavigation());
+    WKPreferencesSetProcessSwapOnNavigationEnabled(preferences, options.contextOptions.shouldEnableProcessSwapOnNavigation());
     WKPreferencesSetPageVisibilityBasedProcessSuppressionEnabled(preferences, false);
     WKPreferencesSetOfflineWebApplicationCacheEnabled(preferences, true);
     WKPreferencesSetSubpixelAntialiasedLayerTextEnabled(preferences, false);
@@ -1229,7 +1231,7 @@
         }
 
         if (key == "language")
-            testOptions.overrideLanguages = String(value.c_str()).split(',');
+            testOptions.contextOptions.overrideLanguages = String(value.c_str()).split(',');
         else if (key == "useThreadedScrolling")
             testOptions.useThreadedScrolling = parseBooleanTestHeaderValue(value);
         else if (key == "useAcceleratedDrawing")
@@ -1273,9 +1275,9 @@
         else if (key == "domPasteAllowed")
             testOptions.domPasteAllowed = parseBooleanTestHeaderValue(value);
         else if (key == "enableProcessSwapOnNavigation")
-            testOptions.enableProcessSwapOnNavigation = parseBooleanTestHeaderValue(value);
+            testOptions.contextOptions.enableProcessSwapOnNavigation = parseBooleanTestHeaderValue(value);
         else if (key == "enableProcessSwapOnWindowOpen")
-            testOptions.enableProcessSwapOnWindowOpen = parseBooleanTestHeaderValue(value);
+            testOptions.contextOptions.enableProcessSwapOnWindowOpen = parseBooleanTestHeaderValue(value);
         else if (key == "enableColorFilter")
             testOptions.enableColorFilter = parseBooleanTestHeaderValue(value);
         else if (key == "punchOutWhiteBackgroundsInDarkMode")
@@ -1297,7 +1299,7 @@
         else if (key == "contentInset.top")
             testOptions.contentInsetTop = std::stod(value);
         else if (key == "ignoreSynchronousMessagingTimeouts")
-            testOptions.ignoreSynchronousMessagingTimeouts = parseBooleanTestHeaderValue(value);
+            testOptions.contextOptions.ignoreSynchronousMessagingTimeouts = parseBooleanTestHeaderValue(value);
         pairStart = pairEnd + 1;
     }
 }

Modified: trunk/Tools/WebKitTestRunner/TestController.h (241820 => 241821)


--- trunk/Tools/WebKitTestRunner/TestController.h	2019-02-20 18:23:19 UTC (rev 241820)
+++ trunk/Tools/WebKitTestRunner/TestController.h	2019-02-20 18:34:44 UTC (rev 241821)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "GeolocationProviderMock.h"
+#include "TestOptions.h"
 #include "WebNotificationProvider.h"
 #include "WorkQueueManager.h"
 #include <WebKit/WKRetainPtr.h>
@@ -298,8 +299,8 @@
     void clearAdClickAttribution();
 
 private:
-    WKRetainPtr<WKPageConfigurationRef> generatePageConfiguration(WKContextConfigurationRef);
-    WKRetainPtr<WKContextConfigurationRef> generateContextConfiguration(const TestOptions&) const;
+    WKRetainPtr<WKPageConfigurationRef> generatePageConfiguration(const TestOptions&);
+    WKRetainPtr<WKContextConfigurationRef> generateContextConfiguration(const TestOptions::ContextOptions&) const;
     void initialize(int argc, const char* argv[]);
     void createWebViewWithOptions(const TestOptions&);
     void run();
@@ -466,6 +467,7 @@
 
     std::unique_ptr<PlatformWebView> m_mainWebView;
     WKRetainPtr<WKContextRef> m_context;
+    Optional<TestOptions::ContextOptions> m_contextOptions;
     WKRetainPtr<WKPageGroupRef> m_pageGroup;
     WKRetainPtr<WKUserContentControllerRef> m_userContentController;
 

Modified: trunk/Tools/WebKitTestRunner/TestOptions.h (241820 => 241821)


--- trunk/Tools/WebKitTestRunner/TestOptions.h	2019-02-20 18:23:19 UTC (rev 241820)
+++ trunk/Tools/WebKitTestRunner/TestOptions.h	2019-02-20 18:34:44 UTC (rev 241821)
@@ -33,6 +33,28 @@
 namespace WTR {
 
 struct TestOptions {
+    struct ContextOptions {
+        Vector<String> overrideLanguages;
+        bool ignoreSynchronousMessagingTimeouts { false };
+        bool enableProcessSwapOnNavigation { true };
+        bool enableProcessSwapOnWindowOpen { false };
+
+        bool hasSameInitializationOptions(const ContextOptions& options) const
+        {
+            if (ignoreSynchronousMessagingTimeouts != options.ignoreSynchronousMessagingTimeouts
+                || overrideLanguages != options.overrideLanguages
+                || enableProcessSwapOnNavigation != options.enableProcessSwapOnNavigation
+                || enableProcessSwapOnWindowOpen != options.enableProcessSwapOnWindowOpen)
+                return false;
+            return true;
+        }
+
+        bool shouldEnableProcessSwapOnNavigation() const
+        {
+            return enableProcessSwapOnNavigation || enableProcessSwapOnWindowOpen;
+        }
+    };
+
     bool useThreadedScrolling { false };
     bool useAcceleratedDrawing { false };
     bool useRemoteLayerTree { false };
@@ -58,8 +80,6 @@
     bool dumpJSConsoleLogInStdErr { false };
     bool allowCrossOriginSubresourcesToAskForCredentials { false };
     bool domPasteAllowed { true };
-    bool enableProcessSwapOnNavigation { true };
-    bool enableProcessSwapOnWindowOpen { false };
     bool enableColorFilter { false };
     bool punchOutWhiteBackgroundsInDarkMode { false };
     bool runSingly { false };
@@ -69,17 +89,17 @@
     bool enableEditableImages { false };
     bool editable { false };
     bool enableUndoManagerAPI { false };
-    bool ignoreSynchronousMessagingTimeouts { false };
 
     double contentInsetTop { 0 };
 
     float deviceScaleFactor { 1 };
-    Vector<String> overrideLanguages;
     std::string applicationManifest;
     std::string jscOptions;
     HashMap<String, bool> experimentalFeatures;
     HashMap<String, bool> internalDebugFeatures;
 
+    ContextOptions contextOptions;
+
     TestOptions(const std::string& pathOrURL);
 
     // Add here options that can only be set upon PlatformWebView
@@ -90,7 +110,6 @@
     {
         if (useThreadedScrolling != options.useThreadedScrolling
             || useAcceleratedDrawing != options.useAcceleratedDrawing
-            || overrideLanguages != options.overrideLanguages
             || useMockScrollbars != options.useMockScrollbars
             || needsSiteSpecificQuirks != options.needsSiteSpecificQuirks
             || useCharacterSelectionGranularity != options.useCharacterSelectionGranularity
@@ -107,8 +126,6 @@
             || applicationManifest != options.applicationManifest
             || allowCrossOriginSubresourcesToAskForCredentials != options.allowCrossOriginSubresourcesToAskForCredentials
             || domPasteAllowed != options.domPasteAllowed
-            || enableProcessSwapOnNavigation != options.enableProcessSwapOnNavigation
-            || enableProcessSwapOnWindowOpen != options.enableProcessSwapOnWindowOpen
             || enableColorFilter != options.enableColorFilter
             || punchOutWhiteBackgroundsInDarkMode != options.punchOutWhiteBackgroundsInDarkMode
             || jscOptions != options.jscOptions
@@ -119,10 +136,12 @@
             || enableEditableImages != options.enableEditableImages
             || editable != options.editable
             || enableUndoManagerAPI != options.enableUndoManagerAPI
-            || contentInsetTop != options.contentInsetTop
-            || ignoreSynchronousMessagingTimeouts != options.ignoreSynchronousMessagingTimeouts)
+            || contentInsetTop != options.contentInsetTop)
             return false;
 
+        if (!contextOptions.hasSameInitializationOptions(options.contextOptions))
+            return false;
+
         if (experimentalFeatures != options.experimentalFeatures)
             return false;
 
@@ -131,11 +150,6 @@
 
         return true;
     }
-    
-    bool shouldEnableProcessSwapOnNavigation() const
-    {
-        return enableProcessSwapOnNavigation || enableProcessSwapOnWindowOpen;
-    }
 };
 
 }

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm (241820 => 241821)


--- trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm	2019-02-20 18:23:19 UTC (rev 241820)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm	2019-02-20 18:34:44 UTC (rev 241821)
@@ -142,9 +142,9 @@
 void TestController::platformAddTestOptions(TestOptions& options) const
 {
     if ([[NSUserDefaults standardUserDefaults] boolForKey:@"EnableProcessSwapOnNavigation"])
-        options.enableProcessSwapOnNavigation = true;
+        options.contextOptions.enableProcessSwapOnNavigation = true;
     if ([[NSUserDefaults standardUserDefaults] boolForKey:@"EnableProcessSwapOnWindowOpen"])
-        options.enableProcessSwapOnWindowOpen = true;
+        options.contextOptions.enableProcessSwapOnWindowOpen = true;
 }
 
 void TestController::platformCreateWebView(WKPageConfigurationRef, const TestOptions& options)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to