Bug 95578 – [WK2][WTR] CodeGeneratorTestRunner should support web idl overloading functions.

• Status: NEW
• Component: Tools / Tests

[Kangil Han]

// FIXME: handle non-boolean preferences.
void overridePreference(in DOMString preference, in boolean value);

[Kangil Han]

Created attachment 162028 [details]
patch

[Kangil Han]

ap : review please?

[Benjamin Poulain]

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.

[Kangil Han]

(In reply to comment #3)
> 
> This is not maintainable.

Agreed. I will change CodeGenerator to support overloading method.

[Jussi Kukkonen (jku)]

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.

[Jussi Kukkonen (jku)]

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...

[Kangil Han]

(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. :)

[Benjamin Poulain]

> 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"?

[Jussi Kukkonen (jku)]

(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.