[webkit-reviews] review denied: [Bug 31204] WebInspector: RR for unit test of timeline data : [Attachment 42670] WebInspector: Adds a unit test for Timeline records
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Nov 6 14:27:12 PST 2009
Pavel Feldman <pfeldman at chromium.org> has denied Eric Ayers
<zundel at google.com>'s request for review:
Bug 31204: WebInspector: RR for unit test of timeline data
https://bugs.webkit.org/show_bug.cgi?id=31204
Attachment 42670: WebInspector: Adds a unit test for Timeline records
https://bugs.webkit.org/attachment.cgi?id=42670&action=review
------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
> + // Synchronous opening of web inspector may fail on Windows
Wait a minute, we don't want that - we do run these on windows bots and need
tests to succeed. You'll need to put stuff under onload / ignoreLoad into
setTimeout to queue I guess once this one is reverted.
> + // Invoke a setup method if it has been specified
> + toInject.push("frontend_setup ? frontend_setup() : undefined");
"(if (frontend_setup) frontend_setup();)" ?
General comments:
1. { } are used for single-line ifs all over the place
2. Use === instead of == for primitive types
Otherwise looks good.
Why added svn:executable flag?
More information about the webkit-reviews
mailing list