Closed Bug 1293355 Opened 3 years ago Closed 3 years ago

fromCache is absent from onResponseStarted callback

Categories

(WebExtensions :: Request Handling, defect, P2)

51 Branch
defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: jon.jwilkes, Assigned: Tomislav)

Details

(Whiteboard: triaged)

Attachments

(2 files)

Attached file fromcache.zip
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160630070928

Steps to reproduce:

1. Load the attached extension.
2. Browse the web.



Actual results:

Notice that fromCache is always undefined.


Expected results:

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/WebRequest/onResponseStarted says fromCache should be set.
Version: 47 Branch → 51 Branch
Component: Untriaged → WebExtensions
Product: Firefox → Toolkit
Component: WebExtensions → WebExtensions: Request Handling
Priority: -- → P2
Whiteboard: triaged
From my investigation, it seems that nsIHttpChannel doesn't have any property or method that would indicate if the response came from the cache (nor can it be QIed to something like nsICachedChannel to check).

The way to distinguish cached vs uncached responses is in WebRequest.jsm when we first observe them:

http://searchfox.org/mozilla-central/source/toolkit/modules/addons/WebRequest.jsm#426

Or did I miss something?
Flags: needinfo?(kmaglione+bmo)
I think we just need to check whether we're examining the response from an http-on-examine-response observer vs. an http-on-examine-cached-response or http-on-examine-merged-response observer.
Flags: needinfo?(kmaglione+bmo)
Status: UNCONFIRMED → NEW
Ever confirmed: true
> I think we just need to check whether we're examining the response from an
> http-on-examine-response observer vs. an http-on-examine-cached-response or

I thought I was saying just that.. ;)
Assignee: nobody → tomica
Status: NEW → ASSIGNED
Comment on attachment 8793469 [details]
bug 1293355 - add fromCache to onResponseStarted details,

https://reviewboard.mozilla.org/r/80198/#review80124

Thanks!

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest.html:429
(Diff revision 1)
> +  function checkFromCache(kind, details) {
> +    if (checkCompleted) {
> +      // If we have already completed a GET request for this url,
> +      // and it was found, we expect for the response to come fromCache.
> +      const completed = (kind in completedUrls) && completedUrls[kind].has(details.url);
> +      const expected = completed && (details.method === "GET") && (details.statusCode != 404);

No parens, please.
Attachment #8793469 - Flags: review?(kmaglione+bmo) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/aeedc4a1f893
add fromCache to onResponseStarted details, r=kmag
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aeedc4a1f893
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.