Summary: Internals should not reset RuntimeEnabledFeatures, they should be set consistently by TestRunners -- Tests are very confused about RuntimeEnabledFeatures. DumpRenderTree: - DumpRenderTree.mm (Mac): resetWebPreferencesToConsistentValues sets a number of WebPreferences APIs that toggle RuntimeEnabledFeatures - DumpRenderTree.cpp (Win): resetWebPreferencesToConsistentValues sets a number of WebPreferences APIs that toggle RuntimeEnabledFeatures WebKitTestRunner: - TestController.cpp (All): TestController::resetPreferencesToConsistentValues toggles RuntimeEnabledFeatures (WKPreferencesEnableAllExperimentalFeatures) WebCoreTestSupport: - Internals.cpp (All): Internals::Internals resets RuntimeEnabledFeatures without taking into account DumpRenderTree/WebKitTestRunner non-defaults - Internals.cpp (All): Various APIs toggle RuntimeEnabledFeatures while a test is running Internals stomps on the Test Runners values, but only sometimes. This results in very confusing results where sometimes a Runtime feature is enabled and sometimes disabled. I'm going to propose some guidelines: 1. DumpRenderTree / WebKitTestRunner should configure settings appropriately - They already do between tests with the appropriately named resetPreferencesToConsistentValues. - This allows our tests to have things like experimental features enabled for tests even if they are not enabled by default for ports. - WebKit2 currently does this automatically. - WebKit1 developers / reviewers will need to remember to follow the pattern. 2. Internals should not reset RuntimeEnabledFeatures - The values in RuntimeEnabledFeatures::reset may not match the defaults set by TestRunners. - Internals doesn't know what features the TestRunners will want. 3. Internal toggle APIs can exist - If a test really wants to enable/disable a runtime feature this is a quick and easy way to do it. This should be, and is, super rare. - Tests shouldn't need to remember to restore the default value. The test runners should already be doing that, see (1). Possible issues: - There may be a RuntimeEnabled feature for which there is no WebPreferences API to toggle it. - Seems in these cases we should just add an API.
Created attachment 299205 [details] [PATCH] Proposed Fix Lets see what the bots think.
Comment on attachment 299205 [details] [PATCH] Proposed Fix Attachment 299205 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2912331 New failing tests: imported/w3c/web-platform-tests/resource-timing/resource_dynamic_insertion.html http/tests/cache/vary-frameless-document.html imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse.html imported/w3c/web-platform-tests/resource-timing/resource_cached.htm
Created attachment 299207 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Ugh. It seems I'm not going to be able to separate this from making a flag for ResourceTiming because existing ResourceTiming tests depend on this reset happening. Adding a runtime toggle for the test runners is the same as bug 167147.
Comment on attachment 299205 [details] [PATCH] Proposed Fix Attachment 299205 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2912406 New failing tests: imported/w3c/web-platform-tests/resource-timing/resource_dynamic_insertion.html http/tests/cache/vary-frameless-document.html imported/w3c/web-platform-tests/resource-timing/resource_cached.htm js/dom/global-constructors-attributes.html imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse.html
Created attachment 299211 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 299205 [details] [PATCH] Proposed Fix Attachment 299205 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2912407 New failing tests: imported/w3c/web-platform-tests/resource-timing/resource_dynamic_insertion.html http/tests/cache/vary-frameless-document.html imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse.html imported/w3c/web-platform-tests/resource-timing/resource_cached.htm
Created attachment 299212 [details] Archive of layout-test-results from ews113 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 299205 [details] [PATCH] Proposed Fix Attachment 299205 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2912384 New failing tests: imported/w3c/web-platform-tests/resource-timing/resource_dynamic_insertion.html imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse.html imported/w3c/web-platform-tests/resource-timing/resource_cached.htm
Created attachment 299213 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
So glad you're fixing this!
I'm going to fold this into bug 167147. Some tests enable ResourceTiming and was no way to disable ResourceTiming from TestRunners. Rather then repeat the work across two bugs, I'm putting it all in one. *** This bug has been marked as a duplicate of bug 167147 ***
Now that I'm separating the original up into smaller pieces I'll reuse this one for the task in the title.
This was handled by bug 167228. *** This bug has been marked as a duplicate of bug 167228 ***