475156 - RFC 2616 1.3 Response Cacheability issue with XHR - Ajax connections cache refresh leading to huge bandwidth and server hits overhead

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[danny]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5

When using a XMLHttpRequest() in JavaScript to retrieve a file from the origin server with a GET method the file gets cached correctly and is correctly returned from cache each next request. If however the file is requested with an HTTP 1/1 (RFC 2616) validator (eg: If-Match) targeting a ETag of a newer version of the file the file is retrieved correctly from the origin server BUT doesn't get cached locally (=does not overwrite the previously OLD cached file). Each next request, even after restarting the browser, the server is hit and the local DISK cache is not used. Verified with the cache viewer as well as with log lines tracing at the origin server. I compared this behavior with Apple Safari, which does correctly update the local DISK cache. I noticed the problem during bandwidth monitoring. After a file gets updated Safari never hits the server again for that file until the file is updated again and receives a new ETag. Firefox, as of the first time the file gets updated on the server, starts consuming a tremendously larger amount of bandwidth, generates a lot of extra round trips to the server and as a consequence performs slower, because it retrieves the file EACH time from the origin server as a result of not updating it's local DISK cache. Tested with Windows XP/Linux Fedora for Firefox and Windwows XP for Safari.

Reproducible: Always

Steps to Reproduce:
1. Clear the Cache (Tools->Clear Private Data)
2. From an HTML or SVG page start a JavaScript (let's call it 'loader') that is ALWAYS retrieved from the origin server (never cached). I did it by calling this 'loader' script as an external script from an .svg page. 
3. Let the loader request a file, I will call it 'MyFile', with an XMLHttpRequest() assync GET method. Assume MyFile has an ETag 'abc_1'. Use NO validator (eg: If-Match) otherwise this first request doesn't get cached either, which is probably due to the same problem as the one described!!!
4. On arrival of MyFile compare the loader's embedded (build in) ETag value with the one of MyFile. The tag matches because MyFile was freshly served from the origin server as expected. Notice how MyFile becomes DISK cache (cache viewer). MyFile has a LONG (1 Year) max-age setting.
5. Request the same page again for a couple of time to see it being served correctly from local DISK cache.
6. Now, at server side, change the ETag of MyFile from abc_1 into abc_2 and update it in the loader (build-in) too. Hit your main page (will reload 'loader' from origin server) and notice how, when MyFile is requested by the XMLHttpRequest(), it is correctly served from local DISK cache but with an ETag abc_1. Because this tag does not match with the one build into the loader request the MyFile with a validator (If-Match:"abc_2").
7. Notice how MyFile is correctly retrieved from the origin server BUT notice also that the local DISK cache isn't updated!
8. Observe now how, no matter how many times you request the MyFile it keeps coming from the origin server EACH time because the one in cache doesn't have the matching ETag 'abc_2'.
9. Stop and restart the browser and notice that the problem of step 8 remains. 
Actual Results:  
MyFile with ETag 'abc_1' is delivered from the origin server and is cached locally as DISK cache. When a newer version becomes available with ETag 'abc_2' that version is correctly retrieved from the origin server BUT the local DISK cache doesn't get updated. This behavior results in files that have been updated once and are retrieved with HTTP validators (If-Match, If-None-Match, If-Last-Modified, etc) are serveed from the origin server EACH time they are requested after there validation target item (ETag, Last-Modified,ect) has been updated once. 

Expected Results:  
When a newer version of MyFile comes in, according REF 2616 13.4 "Response Cacheability" it may be cached unless other cache control elements forbid it. This is not the case in the above described problem. Optimal behavior can be checked by doing the above test with the LATETEST version of Safari, and probably in coming updates from Google Chrome (which in current release only solved this partially but already better).

This described problem has a huge impact on Firefox performance, bandwidth consumption and generates a lot of server round trip and CPU overhead. To put this in numbers. After a first update FF consumes 100% more bandwidth for EACH request of the file compared to what it should be and needs an additional round trip and server look-up per request. Extrapolate this to XHR/Ajax applications that get many small files with XMLHttpRequest() requests and it becomes obvious what's happens. In the best case some public caches can accommodate some but end-users will still get the bandwidth and performance penalty.
I think cache issues are MAJOR features since they have this impact in all area's (bandwidth, speed, CPU usage, round-trips) and that with the XHR/Ajax applications becoming more and more popular this problem should be fixed ASAP because it's impact is exponential.

Comment 1

[danny]

The same test as described above has been done by starting from an HTML main page in stead of an SVG main page. This has no effect and results in identical behavior.

Comment 2

[danny]

Tested SAME concept but in stead of using If-Match and ETag I used the following:
If-None-Match in combination with Etag and If-Modified-Since and If-Unmodified-Since in combination with Last-Modified. They all have exactly the same behavior and the problem persists (Tested only on Windows this time).

Comment 3

[danny]

In an attempt to create a workaround for the above mentioned problem I discovered that in general the HTTP Cache-Control settings are not always obeyed either. must-revalidate and no-cache when set to the request header with the JavaScript XHR instruction are completely ignored and keep returning a cached file (if one exists) rather then retrieving it from the origin server (as is expected according chap. 14 of RFC 2616) when a client set these headers. 
Maybe this may indicate that the problem described in the body of this report may be part of a more elaborated problem. (Tested only on Windows not on Linux this time).

Comment 4

[Boris Zbarsky [:bzbarsky]]

dcamp, want to look into this?

Reporter, a testcase (a page showing the problem) would be helpful here.

Note that the initial Firefox 3.0 release had a known cache bug where we cached things we shouldn't have cached.  That problem is resolved on trunk and in 3.0.x development builds, but I'm not sure whether that fix has shipped yet.

Comment 5

[danny]

<<<Reporter (Danny): I will prepare the HTTP request and response headers (copy from FireBug) and the relevant part of the JavaScript code. 

The test pages wouldn't help they are generated by the test server and just contain : alert("Version 1 - Tag abc "); and the other one contains alert("Version 2 - Tag xyz"); They only differ  ETag:"abc"  and ETag:"xyz"   
in the HTTP header. To simulate a page update xyz-taged page replaces by the abc-taged page. A log in the server monitors if the file is REALLY coming from the server. That's it.

I'll post the headers here. I will concentrate on the main bug because as you may have seen in comments 2,3 other, not all, cache related HTTP headers statements have some problems too. Therefor, Boris, your comment that they may have been corrected could be right.

If you tell me where I can get that version I can test it on Win or Lin (not Mac) if that helps in order to avoid needless efforts. I tested on 3.0.5 !

Comment 6

[Boris Zbarsky [:bzbarsky]]

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/3.0.6-candidates/build1/firefox-3.0.6.en-US.win32.installer.exe or 
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/3.0.6-candidates/build1/firefox-3.0.6.en-US.linux-i686.tar.bz2

are the builds to test.  Or other builds in that same directory if you'd prefer a different localization.

Comment 7

[danny]

I will test the build tomorrow. 

Here is the requested additional info with detailed HTTP headers in a step by step evolution, incl. server side trace info, User Agent Local Cache evolution, etc. Done with Firefox 3.0.5 on Fedora 9 Linux and Glashfish server. Same problems with FF 3.0.5 on Windows XP while server remained on Linux. Compared to Safari to be sure it wasn't an error in my code or in my HTTP settings.

Each test started by a main SVG page that only starts an external JavaScript file that I call the loader in the text below. It is that loader that executes the XHR calls. 
 

1) Original request 

