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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 30 17:01:45 PST 2008


Glenn Wilson <gwilson at google.com> has asked Eric Seidel <eric at webkit.org> 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 26328: Revised patch to issue 20534
https://bugs.webkit.org/attachment.cgi?id=26328&action=review

------- Additional Comments from Glenn Wilson <gwilson at google.com>
Here is a revised patch that may address this issue.

resetToDefaults in WebPreferences.mm was calling init, which is definitely bad.
 I didn't know what other interactions init would have, so hopefully I know
better now.  I modified resetToDefaults to work much like the Win version of
WebPreferences...it gets a dictionary of all the default values, loops through
each entry, and resets each key/value pair to the default.

To do this, I had to break out the default dictionary into its own method that
just returns the dict, and have "initialize" call this method.	This seemed
acceptable, since this is not an instance method, no interface changes were
necessary.  I didn't want to break it out, but using
NSUserDefaults::standardUserDefaults as the default preference value
'repository' didn't seem to work.  (Possibly because WebPreferences::initialize
is never called when running layout tests?)

Eric, I also tried to incorporate some of the cleanup you did elsewhere --
removing all the tabs I could find, including reviewer lines in ChangeLogs,
etc.  Let me know what others I missed so I can clean them up for later
iterations.

Thanks for the feedback, and let me know what else should be improved here.


More information about the webkit-reviews mailing list