[webkit-reviews] review requested: [Bug 20534] DumpRenderTree needs a way to override settings on a per-test basis : [Attachment 27296] Alternate patch to issue 20534

[bugzilla-daemon at webkit.org]

Glenn Wilson <gwilson at google.com> has asked Mark Rowe (bdash) <mrowe at apple.com>
for review:
Bug 20534: DumpRenderTree needs a way to override settings on a per-test basis
https://bugs.webkit.org/show_bug.cgi?id=20534

Attachment 27296: Alternate patch to issue 20534
https://bugs.webkit.org/attachment.cgi?id=27296&action=review

------- Additional Comments from Glenn Wilson <gwilson at google.com>
Here's a modified version of the override preference patch that does not have
any resetToDefault method in WebPreferences.  Instead, this relies on
DumpRenderTree to reset preferences properly between tests.

DumpRenderTree.mm was setting preference values in two places --
setDefaultsToConsistentValueForTesting and also resetWebViewToConsistentState. 
setDefaults was being called once when DRT starts, and resetWebView between
every test.  Since each layout test assumes that its preferences should be set
to their default 'testing' values when it is run, I consolidated all of the web
preference resetting to setDefaults and changed resetWebView to call
setDefaults.

This patch assumes that if a preference is meant to have a default 'testing'
value, it will always be reset in DumpRenderTree.  Thus, if a preference is
added in the future that will need to be reset between tests, setDefaults
should be modified to reset the value of the preference.

I originally wanted to destroy and re-create the instance of WebPreference
between tests.	This is equivalent to resetting the values to their 'testing
state' and getting the rest of the preferences reset as a side effect. 
However, the preference values that should be used for testing can often be
different than the default values in WebPreferences.  Moreover, if a preference
must have a default testing state, it should be clearly stated as such in
DumpRenderTree::setDefaultsToConsistentValueForTesting.  In short, I felt that
just re-instantiating WebPreferences "hides" the desired 'testing state' of
these preferences.

What do you think?  What can I improve?