[webkit-reviews] review denied: [Bug 40774] Web Inspector: implement layout tests for debugger : [Attachment 59258] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 22 01:10:21 PDT 2010


Pavel Feldman <pfeldman at chromium.org> has denied Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 40774: Web Inspector: implement layout tests for debugger
https://bugs.webkit.org/show_bug.cgi?id=40774

Attachment 59258: Patch
https://bugs.webkit.org/attachment.cgi?id=59258&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
LayoutTests/http/tests/inspector/inspector-test.js:35
 +	    if (window.layoutTestController) {
You don't need this change, right? Same here. We should rely on DRT. Is there a
reason we can't trust it?

LayoutTests/http/tests/inspector/inspector-test.js:107
 +	var toolbar = document.getElementById("toolbar");
What is wrong with assigning to the current panel? (that is what IC is doing by
means of WebInspector.show*Panel calls.

WebCore/platform/mac/EventLoopMac.mm:35
 +	NSTimeInterval interval = [[NSDate date]
timeIntervalSinceReferenceDate];
I am hesitant getting into this code / reviewing it in connection with
inspector tests. Why did you change this? Does it work on all other platforms?
(You are only changing the mac's platform). What non-inspector flows will it
affect? r- until this is cleared properly.


More information about the webkit-reviews mailing list