1027579 - Add docShell flag DISABLE_CACHE to disable all caching for the lifetime of a page, including AJAX (XHR), web fonts, images etc.

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Jon Rimmer]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:32.0) Gecko/20100101 Firefox/32.0 (Beta/Release)
Build ID: 20140616004003

Steps to reproduce:

1. Open the DevTools
2. Select "Disable Cache"
3. Browse a site that makes requests via XHR


Actual results:

XHR requests are served from the cache.


Expected results:

XHR requests shouldn't be served from the cache.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

"Disable cache" reloads the page, are you sure you are not observing requests that were made before that?

Comment 2

[Jon Rimmer]

Yes, I'm certain. It's actually that I'm *not* observing requests. When the cache is disabled, everything that triggers and XHR request should show up in the network tab, but I'm seeing many, many requests not appear and instead the data returned to the JS code is the stale, cached version.

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

I can reproduce this on this test page:
http://www.httpwatch.com/demos/ajax_caching/

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

We are using this to disable the cache:
this.docShell.defaultLoadFlags = Ci.nsIRequest.LOAD_BYPASS_CACHE | Ci.nsIRequest.INHIBIT_CACHING;

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: ajax-cache-1027579.patch

This patch should fix it.

Comment 6

[Jon Rimmer]

Sweet! FYI a similar bug seems to affect fonts: https://bugzilla.mozilla.org/show_bug.cgi?id=1027124

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8445164 [details] [diff] [review]
ajax-cache-1027579.patch

Why doesn't this already work?  nsLoadGroup::MergeLoadFlags should propagate at least the LOAD_BYPASS_CACHE flag, though it doesn't propagate INHIBIT_CACHING.

Is this because the XHR is happening after the page has completed loading so !mDefaultLoadRequest in the loadgroup?  If so, that's somewhat on purpose: when doing a cache-bypassing reload we don't want to disable caching completely for the lifetime of the web page...

If we're in that situation, that suggests that if we're going to have a "really, do disable the cache for the lifetime of the web page" mode then we should tell the loadgroup we're in that mode and have our existing machinery for propagating load flags to all requests deal instead of whack-a-moling every single place we start a load.

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Boris Zbarsky [:bz] from comment #7)
> Comment on attachment 8445164 [details] [diff] [review]
> ajax-cache-1027579.patch
> 
> Why doesn't this already work?  nsLoadGroup::MergeLoadFlags should propagate
> at least the LOAD_BYPASS_CACHE flag, though it doesn't propagate
> INHIBIT_CACHING.
> 
> Is this because the XHR is happening after the page has completed loading so
> !mDefaultLoadRequest in the loadgroup?  If so, that's somewhat on purpose:
> when doing a cache-bypassing reload we don't want to disable caching
> completely for the lifetime of the web page...
> 
> If we're in that situation, that suggests that if we're going to have a
> "really, do disable the cache for the lifetime of the web page" mode then we
> should tell the loadgroup we're in that mode and have our existing machinery
> for propagating load flags to all requests deal instead of whack-a-moling
> every single place we start a load.

I think something like a DISABLE_CACHE flag that lasts for the lifetime of the docShell makes the most sense. Of course, this should also work for everything else that we cache (images, fonts etc.)

I am going to bow out of this one and leave it to somebody more familiar with our caching system.

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Repurposing this bug to add a docShell flag DISABLE_CACHE to disable all caching including AJAX (XHR), web fonts, images etc. for the lifetime of the docShell as per comment 7.

Comment 11

[Kumar Harsh]

This issue is still present in Firefox 33.1 AND 35.0a2

