On Sep 26, 2012, at 2:46 PM, Tony Chang <[email protected]> wrote:
> On Wed, Sep 26, 2012 at 2:35 PM, Brady Eidson <[email protected]> wrote:
> On Sep 26, 2012, at 2:05 PM, Adam Barth <[email protected]> wrote:
>> [re-sent from the proper address]
>>
>> On Wed, Sep 26, 2012 at 2:00 PM, Adam Barth <abarth@nowhere> wrote:
>> On Wed, Sep 26, 2012 at 1:53 PM, Brady Eidson <[email protected]> wrote:
>> On Sep 26, 2012, at 1:48 PM, Ryosuke Niwa <[email protected]> wrote:
>>> On Wed, Sep 26, 2012 at 1:44 PM, Simon Fraser <[email protected]>
>>> 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.
> In addition to being a lot of work, it increases the size of each WebKit API
> even if the particular port doesn't want/need to expose the feature.
If a port doesn't want the feature, it can't be tested and needs to be skipped
anyways, right?
~Brady
_______________________________________________
webkit-dev mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-dev
