[webkit-reviews] review denied: [Bug 40227] Web Inspector: Provide detailed network info in the resources panel. : [Attachment 60931] [PATCH] Same with WebKit/chromium/*/WebURLLoadTiming

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 8 12:59:31 PDT 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 40227: Web Inspector: Provide detailed network info in the resources panel.
https://bugs.webkit.org/show_bug.cgi?id=40227

Attachment 60931: [PATCH] Same with WebKit/chromium/*/WebURLLoadTiming
https://bugs.webkit.org/attachment.cgi?id=60931&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebKit/chromium/public/WebURLResponse.h:76
 +	WEBKIT_API void setConnectionID(unsigned);
nit: please make these attributes readable as well.

WebKit/chromium/src/WebURLResponse.cpp:103
 +     
m_private->m_resourceResponse->setResourceLoadTiming(ResourceLoadTiming::create
());
why should setting a connection ID set an implicit ResourceLoadTiming object?

WebKit/chromium/src/WebURLResponse.cpp:109
 +	ASSERT(loadTiming.get());
why is it invalid to set a null loadTiming object?

WebKit/chromium/public/WebURLLoadTiming.h:60
 +	WEBKIT_API void setDomainLookupTimes(double start, double end);
i know it means more typing for you, but individual setters and getters
for these properties is really the way to go.  that way, this API is
more flexible and potentially useful for future applications without
requiring an API change.  these functions will get optimized away by
the compiler :)

I did not review any of the inspector changes.


More information about the webkit-reviews mailing list