I was had set the "Disable Cache" as true, and lodaed my page. My page had a script (bootstrap.js), which used requirejs to dynamically load another script (dyn.js) when a condition is satisfied. Everytime I changed the base `bootstrap.js` script, it loaded correctly, but the `dyn.js` was ALWAYS served from cache. Even after Disabling the cache. I was tearing my hairs apart on this... the only way to get it working was to clear cache manually (Ctrl+Shift+Del) on every reload. :(

Finally, I figured that the "Cache-Control: public, max-age=86400000" header was set, which may be causing this XHR request to be cached. Set the max-age to 0 to get it working, but I realised this is a big bug in Firefox. Especially, if the Firefox version being used is called "Developer Edition"...

Comment 12

[kud]

A work around is to use private tabs. https://addons.mozilla.org/en-US/firefox/addon/private-tab/

:D

But it's definitely a feature I'd love to see. So much cache bug with fonts and swf.

Comment 13

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8445164 [details] [diff] [review]
ajax-cache-1027579.patch

Filter on 1ff0543e-b501-4893-a72b-e4773c01e655

Comment 14

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

@Honza: I have heard a lot of criticism that our devtool's disable cache implementation is broken, which moves people away from our tools.

We are using this to disable the cache:
this.docShell.defaultLoadFlags = Ci.nsIRequest.LOAD_BYPASS_CACHE | Ci.nsIRequest.INHIBIT_CACHING;

But that obviously only disables the cache for page load and then the cache is active again for XHR, font imports etc.

bz suggests that we add something like:
Ci.nsIRequest.DISABLE_CACHE: Really, do disable the cache for the lifetime of the web page for all data types.

I am guessing that the cache is disabled for all data types in private browsing mode so I don't think it would be too much work (at least I hope so).

Is this something that you could take a look at?

Filter on 1ff0543e-b501-4893-a72b-e4773c01e655

Comment 15

[Josh Matthews [:jdm]]

Private browsing mode uses an entirely separate cache, rather than disabling it.

Comment 16

[Honza Bambas (:mayhemer)]

(In reply to Josh Matthews [:jdm] from comment #15)
> Private browsing mode uses an entirely separate cache, rather than disabling
> it.

But still it's caching (memory/session only).  So the bug still applies.

Comment 17

[Honza Bambas (:mayhemer)]

We may want to add some flag on load group (not a load flag, rather a property).  When set, set "NO_CACHE" flags on each added request.

Problem is that load group is reused for following document navigation.  But maybe it's not that big problem, since the devtool's "disable cache" switch should work globally anyway, right?

Comment 18

[Honza Bambas (:mayhemer)]

Jason, could you find someone to work on this?  I won't get to anything sooner then in week or two since now.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Honza Bambas (:mayhemer) from comment #17)
> We may want to add some flag on load group (not a load flag, rather a
> property).  When set, set "NO_CACHE" flags on each added request.
> 
> Problem is that load group is reused for following document navigation.  But
> maybe it's not that big problem, since the devtool's "disable cache" switch
> should work globally anyway, right?

The DevTools "disable cache" feature is expected to apply when enabled and the toolbox is open on the tab.  So, I think what you're saying is fine, as long as resetting these new flags / properties to "default" and forcing the tab to reload is enough to go back to normal cache use when the toolbox is closed.

Comment 20

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Honza Bambas (:mayhemer) from comment #17)
> We may want to add some flag on load group (not a load flag, rather a
> property).  When set, set "NO_CACHE" flags on each added request.
> 
> Problem is that load group is reused for following document navigation.  But
> maybe it's not that big problem, since the devtool's "disable cache" switch
> should work globally anyway, right?

Ideally we would be able to disable the cache for a tab and all of it's children whenever the toolbox is open for that tab.

If that would be too much work then disabling the cache for all tabs and their children would be okay.

We don't mind having to reload the tab to apply the changes if we need to.

Comment 21

[Jason Duell]

Michal's going to look into this.

Comment 22

[kud]

Redirections which are cached are also not disabled when the option in devtools is ticked on.

Comment 23

[Dragana Damjanovic [:dragana]]

Attachment: MozReview Request: Bug 1027579 - Introduce DISABLE_CACHE load mode that disable cahe completely. r=:bz

Bug 1027579 - Introduce DISABLE_CACHE load mode that disable cahe completely. r=:bz

Comment 24

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Sorry about the driveby but does this patch disable the cache for:
- dom/base/nsXMLHttpRequest.cpp
- dom/html/HTMLMediaElement.cpp
- dom/media/MediaResource.cpp
- @imported fonts & stylesheets.

There are probably a bunch of other places but nothing should be cached.

Comment 25

[Dragana Damjanovic [:dragana]]

imported fonds are not downloaded again :)

Comment 26

[Boris Zbarsky [:bzbarsky]]

https://reviewboard.mozilla.org/r/12541/#review11151

::: netwerk/base/nsIRequest.idl:224
(Diff revision 1)
> +    const unsigned long DISABLE_CACHE = 1 << 16;

This is already LOAD_DOCUMENT_URI.

::: netwerk/protocol/http/nsHttpChannel.cpp:2758
(Diff revision 1)
> -    if (offline || (mLoadFlags & INHIBIT_CACHING)) {
> +

I'm not a necko peer.  Please ask one of them to review this....

Comment 27

[Dragana Damjanovic [:dragana]]

Attachment: bug_1027579_v1.patch

Not all calls to NS_NewChannel... have loadGroup set therefore for some loadFlags has flag DISABLE_CACHE is set.
I have not changed all loadFlags to have this flag set, because i am afraid that it is error-prone in the future

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8630468 [details] [diff] [review]
bug_1027579_v1.patch

Seems at least plausible, though I didn't read too carefully.  The nsIRequest change should put things in the right order, of course.

Comment 29

[Dragana Damjanovic [:dragana]]

Comment on attachment 8630468 [details] [diff] [review]
bug_1027579_v1.patch

Thanks, Honza!

Comment 30

[Honza Bambas (:mayhemer)]

Comment on attachment 8630468 [details] [diff] [review]
bug_1027579_v1.patch

Review of attachment 8630468 [details] [diff] [review]:
-----------------------------------------------------------------

dropping r now, few questions raised up.  few comments..

::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +2763,5 @@
>      uint32_t cacheEntryOpenFlags;
>      bool offline = gIOService->IsOffline() || appOffline;
> +    uint32_t loadGroupDefaultFlags;
> +    if (mLoadGroup) {
> +        mLoadGroup->GetDefaultLoadFlags(&loadGroupDefaultFlags);

initialize loadGroupDefaultFlags !!

@@ +5062,5 @@
>      gHttpHandler->AddConnectionHeader(&mRequestHead.Headers(), mCaps);
>  
> +    uint32_t loadGroupDefaultFlags;
> +    if (mLoadGroup) {
> +        mLoadGroup->GetDefaultLoadFlags(&loadGroupDefaultFlags);

as well here!

::: toolkit/devtools/server/actors/webbrowser.js
@@ +1466,2 @@
>  
>      this.docShell.defaultLoadFlags = disabled ? disable : enable;

should this be |= ?

@@ +1522,1 @@
>      return this.docShell.defaultLoadFlags === disable;

isn't this wrong? should be 'return !!(t.dS.dLF & d);' ?

Comment 31

[Dragana Damjanovic [:dragana]]

Bug 1182066 will fix most of cases. The bug 1182066 will land soon.
I notice only that fonts do not check defaults flags. Maybe there are some other cases that I have missed.

Comment 32

[Dragana Damjanovic [:dragana]]

In FontFaceSet only loadType is check to decide if cache needs to be bypassed. The default loadGroup flags are not check. I do not know this code so my question:
Can we add check for default flags: around line 
http://mxr.mozilla.org/mozilla-central/source/layout/style/FontFaceSet.cpp#1333
add 
uint32_t flags;
if (NS_SUCCEEDED(docShell->GetDefaultLoadFlags(&flags))) {
  if (flags & nsIRequest::DISABLE_CACHE) {
    *aBypassCache = true;
  }
}

or we need another flag DISABLE_CACHE that is only use for devTools?

John, I am not sure who is the right person to answer this question.

Comment 33

[John Daggett (:jtd)]

(In reply to Dragana Damjanovic [:dragana] from comment #32)
> In FontFaceSet only loadType is check to decide if cache needs to be
> bypassed. The default loadGroup flags are not check. I do not know this code
> so my question:
> Can we add check for default flags: around line 
> http://mxr.mozilla.org/mozilla-central/source/layout/style/FontFaceSet.
> cpp#1333
> add 
> uint32_t flags;
> if (NS_SUCCEEDED(docShell->GetDefaultLoadFlags(&flags))) {
>   if (flags & nsIRequest::DISABLE_CACHE) {
>     *aBypassCache = true;
>   }
> }
> 
> or we need another flag DISABLE_CACHE that is only use for devTools?
> 
> John, I am not sure who is the right person to answer this question.

Yeah, I'm not really the right person, sorry. bz, sicking maybe?

Comment 34

[Dragana Damjanovic [:dragana]]

Can you answer the question in comment #32 (see also comment #31)
Thank you!

Comment 35

[Boris Zbarsky [:bzbarsky]]

So this is the in-memory font cache, right?

Checking the docshell flags is probably ok if those are meant to bypass in-memory object caches.  But are they?  That's not the behavior we have for images last I checked (or at least I really hope not)...

More specifically, it seems to me that disabling the in-memory font cache is only ok if we do one load per font per document to start with.  If we rely on the font cache to coalesce font loads for a particular document, then we'll need some other coalescing mechanism, right?  It would be pretty weird if we loaded a font multiple times just because multiple elements on the page are styled with that font or something, even if caches are supposed to be "disabled".

Comment 36

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Dragana, what's the latest status for the overall issue here?  With bug 1182066 landed, does it mean caching should be correctly disabled for nearly all request types, except for fonts?  Or are there other exceptions?

Comment 37

[Dragana Damjanovic [:dragana]]

I have tested it  and looked though the code, as far as i know it is only fonts.

Comment 38

[Dragana Damjanovic [:dragana]]

Abouts fonts:

So fonds can be cached in UserFontCache and there is a normal necko cache.
Now, if fonts are found in the UserFontCache the fonts are served from there no matter if nsIRequest::LOAD_BYPASS_CACHE is set or not.

If the fonts are not found in UserFontCache and nsIRequest::LOAD_BYPASS_CACHE is not set fonts will be caught from network 2-3 times and afterwards they are served from http cache (tested it with youtube).

If the fonts are not found in UserFontCache and nsIRequest::LOAD_BYPASS_CACHE is set fonts will be caught from network 5-6 times (maybe they will be served from UserFontCache afterwards but I have not check that) 

I have not taken a good look at UserFontCache, I am not sure how this cache is handled, when entries are deleted.

so the question is - can we make this better to fetch fonts only once. In my opinion this should be in UserFontCache and not in http cache. If we fix this we can check nsIRequest::LOAD_BYPASS_CACHE befor serving font from UserFontCache and refresh it. But I do not know much about UserFontCache.

Comment 39

[Dragana Damjanovic [:dragana]]

Hi
can you look at comment 38(and some above). I am not familiar with FontCache code. I am not sure if you are the right person to ask, but please needinfo someone you thing can take a look.

Maybe it would be the better to open another bug for this.

Comment 40

[Jonathan Kew [:jfkthame]]

(In reply to Boris Zbarsky [:bz] from comment #35)
> So this is the in-memory font cache, right?
> 
> Checking the docshell flags is probably ok if those are meant to bypass
> in-memory object caches.  But are they?  That's not the behavior we have for
> images last I checked (or at least I really hope not)...
> 
> More specifically, it seems to me that disabling the in-memory font cache is
> only ok if we do one load per font per document to start with.  If we rely
> on the font cache to coalesce font loads for a particular document, then
> we'll need some other coalescing mechanism, right?  It would be pretty weird
> if we loaded a font multiple times just because multiple elements on the
> page are styled with that font or something, even if caches are supposed to
> be "disabled".

If we bypass the UserFontCache, that shouldn't make us load a font multiple times just because multiple elements are styled with it, provided those elements are using the same font-family and other properties, resolving to the same @font-face rule. But it likely would cause us to load the font multiple times if there are multiple separate @font-face rules referring to the same resource.

I'm inclined to think that's probably OK. If a page has multiple @font-face rules loading the same resource (or if it's frequently reconstructing stylesheets, such that old @font-face rules get thrown away and new, identical ones get created), that sounds like an inefficiency that could usefully be surfaced by noting that it triggers repeated font loads.

Note that there's another issue with the UserFontCache at the moment, which is that it will fail to coalesce font loads if the first load hasn't completed yet at the time when a second rule requests the same resource. The UserFontCache only knows about fonts that have been fully downloaded and validated. See also bug 1188802.

Comment 41

[Dragana Damjanovic [:dragana]]

Attachment: bug_1027579.patch

Is this ok to do?

Comment 42

[Jonathan Kew [:jfkthame]]

Comment on attachment 8658160 [details] [diff] [review]
bug_1027579.patch

Review of attachment 8658160 [details] [diff] [review]:
-----------------------------------------------------------------

IMO, I think it'd be OK to do this; but if Boris thinks it's a bad idea (after considering comment 40), we should definitely listen to his concerns.

Comment 43

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8658160 [details] [diff] [review]
bug_1027579.patch

Yeah, for purposes of this debugging mode this is probably ok given comment 40.

Comment 44

[Dragana Damjanovic [:dragana]]

Attachment: bug_1027579_v1.patch

For comment #43, I have added a separate flag so that it is used only for devtools.

Comment 45

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=323373ccf809

Comment 46

[Jonathan Kew [:jfkthame]]

Comment on attachment 8659243 [details] [diff] [review]
bug_1027579_v1.patch

Review of attachment 8659243 [details] [diff] [review]:
-----------------------------------------------------------------

Although in principle I think this is OK to do, I'm not comfortable with the proposed flag name here; see below for some ideas, though I'd be happy to consider alternatives as well if something else seems more accurate/helpful.

::: layout/style/FontFaceSet.cpp
@@ +1344,5 @@
> +  if (NS_SUCCEEDED(docShell->GetDefaultLoadFlags(&flags))) {
> +    if (flags & nsIRequest::DISABLE_CACHING_COMPLETELY) {
> +      *aBypassCache = true;
> +    }
> +  }

This whole chunk should be inside the |if (docShell)| condition.

::: netwerk/base/nsIRequest.idl
@@ +153,5 @@
> +     * with LOAD_BYPASS_CACHE, because it is going to disable just special
> +     * cases where LOAD_BYPASS_CACHE cannot be used, e.g. FontCache. It is used
> +     * for dev tools.
> +     */
> +    const unsigned long DISABLE_CACHING_COMPLETELY = 1 << 5;

I don't think this is a good name for the new flag, as this bit does not "disable caching completely" at all. As the comment suggests, it is more like a supplement to the existing BYPASS_CACHE.

Maybe something more like BYPASS_ (or DISABLE_) SPECIAL_CACHES?

Are there any other "special caches" that we should consider here? If not, maybe this should be explicitly BYPASS_FONT_CACHE.

Also, please move this so that this group of flag bits are listed in sequential order; that makes it easier to see which bits are defined.

Comment 47

[Dragana Damjanovic [:dragana]]

(In reply to Jonathan Kew (:jfkthame) from comment #46)
> Comment on attachment 8659243 [details] [diff] [review]
> bug_1027579_v1.patch
> 
> Review of attachment 8659243 [details] [diff] [review]:
> -----------------------------------------------------------------
> ::: netwerk/base/nsIRequest.idl
> @@ +153,5 @@
> > +     * with LOAD_BYPASS_CACHE, because it is going to disable just special
> > +     * cases where LOAD_BYPASS_CACHE cannot be used, e.g. FontCache. It is used
> > +     * for dev tools.
> > +     */
> > +    const unsigned long DISABLE_CACHING_COMPLETELY = 1 << 5;
> 
> I don't think this is a good name for the new flag, as this bit does not
> "disable caching completely" at all. As the comment suggests, it is more
> like a supplement to the existing BYPASS_CACHE.
> 
> Maybe something more like BYPASS_ (or DISABLE_) SPECIAL_CACHES?
> 
> Are there any other "special caches" that we should consider here? If not,
> maybe this should be explicitly BYPASS_FONT_CACHE.
> 
> Also, please move this so that this group of flag bits are listed in
> sequential order; that makes it easier to see which bits are defined.

I gave it a more generic name. I only know that this is needed for FontCache, but I am not sure about other cases, maybe there are. I had investigated, but I could not guaranty that this is the only one. 

I changed it to LOAD_BYPASS_FONT_CACHE

Comment 48

[Dragana Damjanovic [:dragana]]

Attachment: bug_1027579_v1.patch

Comment 49

[Jonathan Kew [:jfkthame]]

Comment on attachment 8660658 [details] [diff] [review]
bug_1027579_v1.patch

Review of attachment 8660658 [details] [diff] [review]:
-----------------------------------------------------------------

This seems OK to me, modulo some comment tidyup (see below).

However, note that I'm not a docshell peer (and have no experience there), so you should probably get the addition of this flag reviewed by someone from that module too.

::: netwerk/base/nsIRequest.idl
@@ +129,5 @@
>       */
>  
>      /**
> +     * For devtools it is necessary to disable caching completely and
> +     * LOAD_BYPASS_CACHE will not disable FondCache, because of the performance

s/FontCache/UserFontCache/

@@ +130,5 @@
>  
>      /**
> +     * For devtools it is necessary to disable caching completely and
> +     * LOAD_BYPASS_CACHE will not disable FondCache, because of the performance
> +     * reasons. This flag is used for disabling caching in this spacial case.

s/spacial/special/

@@ +131,5 @@
>      /**
> +     * For devtools it is necessary to disable caching completely and
> +     * LOAD_BYPASS_CACHE will not disable FondCache, because of the performance
> +     * reasons. This flag is used for disabling caching in this spacial case.
> +     * It should be used with LOAD_BYPASS_CACHE.

Hmm, the comment above this group of flags says that they "control the flow of data into the cache"; but that's not an accurate description in this case, AFAICS. This flag affects lookup of existing cache entries (which is actually the more important consideration here) as well as caching of new ones. So perhaps better to add the new flag separately, before that comment.

(It's also not clear to me that INHIBIT_PIPELINE really fits under the above comment either, in the way the next two flags do. Maybe that flag, added back in bug 599164, should have gone above that comment, too.)

Comment 50

[Michal Novotny [:michal]]

Comment on attachment 8660658 [details] [diff] [review]
bug_1027579_v1.patch

Review of attachment 8660658 [details] [diff] [review]:
-----------------------------------------------------------------

r+ for the change in nsIRequest.idl

Comment 51

[Dragana Damjanovic [:dragana]]

Attachment: bug_1027579_v1.patch

only incorporated comments from jfkthame.

Boris, can you review docShell part. Thanks!

Comment 52

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8661114 [details] [diff] [review]
bug_1027579_v1.patch

There isn't any obvious docshell stuff here...  Did you mean the font bits?

I think we should have a better name for this flag that's not font-specific, if it's going to be a general nsIRequest flag; on the other hand nothing else looks at the flag.  We should probably also keep the flags in nsIRequest.idl ordered. 

But past that, what exactly are these unspecified "performance issues" that mean we can't just use LOAD_BYPASS_CACHE?

Comment 53

[Dragana Damjanovic [:dragana]]

(In reply to Boris Zbarsky [:bz] from comment #52)
> Comment on attachment 8661114 [details] [diff] [review]
> bug_1027579_v1.patch
> 
> There isn't any obvious docshell stuff here...  Did you mean the font bits?

jfkthame wanted someone who knows docshell to take a look.

> 
> I think we should have a better name for this flag that's not font-specific,
> if it's going to be a general nsIRequest flag; on the other hand nothing
> else looks at the flag.  We should probably also keep the flags in
> nsIRequest.idl ordered. 
> 

I have not found any other special case that would need a special cache-disable flag. I do not know font's code at all I will try to figure it out :)

> But past that, what exactly are these unspecified "performance issues" that
> mean we can't just use LOAD_BYPASS_CACHE?

comment #40 and comment #43.

Comment 54

[Boris Zbarsky [:bzbarsky]]

> comment #40 and comment #43.

I thought the upshot of those was that just bypassing the font cache on LOAD_BYPASS_CACHE was fine?

Comment 55

[Dragana Damjanovic [:dragana]]

Attachment: bug_1027579.patch

Sorry, I misunderstood. 

this patch blocks loading of fonds from cache if LOAD_BYPASS_CACHE is set.

Comment 56

[Jonathan Kew [:jfkthame]]

Comment on attachment 8661737 [details] [diff] [review]
bug_1027579.patch

Review of attachment 8661737 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine, r=me.

Given all the preceding discussion, I don't think you need an additional review from bz here.

Comment 57

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=19d12314d9cc

Comment 58

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3560ae11e7eb

Comment 59

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3560ae11e7eb

Comment 60

[Andrew Overholt [:overholt]]

Did we fix not letting post-onload xhr load from the cache in this bug?

Comment 61

[Dragana Damjanovic [:dragana]]

(In reply to Andrew Overholt [:overholt] from comment #60)
> Did we fix not letting post-onload xhr load from the cache in this bug?

I am not sure. Maybe  bug 1182066 fix that. Do you have an example (you mean just do  xhr request when document onload is fired)?

I did test some xhr request, but maybe I miss some cases.

Somewhere in this bug I mentioned that it would be good to have list of all cases that do not work to test so that I do not miss something.