|
|
Created:
6 years, 7 months ago by eustas Modified:
6 years, 6 months ago Reviewers:
vsevik CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Visibility:
Public. |
DescriptionDevTools: show HTTP headers of cached resources in network panel.
Screenshot: http://picpaste.com/RaGXNCAZ.png
BUG=91276
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175535
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed comments #
Total comments: 4
Patch Set 3 : Addressed comments #Patch Set 4 : Fixed test #
Messages
Total messages: 43 (0 generated)
https://chromiumcodereview.appspot.com/300913002/diff/1/Source/devtools/front... File Source/devtools/front_end/networkPanel.css (left): https://chromiumcodereview.appspot.com/300913002/diff/1/Source/devtools/front... Source/devtools/front_end/networkPanel.css:142: .resource-headers-view .outline-disclosure li .status-from-cache { This stile is still used in RequestHeadersView https://chromiumcodereview.appspot.com/300913002/diff/1/Source/devtools/front... File Source/devtools/front_end/networkPanel.css (right): https://chromiumcodereview.appspot.com/300913002/diff/1/Source/devtools/front... Source/devtools/front_end/networkPanel.css:114: color: #888; Could you please provide a screenshot for this UI change?
What kind of headers will we show in this case? I would assume that like for a network case there are provisional headers (sent by the renderer) and the actual headers that were sent previously when we sent the original network request. The latter seems more valuable.
On 2014/05/28 17:40:38, vsevik wrote: > What kind of headers will we show in this case? > I would assume that like for a network case there are provisional headers (sent > by the renderer) and the actual headers that were sent previously when we sent > the original network request. The latter seems more valuable. It depends. There are 2 cases: 1) Renderer process sends request and misses the disc cache. Then (after page refresh) same renderer process serves request from memory cache. In this scenario actual request headers will be shown. 2) Renderer process sends request and hits the disc cache. In this case provisional headers are shown (because request headers are not persisted).
https://codereview.chromium.org/300913002/diff/1/Source/devtools/front_end/ne... File Source/devtools/front_end/networkPanel.css (left): https://codereview.chromium.org/300913002/diff/1/Source/devtools/front_end/ne... Source/devtools/front_end/networkPanel.css:142: .resource-headers-view .outline-disclosure li .status-from-cache { On 2014/05/28 17:34:44, vsevik wrote: > This stile is still used in RequestHeadersView Done. https://codereview.chromium.org/300913002/diff/1/Source/devtools/front_end/ne... File Source/devtools/front_end/networkPanel.css (right): https://codereview.chromium.org/300913002/diff/1/Source/devtools/front_end/ne... Source/devtools/front_end/networkPanel.css:114: color: #888; On 2014/05/28 17:34:44, vsevik wrote: > Could you please provide a screenshot for this UI change? http://picpaste.com/RaGXNCAZ.png
On 2014/05/29 07:10:26, eustas wrote: > On 2014/05/28 17:40:38, vsevik wrote: > > What kind of headers will we show in this case? > > I would assume that like for a network case there are provisional headers > (sent > > by the renderer) and the actual headers that were sent previously when we sent > > the original network request. The latter seems more valuable. > > It depends. There are 2 cases: > > 1) Renderer process sends request and misses the disc cache. > Then (after page refresh) same renderer process serves request from memory > cache. > In this scenario actual request headers will be shown. > > 2) Renderer process sends request and hits the disc cache. > In this case provisional headers are shown (because request headers are not > persisted). Is it technically possible that the "actual" and "provisional" headers may actually be different? Or the renderer would invalidate the disc cache so that this should never happen? If they can be different, I think we can hint about it in the UI, like we already do with "(from cache)" label?
Ping
lgtm https://chromiumcodereview.appspot.com/300913002/diff/10001/Source/devtools/f... File Source/devtools/front_end/network/RequestHeadersView.js (right): https://chromiumcodereview.appspot.com/300913002/diff/10001/Source/devtools/f... Source/devtools/front_end/network/RequestHeadersView.js:438: var showCaution = provisionalHeaders && !this._request.cached; I think we should always show this warning for provisional headers. https://chromiumcodereview.appspot.com/300913002/diff/10001/Source/devtools/f... File Source/devtools/front_end/networkPanel.css (right): https://chromiumcodereview.appspot.com/300913002/diff/10001/Source/devtools/f... Source/devtools/front_end/networkPanel.css:114: color: #888; I don't think we need this. This makes text unreadable. I think (from cache) in status line is enough. We could probably make the status line gray instead.
https://codereview.chromium.org/300913002/diff/10001/Source/devtools/front_en... File Source/devtools/front_end/network/RequestHeadersView.js (right): https://codereview.chromium.org/300913002/diff/10001/Source/devtools/front_en... Source/devtools/front_end/network/RequestHeadersView.js:438: var showCaution = provisionalHeaders && !this._request.cached; On 2014/06/02 14:49:27, vsevik wrote: > I think we should always show this warning for provisional headers. Done. https://codereview.chromium.org/300913002/diff/10001/Source/devtools/front_en... File Source/devtools/front_end/networkPanel.css (right): https://codereview.chromium.org/300913002/diff/10001/Source/devtools/front_en... Source/devtools/front_end/networkPanel.css:114: color: #888; On 2014/06/02 14:49:27, vsevik wrote: > I don't think we need this. > This makes text unreadable. > I think (from cache) in status line is enough. > We could probably make the status line gray instead. Done.
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/300913002/30001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6754) linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/9996) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10299)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/300913002/30001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10014) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10314)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10029)
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/300913002/30001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10042) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10342)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/300913002/30001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10074) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10342)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10085)
PTAL
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/300913002/50001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6888) linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10205) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10533) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/12684)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10213)
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/300913002/50001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/300913002/50001
Message was sent while issue was closed.
Change committed as 175535 |