Preliminary      Cleared User Agents Local Disk Cache
                 Started new browser session

XHR Requested    http://localhost:12693/TestZone/TestZone?File=1.0.2.1.0.0
HTTP Method      GET
XHR set headers  none for this original call, browser standard (see below)
   
FireBug          Request and Response Headers below

Request Headers
Host             localhost:12693
User-Agent       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.5) Gecko/2008121622 Fedora/3.0.5-1.fc9 Firefox/3.0.5
Accept           text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language	 en-us,en;q=0.5
Accept-Encoding	 gzip,deflate
Accept-Charset	 ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive	 300
Connection	 keep-alive
Referer	         http://localhost:12693/TestZone/TestZone?File=3.1.0.1.0.0
Cookie           UAConfig=1|1|01|1065484223|0189|0300|0|1|EN

Response Header
Etag             "abc"        <--------------- Notice how 'ETag' became 'Etag', could be FireBug
MetaRid          1.0.2.1.0.0
Cache-Control    public, no-transform, max-age=31536000
Last-Modified    Tue, 03 Feb 2009 15:50:13 GMT
Content-Type     application/octed-stream;charset=UTF-8
Content-Length   26
Date             Tue, 03 Feb 2009 15:53:47 GMT

RESULT           Correct reply. Page contained JavaScript : alert("Version 1 - abc"); followed by a new-line
CACHE VIEWER     File is in local Disk Cache (CORRECT BEHAVIOUR)
SERVER TRACE     File was retrieved from server (EXPECTED BEHAVIOUR)

