[webkit-dev] Settings and Preferences in layout tests

Ryosuke Niwa rniwa at webkit.org
Wed Sep 26 13:48:57 PDT 2012

On Wed, Sep 26, 2012 at 1:44 PM, Simon Fraser <simon.fraser at apple.com>wrote:

> We have a lot of tests that poke internal settings, via testRunner:
>     testRunner.setFrameFlatteningEnabled(true);
> or window.internals:
>     internals.settings.setPageScaleFactor(0.5, 0, 0);
> and some that poke preferences, like:
>     testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
> 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?

Secondly, I see no guarantee that these settings and preferences are reset
> to their default values before the next test, and we don't seem to have a
> consistent strategy about how to do this.

That sounds really bad. We should fix that.

For internal settings, we have InternalSettings::Backup methods. However,
> there's no enforcement that when changing a setting in a test for the first
> time, developers also add code to reset it in InternalSettings.
> I looked at testRunner.overridePreference(), and it doesn't appear to
> reset the value at the end of the test.
> So I think we need clearer rules here. I suggest:
> * testRunner methods that just set preferences should migrate to
> internals.settings or testRunner.overridePreference
> * we should choose between internals.settings or
> testRunner.overridePreference if that makes sense.
> * we should enforce a policy that patches adding a settings/prefs toggle
> should, if necessary, add code to reset between tests.

Sounds good to me. I'm surprised that the third point isn't already
enforced. It seems like that'll be a review failure.

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

More information about the webkit-dev mailing list