[webkit-dev] Settings and Preferences in layout tests

[Simon Fraser]

On Sep 26, 2012, at 4:13 PM, Brady Eidson <beidson at apple.com> wrote:

> 
> On Sep 26, 2012, at 2:46 PM, Tony Chang <tony at chromium.org> wrote:
> 
>> On Wed, Sep 26, 2012 at 2:35 PM, Brady Eidson <beidson at apple.com> wrote:
>> On Sep 26, 2012, at 2:05 PM, Adam Barth <abarth at webkit.org> wrote:
>>> [re-sent from the proper address]
>>> 
>>> On Wed, Sep 26, 2012 at 2:00 PM, Adam Barth <abarth at nowhere> wrote:
>>> On Wed, Sep 26, 2012 at 1:53 PM, Brady Eidson <beidson at apple.com> wrote:
>>> On Sep 26, 2012, at 1:48 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>>>> On Wed, Sep 26, 2012 at 1:44 PM, Simon Fraser <simon.fraser at apple.com> wrote:
>>>> 
>>>>  First, direct calls on testRunner that just set preferences should be migrated to internals.settings or testRunner.overridePreference calls, I think (I don't know if either is preferred).
>>>> 
>>>> I support the idea of unifying the approaches and just use internals.settings. However, the last time I checked, Alexey had some concerns about using internals due to settings may not be properly propagated to WebKit2 layer. Has this concern been addressed?
>>> 
>>> In general I prefer the overridePreference() calls whenever they exist.
>>> 
>>> internals.settings are not exposed in any real-world product whereas preferences exist in each platform's WebKit-layer API that they expose to their embedders in some form.
>>> 
>>> The main downside of overridePreference is that it requires that you expose an API for twiddling the preference on every port.  That can lead to us exposing unneeded APIs (making them harder to remove) and to a bunch of port-specific code in an otherwise port-independent patch.
>>> 
>>> IMHO, we should prefer InternalSettings unless we need to test the WebKit-layer code.
>> 
>> I suppose we're biased in Mac-land where the mechanism originated, but the "API" is simply a single string based API that only had to be implemented once.
>> 
>> Your comment leads me to understand that at least one other port doesn't have simple string based preferences.
>> 
>> Of course to add *any* new internal setting new code must be added specifically for that setting...
>> 
>> Of course that code only has to be added once for all platforms…
>> 
>> I think for all the major ports, they are simple string based preferences.  However, adding a new overridePreference call means updating 5 WebKit API interfaces (Mac, Win, Chromium, GTK+, QT),  and updating 5 DRTs and 1 WTR.  Compared to updating a single internal.settings implementation, this is a lot of work.
> 
> I think you misunderstand what I meant.
> 
> The Mac DRT implementation of overridePreference has the following implementation:
> 
> void TestRunner::overridePreference(JSStringRef key, JSStringRef value)
> {
>     RetainPtr<CFStringRef> keyCF(AdoptCF, JSStringCopyCFString(kCFAllocatorDefault, key));
>     NSString *keyNS = (NSString *)keyCF.get();
> 
>     RetainPtr<CFStringRef> valueCF(AdoptCF, JSStringCopyCFString(kCFAllocatorDefault, value));
>     NSString *valueNS = (NSString *)valueCF.get();
> 
>     [[WebPreferences standardPreferences] _setPreferenceForTestWithValue:valueNS forKey:keyNS];
> }
> 
> This works for any preference;  Even a new one that has never been twiddled in a regression test before.
> 
> For example in http://trac.webkit.org/changeset/127956 we added a new test that twiddled the "WebKitStorageBlockingPolicy" preference and we didn't need to change any DRT Mac code to accomplish this.
> 
> Compared to adding a single implementation to internal.settings, this was *NO* additional work.

But is there code to undo this pref change for subsequent tests?

Simon


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120926/432d7e81/attachment.html>