2) Same request again

Preliminary      Started new browser session to avoid local memory cache

XHR Requested    http://localhost:12693/TestZone/TestZone?File=1.0.2.1.0.0
HTTP Method      GET
XHR set headers  none, browser's default
 
FireBug          No entry for this page request in FireBug window


RESULT           Correct reply. Page still contained JavaScript : alert("Version 1 - abc");
CACHE VIEWER     File in local Disk Cache has not changed (CORRECT BEHAVIOUR)
SERVER TRACE     File was NOT retrieved from server (CORRECT BEHAVIOUR)

3) Update ETag of page on server and in loader from "abc" to "xyz" and request again

Preliminary      Started new browser session

XHR Requested    http://localhost:12693/TestZone/TestZone?File=1.0.2.1.0.0
HTTP Method      GET
XHR set headers  none, browser standard (see below)
   
FireBug          Request header only

Request Headers
Host             localhost:12693
User-Agent       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.5) Gecko/2008121622 Fedora/3.0.5-1.fc9 Firefox/3.0.5
Accept           text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language	 en-us,en;q=0.5
Accept-Encoding	 gzip,deflate
Accept-Charset	 ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive	 300
Connection	 keep-alive
Referer	         http://localhost:12693/TestZone/TestZone?File=3.1.0.1.0.0
Cookie           UAConfig=1|1|01|1065484223|0189|0300|0|1|EN

RESULT           Old page from local cache with ETag "abc". ETag does NOT match (loader now expects ETag "xyz")
CACHE VIEWER     File in local Disk Cache has not changed (STILL CORRECT BEHAVIOUR)
SERVER TRACE     File was NOT retrieved from server (STILL CORRECT BEHAVIOUR)
ACTION           XHR Requests the page again but with HTTP header:   If-Match:"xyz"
                 NOTE: This was implemented this way because requesting the page oringinally with "If-Match" results in 
                       the page NEVER being cached at all, possibly due to the same problem as why in a minute it isn't 
                       going to be updated in local cache.

FireBug          Request and Receive header

Request Headers
Host             localhost:12693
User-Agent       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.5) Gecko/2008121622 Fedora/3.0.5-1.fc9 Firefox/3.0.5
Accept           text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language	 en-us,en;q=0.5
Accept-Encoding	 gzip,deflate
Accept-Charset	 ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive	 300
Connection	 keep-alive
If-Match         "xyz"    <---------------------------------------------------- set with XHR setRequestHeader() method.
Referer	         http://localhost:12693/TestZone/TestZone?File=3.1.0.1.0.0
Cookie           UAConfig=1|1|01|1065484223|0189|0300|0|1|EN

Response Header
Etag             "xyz"    <---------------------------------------------------- as expected.
MetaRid          1.0.2.1.0.0
Cache-Control    public, no-transform, max-age=31536000
Last-Modified    Tue, 03 Feb 2009 16:21:07 GMT
Content-Type     application/octed-stream;charset=UTF-8
Content-Length   26
Date             Tue, 03 Feb 2009 16:26:38 GMT

RESULT           Correct reply. Page contained  JavaScript : alert("Version 2 - xyz"); followed by a new-line
CACHE VIEWER     File is NOT in local Disk Cache, old one with ETag "abc" is still there  (***** INCORRECT BEHAVIOUR ***** )
SERVER TRACE     File (with "xyz" tag) was retrieved from server (CORRECT BEHAVIOUR for first request after page update).


4) Same request again

Preliminary      Started new browser session to avoid local memory cache

