WebKit Bugzilla
Attachment 362427 Details for
Bug 194829
: [WKTR] Avoid starting new NetworkProcesses unnecessarily when running the layout tests
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194829-20190219141921.patch (text/plain), 17.52 KB, created by
Chris Dumez
on 2019-02-19 14:19:22 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2019-02-19 14:19:22 PST
Size:
17.52 KB
patch
obsolete
>Subversion Revision: 241761 >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 84fe3d6f546a00479cb1b4907ca1cf3d7c016267..3469b760617570bdb4d5b297b6a5020adf6b2f10 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,32 @@ >+2019-02-19 Chris Dumez <cdumez@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 NOBODY (OOPS!). >+ >+ 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-19 Pablo Saavedra <psaavedra@igalia.com> > > pytest is not correctly auto-installed >diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp >index 10aebcfd4bb9226c9818fee26ee6c00d17ac1794..0578393684f82a97096f1f209975ecdf8e652c9e 100644 >--- a/Tools/WebKitTestRunner/TestController.cpp >+++ b/Tools/WebKitTestRunner/TestController.cpp >@@ -454,13 +454,24 @@ void TestController::initialize(int argc, const char* argv[]) > 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()); > 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,29 +486,33 @@ WKRetainPtr<WKContextConfigurationRef> TestController::generateContextConfigurat > 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()); >+ 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); > >- auto* websiteDataStore = WKContextGetWebsiteDataStore(m_context.get()); >- WKWebsiteDataStoreSetCacheStoragePerOriginQuota(websiteDataStore, 400 * 1024); >- >- platformInitializeContext(); >+ platformInitializeContext(); >+ } >+ >+ m_geolocationProvider = std::make_unique<GeolocationProviderMock>(m_context.get()); > > WKContextInjectedBundleClientV1 injectedBundleClient = { > { 1, this }, >@@ -547,20 +562,7 @@ WKRetainPtr<WKPageConfigurationRef> TestController::generatePageConfiguration(WK > > void TestController::createWebViewWithOptions(const TestOptions& options) > { >- auto contextConfiguration = generateContextConfiguration(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()); >+ auto configuration = generatePageConfiguration(options); > > // 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. >@@ -736,7 +738,7 @@ void TestController::resetPreferencesToConsistentValues(const TestOptions& optio > 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 @@ static void updateTestOptionsFromTestHeader(TestOptions& testOptions, const std: > } > > 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 @@ static void updateTestOptionsFromTestHeader(TestOptions& testOptions, const std: > 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 @@ static void updateTestOptionsFromTestHeader(TestOptions& testOptions, const std: > 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; > } > } >diff --git a/Tools/WebKitTestRunner/TestController.h b/Tools/WebKitTestRunner/TestController.h >index b43c55c28665dfe65f42215aebc40271b3254c64..a0c3a72e782030c862ed3b040688f7ea47fbac07 100644 >--- a/Tools/WebKitTestRunner/TestController.h >+++ b/Tools/WebKitTestRunner/TestController.h >@@ -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 @@ public: > 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 @@ private: > > std::unique_ptr<PlatformWebView> m_mainWebView; > WKRetainPtr<WKContextRef> m_context; >+ Optional<TestOptions::ContextOptions> m_contextOptions; > WKRetainPtr<WKPageGroupRef> m_pageGroup; > WKRetainPtr<WKUserContentControllerRef> m_userContentController; > >diff --git a/Tools/WebKitTestRunner/TestOptions.h b/Tools/WebKitTestRunner/TestOptions.h >index 392d8e70b409e88e5924e3fddc0de77628dd91d4..b0fed723a273d8b9fe7f6e866b1700d97834757e 100644 >--- a/Tools/WebKitTestRunner/TestOptions.h >+++ b/Tools/WebKitTestRunner/TestOptions.h >@@ -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 @@ struct TestOptions { > 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 @@ struct TestOptions { > 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 @@ struct TestOptions { > { > if (useThreadedScrolling != options.useThreadedScrolling > || useAcceleratedDrawing != options.useAcceleratedDrawing >- || overrideLanguages != options.overrideLanguages > || useMockScrollbars != options.useMockScrollbars > || needsSiteSpecificQuirks != options.needsSiteSpecificQuirks > || useCharacterSelectionGranularity != options.useCharacterSelectionGranularity >@@ -107,8 +126,6 @@ struct TestOptions { > || 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,8 +136,10 @@ struct TestOptions { > || 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) >@@ -131,11 +150,6 @@ struct TestOptions { > > return true; > } >- >- bool shouldEnableProcessSwapOnNavigation() const >- { >- return enableProcessSwapOnNavigation || enableProcessSwapOnWindowOpen; >- } > }; > > } >diff --git a/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm b/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm >index fe3bd6e0e5484cf83233b727fb2c1d56293beadf..be5fec43332775ac12d5c5ee71749a3e8651a758 100644 >--- a/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm >+++ b/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm >@@ -142,9 +142,9 @@ WKPreferencesRef TestController::platformPreferences() > 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)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=362427">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194829
:
362427
|
362453
|
362499