Bug 95578

Created attachment 162028 [details] patch

ap : review please?

Comment on attachment 162028 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=162028&action=review r- as discussed on IRC. > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:213 > + if (preference == "WebKit2AsynchronousPluginInitializationEnabled" || preference == "WebKit2AsynchronousPluginInitializationEnabledForAllPlugins" > + || preference == "WebKit2ArtificialPluginInitializationDelayEnabled" || preference == "WebKitAcceleratedCompositingEnabled" > + || preference == "WebKitCSSCustomFilterEnabled" || preference == "WebKitCSSRegionsEnabled" > + || preference == "WebKitCSSGridLayoutEnabled" || preference == "WebKitJavaEnabled" > + || preference == "WebKitJavaScriptEnabled" || preference == "WebKitLoadSiteIconsKey" > + || preference == "WebKitOfflineWebApplicationCacheEnabled" || preference == "WebKitPageCacheSupportsPluginsPreferenceKey" > + || preference == "WebKitPluginsEnabled" || preference == "WebKitUsesPageCachePreferenceKey" > + || preference == "WebKitWebAudioEnabled" || preference == "WebKitWebGLEnabled" > + || preference == "WebKitXSSAuditorEnabled" || preference == "WebKitShouldRespectImageOrientation" > + || preference == "WebKitEnableCaretBrowsing" || preference == "WebKitAcceleratedCompositingEnabled") > + return overrideBoolPreferenceForTestRunner(pageGroup, preference, toBool(value)); This is not maintainable.

(In reply to comment #3) > > This is not maintainable. Agreed. I will change CodeGenerator to support overloading method.

Seo Sanghyeon, Kangil: hi. Is one of you working on this or should I take a stab? I think I have a vision of how this could be done.

Actually, how many tests really use non-boolean overridePreference()? I can only see a couple on a quick look through. This might not be worth it if it would only be used in a few tests, and there are no other overloaded IDL functions we need...

(In reply to comment #6) > Actually, how many tests really use non-boolean overridePreference()? I can only see a couple on a quick look through. > > This might not be worth it if it would only be used in a few tests, and there are no other overloaded IDL functions we need... I am okay if benjamin would agree with you. :)

> This might not be worth it if it would only be used in a few tests, and there are no other overloaded IDL functions we need... Can you please quantify "a few tests"?

(In reply to comment #8) > > This might not be worth it if it would only be used in a few tests, and there are no other overloaded IDL functions we need... > > Can you please quantify "a few tests"? I have not checked each preference yet. I can tell that only three tests use overrridePreference() with a value other than [true, false, 0, 1, "0", "1"]: LayoutTests/fast/harness/override-preferences-2.html: testRunner.overridePreference("WebKitDefaultFontSize", "24") LayoutTests/fast/harness/override-preferences-2.html: testRunner.overridePreference("WebKitMinimumFontSize", "12"); LayoutTests/http/tests/download/default-encoding.html: testRunner.overridePreference("WebKitDefaultTextEncodingName", "koi8-r"); Btw, as I was checking things I realized that we have 99 webgl tests in wk2/TestExpectations with this comment: # WebKit2 needs layoutTestController.overridePreference # <https://bugs.webkit.org/show_bug.cgi?id=42197> My understanding was that boolean preferences already work and it looks like these tests only use a boolean override... I run the tests and 82/99 tests pass on wk2-efl.