XHR Requested    http://localhost:12693/TestZone/TestZone?File=1.0.2.1.0.0
HTTP Method      GET
XHR set headers  none, browser's default
 
RESULT           Exactly the SAME as test (3). The file has been retrieved from server and that shouldn't happen. The file that was received in (3) above qualified in all aspects and HTTP settings to be cachable. ***** INCORRECT BEHAVIOUR *****

5) Same request again with F5

Preliminary      none

XHR Requested    http://localhost:12693/TestZone/TestZone?File=1.0.2.1.0.0
HTTP Method      GET
XHR set headers  none, browser's default
 
RESULT           Exactly the SAME as test (3) and same result comment as in (4)

6) Same request again (forced without "If-Match")

Preliminary      Started new browser session to avoid local memory cache

XHR Requested    http://localhost:12693/TestZone/TestZone?File=1.0.2.1.0.0
HTTP Method      GET
XHR set headers  none, browser's default
 
FireBug          No entry for this page request in FireBug window


RESULT           ***** Incorrect reply *****. Old page again contained JavaScript : alert("Version 1 - abc");
CACHE VIEWER     File in local Disk Cache still old abc-taged version, has not changed (INCORRECT BEHAVIOUR)
SERVER TRACE     File was NOT retrieved from server (IRRELEVANT, SHOULDN'T BE ANYWAY AT THIS POINT)


CONCLUSION
Files requested with "If-Match" (and as I stated before with conditional HTTP headers in general) are not cached on 
arrival, even if they qualify for caching. As a consequence when a page is updated once and is requested with a 
conditional header entry it is retrieved from the origin server each time. That's why the result looks ok but browser speed, bandwidth, cpu and user experience suffer from it.

Comment 8

[Dave Camp (:dcamp)]

Caching of conditional requests was explicitly disabled in bug 331825.  The reasoning was this bit of the XMLHttpRequest spec:

For 304 Not Modified responses that are a result of a user agent generated conditional request the user agent must act as if the server gave a 200 OK  response with the appropriate content. The user agent must allow setRequestHeader() to override automatic cache validation by setting request headers (e.g., If-None-Match, If-Modified-Since), in which case 304 Not Modified responses must be passed through. [RFC2616]

Necko always returns the 200 response if it can read from its own cache, and the decision at the time was to just skip using the cache entirely.

It should be possible to expose the 304 response to XMLHttpRequest somehow, and let it decide what to show the user.  If we did that, we could turn caching back on for these requests.

Comment 9

[Dave Camp (:dcamp)]

(In reply to comment #8)

> It should be possible to expose the 304 response to XMLHttpRequest somehow, and
> let it decide what to show the user.  If we did that, we could turn caching
> back on for these requests.

And in that case we'd also need to avoid sending the cached body data too (LOAD_ONLY_IF_MODIFIED should probably work for that).

Comment 10

[danny]

Tests done on Release Candidate 3.0.6 : Exactly same behavior.
In the light of Dave's comment #8 this is within expectation.

Comment 11

[danny]

(In reply to comment #9)
> (In reply to comment #8)
> 
> > It should be possible to expose the 304 response to XMLHttpRequest somehow, and
> > let it decide what to show the user.  If we did that, we could turn caching
> > back on for these requests.
> 
> And in that case we'd also need to avoid sending the cached body data too
> (LOAD_ONLY_IF_MODIFIED should probably work for that).

Dave, about your comment # 8 and # 9 (304 vs 200)

I have bypassed that problem in the JavaScript code by doing a double request for old files and therefor could propose a simplified solution for this problem (see end of comment).

So technically the solution could be achieved more simply and stay within full standards not needing any "possibly FF specific" way to pass on a 304 to JavaScript in stead of the 200 OK, neither changing anything in the interaction with JavaScript XHR. 

This is what I tried starting with empty cache :

Loader Requests page < Receives file with ETag "abc" : 200 OK
Loader (JavaScript) compares tags (getResponseHeader()) : tags match : done, use this file.

Loader Requests same page again < Receives file with ETag "abc" FROM UA's local Disk Cache : 304 BUT xhr.status=200 OK
Loader compares tags (getResponseHeader()) : tags STILL match (Loader had not been changed) : done, use this file

Change ETag in server page from "abc" to "xyz" and update loader. NOTE Loader is NEVER cached thanks to Cache-Control "no-cache"

Loader Requests page again < Receives OLD file with tag "abc" FROM UA's local Disk Cache : 304 BUT xhr.status=200 OK
Loader compares tags (getResponseHeader()) : tags DON'T match (Loader was updated to look for "xyz") 
Request Again with  If-Match:"yxz" < Receives NEW file with tag "xyz" FROM ORIGIN SERVER : 200 OK
Loader compares tags (getResponseHeader()) : tags match again now ("xyz") : done, use this file

And here is the need for the small fix because as of now we will ENDLESSLY fall into the file being served from the origin server as described in the 4 lines above while it should do this :

Loader Requests page again < Receives NEW file IMMEDIATELY with tag "xyz" FROM UA's local Disk Cache : 304 BUT xhr.status=200 OK
Loader compares tags (getResponseHeader()) : tags match (Loader was not updated again and still looks for "xyz") : Done, use this file 

So put simply : If a request for a page, even when requested with a conditional "If-..." HTTP header, matches a file in the UA's Local Disk cache
and that page cannot be used because of the 'If-..." or other (Expired, max-age) condition and as a consequence a new page is requested from the 
origin server (up to here that is exactly how things work in FF today) then if a response from the origin server (or intermediate cache) has a 
status of 200 OK (NOT 304) the cached page gets overwritten by the new page if the rules laid out in RFC 2616 13.4 Response Cacheability are met.

This solutions does NOT change anything to the interaction with JavaScript, it leaves in place decisions made earlier about cache and it also 
does NOT need any changes in what BODY is being send to JavaScript because that is already OK and working fine today. The fact that a cache was overwritten in request 'n' is transparent to request 'n+1'.

I hope this may help.

Comment 12

[Dave Camp (:dcamp)]

(In reply to comment #11)
> (In reply to comment #9)
> > (In reply to comment #8)
> > 
> > > It should be possible to expose the 304 response to XMLHttpRequest somehow, and
> > > let it decide what to show the user.  If we did that, we could turn caching
> > > back on for these requests.
> > 
> > And in that case we'd also need to avoid sending the cached body data too
> > (LOAD_ONLY_IF_MODIFIED should probably work for that).
> 
> So technically the solution could be achieved more simply and stay within full
> standards not needing any "possibly FF specific" way to pass on a 304 to
> JavaScript in stead of the 200 OK, neither changing anything in the interaction
> with JavaScript XHR. 

Sorry, I was unclear;  I wasn't talking about changing the API exposed to javascript.  I was proposing tweaking the interaction between nsHttpChannel and nsXMLHttpRequest so that the same javascript API would support caching of new items (as you described).

Comment 13

[danny]

ok, Dave, could have been me interpreting text wrong too.

How are we going to get this fixed? How does this bug get confirmed?
I don't know if you guys have to sit in a committee or so to discuss priorities but if it's the case then here are some argumentations why this bug is a general purpose one and possibly with STRATEGIC advantages for Firefox given the raising popularity of XHR and Ajax technology in the field.

Server Owners/Renters
- less hits=less CPU=less congestion=less cost of infrastructure (UA per Server)
- less bandwidth=less bandwidth cost (external/internal depending on charge)

Data Centers
- less bandwidth=less bottleneck on pipes to Internet=less pipe investment
- less hits=less traffic=smaller queues=less metering CPU=better performance

End-Users (UA side)
- less hits=higher performance=better user experience
- local cache=higher performance, less bandwidth
- less bandwidth=less provider cost (no extra bandwidth or smaller subscription)

Mozilla Firefox
- better performance, better standard compliance=better choice of browser
- Except for Safari no other browser have this fixed yet 
- Chrome and IE8beta only have it fixed wrongly and must correct it again.
  They are no where FF is already and do now also check the 'If-Match' but they have the caching wrong.
- Opera and Konqueror added the conditionals but already have a cache problem with the original file (without 'If-Match') in XHR/Ajax and therefor retrieve the file each time from the start.

So this is an opportunity, probably with a relatively small effort, to provide the market with a good reason to use Firefox at the benefit off all involved players, certainly given the economic side effects of this fix in the current economic situation. IT cost is under stress.

I am prepared to contribute and can perform the tests again on fixed versions and report the results (Windows/Linux).

Comment 14

[danny]

Request for Confirmation. Can this bug be confirmed ?

Comment 15

[danny]

Dave, is there any decision yet about turning caching back on for conditional HTTP requests such as 'if-Match', etc ?

Comment 16

[Walter Adams]

Dave, 

I too have found this problem in caching while testing one of my applications retrieving files through a javascript XMLHttpRequest(

Before registering the bug, I saw the issue was already posted by another developer. I concur this is indeed an easily reproducible issue and should definitely be confirmed as a bug.

I'm currently on Firefox v3.0.6 (on Windows)

Dave: please confirm the status of this issue so it gets the proper attention to get it solved.

Thx,
Walter

Comment 17

[Honza Bambas (:mayhemer)]

-> me

Comment 18

[Johnny Stenback (:jst)]

Let's see if we can do this for 1.9.2.

Comment 19

[Johnny Stenback (:jst)]

On second thought, I don't think we should change this any more for 3.6. This is 3.7 material for sure thought!

Comment 20

[danny]

In case it is moved to 1.9.2 you may as well consider fixing all the other HTTP problems for the XHR connections. Some of them are also pointed to in the body of the report. Mainly cache related stuff that logically should all be handled at the same place where the core problem of this bug will be fixed.

Of course not fixing it in 1.9.1 is a nice present to competing browsers now that the race for speed and bandwidth reduction started.

Comment 21

[danny]

In case you would want to fix the XHR HTTP cache more completely here is a page that may be a help : 
http://www.mnot.net/javascript/xmlhttprequest/cache.html

Comment 22

[Honza Bambas (:mayhemer)]

Attachment: v1

First version, work in progress. Now for conditional request we use cache as in usual cases. When If-Match header is defined, we check the ETag we have in cache (if any) with the one provided by the consumer. If it matches, we return the cached entity (there is no revalidation with the origin server). If it doesn't match we force revalidation.

To preserve previous behavior with custom conditional request and 304 response I check for any conditional header, if found, I prevent their override by the cache code and prevent handling of ProcessNotModified that loads cached response.

Going to push this to tryserver.

Comment 23

[Honza Bambas (:mayhemer)]

Comment on attachment 403274 [details] [diff] [review]
v1

The patch just passed a try server build.

Comment 24

[Boris Zbarsky [:bzbarsky]]

I don't follow the CheckCache() change.  If !mCustomConditionalReuqest (mis-spelled, by the way), why do you need to clear anything?

Comment 25

[Honza Bambas (:mayhemer)]

(In reply to comment #24)
> I don't follow the CheckCache() change.  If !mCustomConditionalReuqest
> (mis-spelled, by the way), why do you need to clear anything?

Yes, it is a left over. I forgot to remove it. Thanks for the catch.

Comment 26

[Honza Bambas (:mayhemer)]

Attachment: v2

I removed the code from CheckCache(). Again passed try server round.

Comment 27

[Honza Bambas (:mayhemer)]

Attachment: v2 [Checkin m-c comment 29]

And this is the patch with fixed typo ;) Sorry for bug spam.

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 405432 [details] [diff] [review]
v2 [Checkin m-c comment 29]

>+++ b/netwerk/protocol/http/src/nsHttpChannel.cpp
> nsHttpChannel::ProcessNotModified()
>+    if (mCustomConditionalRequest)
>+        return NS_ERROR_FAILURE;

Please log that.

>+++ b/netwerk/protocol/http/src/nsHttpChannel.h
>+    // True if consumer added it's own If-None-Match or If-Modified-Since

"its own"

r=bzbarsky with those nits fixed.

Comment 29

[Honza Bambas (:mayhemer)]

Comment on attachment 405432 [details] [diff] [review]
v2 [Checkin m-c comment 29]

http://hg.mozilla.org/mozilla-central/rev/f0ba915153e4

Comment 30

[Honza Bambas (:mayhemer)]

And forgotton nit fixes here:
http://hg.mozilla.org/mozilla-central/rev/e8d729640a71

Comment 31

[John J. Barton]

Bug 522463 may interest the followers of this bug. It's about correctness rather than performance however (stale data). I verified that the test case there still fails on the 3.7 nightly.