[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