On Sep 26, 2012, at 4:13 PM, Brady Eidson <beid...@apple.com> wrote:
>
> On Sep 26, 2012, at 2:46 PM, Tony Chang <t...@chromium.org> wrote:
>
>> On Wed, Sep 26, 2012 at 2:35 PM, Brady Eidson <beid...@apple.com> wrote:
>> On Sep 26, 2012, at 2:05 PM, Adam Barth <aba...@webkit.org> 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 <beid...@apple.com> wrote:
>>> On Sep 26, 2012, at 1:48 PM, Ryosuke Niwa <rn...@webkit.org> wrote:
>>>> On Wed, Sep 26, 2012 at 1:44 PM, Simon Fraser <simon.fra...@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
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
