[webkit-reviews] review denied: [Bug 58139] Web Inspector: No headers information in network panel for downloads. : [Attachment 88827] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 11 22:03:12 PDT 2011


Pavel Feldman <pfeldman at chromium.org> has denied Vsevolod Vlasov
<vsevik at chromium.org>'s request for review:
Bug 58139: Web Inspector: No headers information in network panel for
downloads.
https://bugs.webkit.org/show_bug.cgi?id=58139

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88827&action=review

A couple of nits, otherwise looks good.

> LayoutTests/http/tests/inspector/network/download-expected.txt:1
> +http://127.0.0.1:8000/inspector/network/resources/download.zzz -
willSendRequest <NSURLRequest URL
http://127.0.0.1:8000/inspector/network/resources/download.zzz, main document
URL http://127.0.0.1:8000/inspector/network/download.html, http method GET>
redirectResponse (null)

This looks platform specific.

> Source/WebCore/loader/MainResourceLoader.cpp:261
> +	  
InspectorInstrumentation::didReceiveResourceResponseButCanceled(m_frame.get(),
documentLoader(), identifier(), r);

You should provide as much context into the signal as possible, in this case I
would call it "continueWithPolicyDownload" or similar.

> Source/WebCore/loader/MainResourceLoader.cpp:274
> +	  
InspectorInstrumentation::didReceiveResourceResponseButCanceled(m_frame.get(),
documentLoader(), identifier(), r);

ditto


More information about the webkit-reviews mailing list