Bug 95578 - [WK2][WTR] CodeGeneratorTestRunner should support web idl overloading functions.
Summary: [WK2][WTR] CodeGeneratorTestRunner should support web idl overloading functions.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 42197
  Show dependency treegraph
 
Reported: 2012-08-31 08:23 PDT by Kangil Han
Modified: 2017-05-12 06:48 PDT (History)
7 users (show)

See Also:


Attachments
patch (9.68 KB, patch)
2012-09-04 06:24 PDT, Kangil Han
benjamin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kangil Han 2012-08-31 08:23:29 PDT
// FIXME: handle non-boolean preferences.
void overridePreference(in DOMString preference, in boolean value);
Comment 1 Kangil Han 2012-09-04 06:24:00 PDT
Created attachment 162028 [details]
patch
Comment 2 Kangil Han 2012-09-04 23:41:03 PDT
ap : review please?
Comment 3 Benjamin Poulain 2012-09-07 21:46:07 PDT
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.
Comment 4 Kangil Han 2012-09-07 21:47:53 PDT
(In reply to comment #3)
> 
> This is not maintainable.

Agreed. I will change CodeGenerator to support overloading method.
Comment 5 Jussi Kukkonen (jku) 2012-12-13 04:35:41 PST
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.
Comment 6 Jussi Kukkonen (jku) 2012-12-13 04:41:21 PST
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...
Comment 7 Kangil Han 2012-12-13 04:48:37 PST
(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. :)
Comment 8 Benjamin Poulain 2012-12-13 13:53:37 PST
> 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"?
Comment 9 Jussi Kukkonen (jku) 2012-12-14 03:02:51 PST
(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.