Closed Bug 764958 Opened 13 years ago Closed 10 years ago

Network monitor doesn't show network requests for cached content

Categories

(DevTools :: Netmonitor, defect, P2)

defect

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: tanvi, Assigned: jlong)

References

(Blocks 1 open bug)

Details

(Whiteboard: [parity-firebug][polish-backlog])

Attachments

(1 file, 7 obsolete files)

Webconsole's net panel should include cached content. Perhaps with a link a message that shows the content was retrieved from the cache and a network request was not made.
Mihai - have you had a chance to take a look at this? If cache'd entries showed up in the webconsole, it would save me from constantly clearing my cache or doing a lot of shift-refresh's ;)
Tanvi: Unfortunately I've been really busy with work for bug 768096. :(
What's the use case here? Why is this important to show in the web console?
Flags: needinfo?(tanvi)
If I open the webconsole and go to https://people.mozilla.com/~bsterne/tests/62178/ in my browser, I get 5 GET requests in the webconsole. Then when I click on test.html, no new entries show up in the webconsole since I've been to test.html so many times. If I then again manually navigate to https://people.mozilla.com/~bsterne/tests/62178/, I get 1 GET request in the webconsole. (Note that I am not familiar with Firefox's cache implementation, and hence can't comment on what this means.) When you open a new tab/window and visit a page that is in your cache, the webconsole is empty. The developer cannot identity what resources were loaded (js, images, css, etc) unless they refresh the page. One place that the webconsole could retrieve this information is from about:cache-entry. For example: about:cache-entry?client=HTTP&sb=1&key=https://people.mozilla.com/~bsterne/tests/62178/test.html (Note that this is not in regard to back and forth events in the browser. If the user hits the back or forward buttons, the webconsole does not need to re-populate itself.) Here are two potential use cases: * A developer is trying to identify all the mixed content on their pages. They open webconsole and go to their website (that they have been to many many times and hence is probably cached). They observe 3 network logging events in the webconsole (perhaps for content that had a no-cache header) and identity one mixed content issue. They fix that issue, and then later learn from their users that their page still has mixed content on it. They go to their website again and open webconsole, but webconsole shows no issues. The tool does not provide the data the developer is looking for because it doesn't provide data for the cached resources. * A developer is trying to debug an report that an image is not loading correctly on their webpage. They open webconsole and visit their webpage. The image is cached and does not show up in the webconsole. They may or may not realize that they need to refresh the page in order to re-request the page. If the user does realize this, they will re-request the content with a refresh, inspect the network request and find that the content-type was set to text/html instead of image/jpeg. If the user does not realize that he or she is supposed to refresh the page, then they may try out a different developer tool to help them debug the problem (perhaps an add-on or tools on another browser).
Flags: needinfo?(tanvi) needinfo?(tanvi) → needinfo+
Really interested in a fix for this bug. When debugging pages that rely on CORS it's very important to be able to easily see HTTP request/response headers, regardless of caching.
Tanvi: isn't this a duplicate of bug 764960?
Component: Developer Tools: Console → Developer Tools: Netmonitor
Flags: needinfo?(tanvi)
Summary: Webconsole Doesn't Show network requests for cached content → Network monitor doesn't show network requests for cached content
Hi Mihai, This was the original bug and main issue I was having. Bug 764960 was a secondary issue (I'm less bothered about missing entries in the console when the user explicitly goes into offline mode). However, perhaps the code to fix these two bugs is the same.
Flags: needinfo?(tanvi)
Priority: -- → P3
Hardware: x86 → All
The shift-reload workaround doesn't even work with XHR requests like the one on the following page to graphs.mozilla.org: http://perf.snarkfest.net/compare-talos/index.html?oldRevs=69bbd8165e4e&newRev=67838efec1f6&submit=true This made it impossible for me to find out what URL was being requested without clearing the cache. I think this should be higher than P3 since people expect this to work as it has in Firebug for a long time.
Whiteboard: [parity-firebug]
Priority: P3 → P2
Just tripped over this "bug" as well. If you look at the browsers' behavior closely, IE, FF and Chrome all differ: When fetching cached content, IE reports a server(!) response of 304 (not modified). However, it isn't even contacting the server, yet reports it as such. Awful! Chrome on the other hand response with 200 and as content size "(from cache)". It sometimes contacts the server to correctly display 304, noting the actual response's size (not that of the cached content). Pretty good! And we all know FFs behavior: Somewhat like Chrome, just without logging it at all. I kinda like Chromes way of handling things, though both 200 and 304 as a response are kinda unfitting... P.S. hard refresh of the webpage in ff won't clear the cache for content loaded dynamically via JS (such as an XML file or something).
Same here, Firefox 27beta. It looks deceptive when the network panel is filtered for JS for example, and it says there is only 1 request, and small number KB.
Another use case: 1. Clear the browser cache. 2. Open the Network panel. 3. Navigate to: http://mr-andersen.no/gfcycat-companion-test/issues/7.html (simple html page containing a video with webm source. Warning: autoplay=true / sound). 4. Force reload the page. After the page refresh the video/webm request does not appear in the list.
I've got this. Has to be fixed.
Assignee: nobody → fayearthur
I suppose a UI nicety would be to fade or make the cached requests appear less prominent than the uncached ones. I recall Firebug doing this.
Attached patch WIP: cached requests show up (obsolete) (deleted) — Splinter Review
This a WIP from a few weeks ago. I had to abandon it to work on some other things, but I'd like to come back around to it soon. It's basic. Cached requests show up in the network view, and there's no real difference in the UI except that there's a grey circle next to them, and there's no headers or timings data.
Assignee: fayearthur → nobody
Attached image What the WIP looks like (obsolete) (deleted) —
Here's what it looks like with the WIP applied, no special UI.
Blocks: 1037489
That's nice to see already. Would caching the response and request headers be on the roadmap for this?
(In reply to Heather Arthur [:harth] from comment #18) > Created attachment 8442420 [details] > What the WIP looks like > > Here's what it looks like with the WIP applied, no special UI. One more thing to ask please (it's not shown in the screenshot): cached entries to show "(from cache)" in the Size column - instead of "0 KB", and the font color for such entries (I mean for the "HTTP status code" and the "Size" columns) to be grey in order to be distinguished more easily.
Btw, anyone is welcome to take my patch and run with it (needs different UI and tests).
I found that ticking the "Disable Cache*" checkbox in the Developer Tools Options does not work for dynamically loaded JS that's already cached. The cached version is always returned. I'd expect that checkbox to make sure the cache is never used for anything. Same goes for Shift-Reload and Ctrl-F5 hard refreshes mentioned above - probably related.
Assignee: nobody → jlong
Attached patch 764958.patch (obsolete) (deleted) — Splinter Review
This is based off of harthur's original patch. It works so far. Now the question is: what should cached requests look like? The screenshot attached is what they look like right now. They just have the grey circle icon (the default one). Should we maybe also grey out the name so that its obvious they are cached requests?
Attachment #8442415 - Attachment is obsolete: true
I'd love to see a hollow (possibly yellow) circle.
Whiteboard: [parity-firebug] → [parity-firebug][devedition-40]
Note: The WIP patch also fixes bug 1151932. As for how the UI should look like for resources served from cache, we can: 1. Invent a new design language for them (e.g. hollow / yellow circle as Victor suggests). 2. Make them look similar to other cached resources: e.g. the ones with "If-Modified-Since" header and "304 Not-Modified" response (the icon is an orange triangle). 3. Show a fake "200 OK" status, but in discreet gray instead of bold black (like Chromium does). I personally like 2 or 3, with an additional "(from cache)" text somewhere, e.g. in the timeline, because there is not going to be timing information there anyway. Note: For cached resources, Chromium replaces size information with "(from cache)".
Blocks: 1151932
Attached image cached-concepts.png (obsolete) (deleted) —
I tried to illustrate different ways to represent cached resources. In general, I think it's good to grey out their status information, and show "(from cache)" somewhere.
Some text indicating that it is cached would be great. What happens if it's a cached redirect? You should see a 302 Found instead of the 200 OK? If that makes this patch too complicated though, please proceed without worry about redirects and file a followup for redirects. I can't wait to have this functionality in the console!
Not going to be working on this the next few days, but I will definitely pick it back up next week. Also probably depends on jsantell's refactoring in bug 1149634, so I may be blocked on that anyway
Assignee: jlong → nobody
Assignee: nobody → jlong
(In reply to Tanvi Vyas [:tanvi] from comment #27) > Some text indicating that it is cached would be great. > > What happens if it's a cached redirect? You should see a 302 Found instead > of the 200 OK? If that makes this patch too complicated though, please > proceed without worry about redirects and file a followup for redirects. I > can't wait to have this functionality in the console! I don't have a test case for that yet, but for cached responses it looks like we don't even have an HTTP code. We just have the cached request/response headers and other info, so you could look at the headers and see the `Location` field. I don't know if we should have a cached HTTP code or not, but at least showing the cached responses will be better than nothing.
Latest screenshot: http://jlongster.com/s/upload/net-cached2.png What do you all think?
Greying out all the text may have been too much, I like this simple version better: http://jlongster.com/s/upload/net-cached3.png
(In reply to James Long (:jlongster) from comment #31) > Greying out all the text may have been too much, I like this simple version > better: http://jlongster.com/s/upload/net-cached3.png Me too, looks good to me
Status: NEW → ASSIGNED
3rd variation: http://jlongster.com/s/upload/net-cached4.png I liked Victor's idea of showing hollow circles for the status icon, so here it is. I think this looks pretty good. Only issue is that there is an initial flash because the icon by default is a solid circle.
I prefer that last (3rd) iteration as well, because it differentiates itself from requests that are still pending, which use a solid greyed out circle. Anything to increase scan/skim speed is great.
(In reply to James Long (:jlongster) from comment #33) > 3rd variation: http://jlongster.com/s/upload/net-cached4.png > > I liked Victor's idea of showing hollow circles for the status icon, so here > it is. I think this looks pretty good. Only issue is that there is an > initial flash because the icon by default is a solid circle. Yes, this seems best to me of the options so far.
Attached patch 764958.patch (obsolete) (deleted) — Splinter Review
Here's a patch that's close to being final. I want to make one more pass tomorrow before I ask for review. I added a test and most things seem to be working. Here's the final design: http://jlongster.com/s/upload/net-cached.png I put "cached" in the transferred size because I think it makes a lot more sense there. Nothing is transferred, and it was too many hacks to put it in the waterfall view. There are a lot of assumptions that only the waterfall graph exists there. It didn't feel right anyway.
Attachment #8442420 - Attachment is obsolete: true
Attachment #8576157 - Attachment is obsolete: true
Attachment #8591697 - Attachment is obsolete: true
(In reply to James Long (:jlongster) from comment #36) > > Here's the final design: http://jlongster.com/s/upload/net-cached.png Looking good! I wasn't sure about the hollow circle, but it's easy to understand. Will you use this appearance for other types of cached resources, e.g. 304 Not-Modified? > I put "cached" in the transferred size because I think it makes a lot more > sense there. Nothing is transferred, and it was too many hacks to put it in > the waterfall view. There are a lot of assumptions that only the waterfall > graph exists there. It didn't feel right anyway. Good idea! Chrome also puts the "(from cache)" text in the Size column, that way we can still put a blip in the timeline when the resource was loaded.
(In reply to Jan Keromnes [:janx] from comment #37) > (In reply to James Long (:jlongster) from comment #36) > > > > Here's the final design: http://jlongster.com/s/upload/net-cached.png > > Looking good! I wasn't sure about the hollow circle, but it's easy to > understand. > > Will you use this appearance for other types of cached resources, e.g. 304 > Not-Modified? I don't think you can ever get a cached 304. Browsers don't cache the response from a validator, they just cache the actual resource and will re-use it without checking the server. If they do have the use a validator and check the server, they will get back either the resource or a 304. What we are showing here are resources straight from the browser cache which don't involve the 304 validator requests. I could be wrong, but I think only 200 GETs and 301/302 redirects are locally cached. I can't think of any other requests that could be cached. I actually think we do have status code information, btw. I just need to wire it up. So we can show the right status code, whether its 200, 301, or 302. I'm thinking they will all use the hollow circle. > > > I put "cached" in the transferred size because I think it makes a lot more > > sense there. Nothing is transferred, and it was too many hacks to put it in > > the waterfall view. There are a lot of assumptions that only the waterfall > > graph exists there. It didn't feel right anyway. > > Good idea! Chrome also puts the "(from cache)" text in the Size column, that > way we can still put a blip in the timeline when the resource was loaded. Cool. I think we might be able to get some timing info in the future and show it on the graph. Right now I don't know why, but if several cached resources are loaded, they obviously take some time because the next non-cached resource starts later than the last non-cached resource (there is a gap in the timeline). Chrome does show a blip on their waterfall but it looks like there are still obvious gaps.
Sweet, I was able to clean this up a bit and make things more consistent. We now have status codes as well, but if it's cached it will always use the hollow icon. Here's the final final design: http://jlongster.com/s/upload/4D48CD37.png Note that there's a 301 redirect that happens twice, but the second time it is cached.
This bug was originally filed for webconsole's Net panel and changed somewhere along the way. These changes are great for netmonitor; is there an easy way to extend it to webconsole as well?
(In reply to Tanvi Vyas [:tanvi] from comment #40) > This bug was originally filed for webconsole's Net panel and changed > somewhere along the way. These changes are great for netmonitor; is there > an easy way to extend it to webconsole as well? The cached resources will show up in the console as well now, although there isn't anything indicating they are cached.
(In reply to James Long (:jlongster) from comment #41) > The cached resources will show up in the console as well now, although there > isn't anything indicating they are cached. Okay, great. Thanks James!
Attached patch 764958.patch (obsolete) (deleted) — Splinter Review
r? jsantell because Victor's on PTO, and wanted to let you know about this patch because we'll have to rebase your modularization on top of this (I can help with that)
Attachment #8596177 - Attachment is obsolete: true
Attachment #8596810 - Flags: review?(jsantell)
Attached patch 764958.patch (obsolete) (deleted) — Splinter Review
fixed a style editor test
Attachment #8596810 - Attachment is obsolete: true
Attachment #8596810 - Flags: review?(jsantell)
Attachment #8597295 - Flags: review?(jsantell)
(In reply to James Long (:jlongster) from comment #38) > (In reply to Jan Keromnes [:janx] from comment #37) > > Will you use this appearance for other types of cached resources, e.g. 304 > > Not-Modified? > > I don't think you can ever get a cached 304. Browsers don't cache the > response from a validator, they just cache the actual resource and will > re-use it without checking the server. Sorry for being unclear, that's what I actually meant: it's cache at a different level. Yes the browser does an actual (uncached) request, but ends up loading the resource from cache if the server replies "304 Not Modified", so I argue that this particular resource is cached as well. For example, when I load MDN, I can see an item called "237572123.js" with 304 response, Transferred = 56.73 KB, and Size = 159.78 KB. I can hardly believe that the "304 Not Modified" empty response actually transferred 56.73 KB, rather I guess that's what was initially transferred the first time, when the resource was originally saved into the cache. I think in this case it would make sense to say "cached" instead of showing a confusing "Transferred" size, and maybe also show the grey hollow circle to be consistent. Or at least, "Transferred" should show the actual (small) transferred size of the (empty) 304 response.
Comment on attachment 8597295 [details] [diff] [review] 764958.patch Review of attachment 8597295 [details] [diff] [review]: ----------------------------------------------------------------- Good stuff -- I think this should be pretty straight forward to merge into any changes in the refactoring, but that being said, double check that the tests are bullet proof for that reason :) r+ on my end for the implementation, but deferring to Jan for his comment #47 ::: toolkit/devtools/webconsole/network-monitor.js @@ +583,5 @@ > httpVersionMin.value; > > this.openResponses[response.id] = response; > + > + if(aTopic === "http-on-examine-cached-response") { nit: some comments here on what this is doing, seems like adding a "fake" request from the cache?
Attachment #8597295 - Flags: review?(jsantell) → review+
(In reply to Jan Keromnes [:janx] from comment #47) > I think in this case it would make sense to say "cached" instead of showing > a confusing "Transferred" size, and maybe also show the grey hollow circle > to be consistent. Or at least, "Transferred" should show the actual (small) > transferred size of the (empty) 304 response. I see, you're right, we probably could be clearer there. Although I don't think it should look the same as the requests in my patch. These are clearly shown as coming from the *browser* cache, which is quite different than using 304 validators. But I think you're right that the transfer size is wrong and we can make it clear that it is another type of cache. I'd like to do that as a follow-up though, especially since that's a very different code path than what my patch touches.
Comment on attachment 8597295 [details] [diff] [review] 764958.patch Review of attachment 8597295 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/devtools/webconsole/network-monitor.js @@ +583,5 @@ > httpVersionMin.value; > > this.openResponses[response.id] = response; > + > + if(aTopic === "http-on-examine-cached-response") { yep, we don't get any request events, and only some of the response events. So this fires off events to simulate a request and fills in extra response data that the client wouldn't normally get. The initial network event that the frontend gets has a "fromCache" property to indicate it came from the cache, so it can appropriately show it as so.
Attached patch 764958.patch (deleted) — Splinter Review
added some comments about the fake events
Attachment #8597295 - Attachment is obsolete: true
Keywords: checkin-needed
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [parity-firebug][devedition-40] → [parity-firebug][devedition-40][fixed-in-fx-team]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [parity-firebug][devedition-40][fixed-in-fx-team] → [parity-firebug][devedition-40]
Target Milestone: --- → Firefox 40
Depends on: 1163024
Whiteboard: [parity-firebug][devedition-40] → [parity-firebug][polish-backlog]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: