[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