[webkit-dev] Settings and Preferences in layout tests
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:
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the webkit-dev