1027944 - When disk cache is disabled, hide "disk" cache section in about:cache (otherwise it repeats "memory" entries for "disk" category)

Status: NEW

(Core :: Networking: Cache, defect, P5)

Description

[rsx11m]

Steps to reproduce:

 - Switch browser.cache.disk.enable to false
 - go into about:cache
 - look at "memory" summary and entries
 - look at "disk" summary and entries
 - they are identical

This is confusing as the disk cache shouldn't have any active entries.

The expected behavior would probably be either the "disk" section not appearing in this case (as per design prior to bug 916052), or showing zero with no disk location provided.

Observed on trunk (Firefox/SeaMonkey) and aurora (SeaMonkey) nightly builds.

Comment 1

[Honza Bambas (:mayhemer)]

The thing is that "disk" cache now also includes all "memory" entries.  It's by design.

So, if anything to fix, it's the naming - there should be "disk and memory" and "memory-only" sections.

Comment 2

[Honza Bambas (:mayhemer)]

Err... not when enumerating.  The thing is that the disk storage is actually working in memory when browser.cache.disk.enable is false.

Looking at the implementation there seems to be something we can do better here.

Comment 3

[Honza Bambas (:mayhemer)]

I decided to leave the implementation as is.  The lists should show what the disk cache, when programmatically accessed, contains.  And this is it.

Comment 4

[Ben]

Note: When *memory* cache pref is disabled & disk cache still enabled, about:cache shows the correct values for memory entries (0) & still shows "Maximum storage size" value that's entered under "browser.cache.memory.capacity."
But when disk cache is disabled & memory cache enabled, about:cache shows incorrect & confusing values under "disk."
about:cache example when memory cache is disabled, but disk cache is still enabled:
  
memory
Number of entries: 	0
Maximum storage size: 	614400 KiB
Storage in use: 	0 KiB
Storage disk location: 	none, only stored in memory

disk
Number of entries: 	383
Maximum storage size: 	358400 KiB
Storage in use: 	4863 KiB
Storage disk location: 	F:\Mozilla Shared Profiles\Firefox\t681iswd.FX-shrd-5-3-14\cache2
List Cache Entries

Honza,
I don't understand the won't fix stance (not make the about:cache info less confusing; instead making it more useful & understandable).
You said,
"The thing is that "disk" cache now also includes all "memory" entries.  It's by design.
So, if anything to fix, it's the naming - there should be "disk and memory" and "memory-only" sections."

Can you explain that, because it likely doesn't make sense to many advanced, much less average users (that I asked).
Did you really mean, when "browser.cache.disk.enable =false," that all memory cache is ALSO stored on disk?  That's how it reads.  
Memory & disk cache obviously aren't the same, but that's what about:cache (now) indicates when disk cache is disabled.  
If they were the same, about:cache shouldn't show *separate* values when both disk & memory caches are enabled.
 
Listing identical memory cache entries ALSO under "Disk - List Cache Entries" seems wrong & is confusing. 

1) **When users disable disk cache** (browser.cache.disk.enable =false), why show the RAM cache entries also under "disk" at all?  Why not show *disk* cached data 0 KiB, the same as under "memory" when memory cache is disabled?  
* And show the *disk* max storage value that's entered in "browser.cache.disk.capacity" ?  
* When users look in about:cache, it indicates disk cache still holds data (when they disabled it).

2) When disk cache is disabled, then *disk* cache's "Maximum storage size" changes from the value entered in "browser.cache.disk.capacity" to the value for "browser.cache.memory.capacity" (when that isn't disk cache's capacity).  Seems wrong & very confusing.

Thanks for re-considering this & any explanations.

Comment 5

[Honza Bambas (:mayhemer)]

(In reply to Ben from comment #4)
> Note: When *memory* cache pref is disabled & disk cache still enabled,
> about:cache shows the correct values for memory entries (0) & still shows
> "Maximum storage size" value that's entered under
> "browser.cache.memory.capacity."
> But when disk cache is disabled & memory cache enabled, about:cache shows
> incorrect & confusing values under "disk."
> about:cache example when memory cache is disabled, but disk cache is still
> enabled:
>   
> memory
> Number of entries: 	0
> Maximum storage size: 	614400 KiB
> Storage in use: 	0 KiB
> Storage disk location: 	none, only stored in memory
> 
> disk
> Number of entries: 	383
> Maximum storage size: 	358400 KiB
> Storage in use: 	4863 KiB
> Storage disk location: 	F:\Mozilla Shared
> Profiles\Firefox\t681iswd.FX-shrd-5-3-14\cache2
> List Cache Entries
> 
> Honza,
> I don't understand the won't fix stance (not make the about:cache info less
> confusing; instead making it more useful & understandable).
> You said,
> "The thing is that "disk" cache now also includes all "memory" entries. 
> It's by design.
> So, if anything to fix, it's the naming - there should be "disk and memory"
> and "memory-only" sections."
> 
> Can you explain that, because it likely doesn't make sense to many advanced,
> much less average users (that I asked).
> Did you really mean, when "browser.cache.disk.enable =false," that all
> memory cache is ALSO stored on disk?  

No.  What I meant was the following: about:cache shows the internal state of what we internally call "disk cache" and "memory cache".  Those are just two ways to ask the cache for something (a URL's cached metadata+content), those are two lists of entries stored in the cache.  

When we ask the "disk cache" it simply looks at both the on-disk cache (for a file) AND to the memory only cache.  In other words, the "disk cache" contains a list of entries regardless of whether a particular entry is on disk or in memory.  

(Note that a single URL can only be either in memory or on disk.  Not both.  Where an entry is created is determined during it's creation and never changes there on.  To change it, the existing entry is removed and new is created.)

The "memory cache" only lists what is in the memory cache.

> That's how it reads.  

Sorry if that was confusing.

> Memory & disk cache obviously aren't the same, but that's what about:cache
> (now) indicates when disk cache is disabled.  

That's why I suggested the rename.

> If they were the same, about:cache shouldn't show *separate* values when
> both disk & memory caches are enabled.

They are not.

>  
> Listing identical memory cache entries ALSO under "Disk - List Cache
> Entries" seems wrong & is confusing. 
> 
> 1) **When users disable disk cache** (browser.cache.disk.enable =false), why
> show the RAM cache entries also under "disk" at all?  Why not show *disk*
> cached data 0 KiB, the same as under "memory" when memory cache is disabled?

As explained above - the "disk cache" also lists the memory only cache entries.

> 
> * And show the *disk* max storage value that's entered in
> "browser.cache.disk.capacity" ?  
> * When users look in about:cache, it indicates disk cache still holds data
> (when they disabled it).

I think the best solution would be to just hide the disk cache section when disk cache is disabled then.

> 
> 2) When disk cache is disabled, then *disk* cache's "Maximum storage size"
> changes from the value entered in "browser.cache.disk.capacity" to the value
> for "browser.cache.memory.capacity" (when that isn't disk cache's capacity).
> Seems wrong & very confusing.

The thing is that we show values returned by internal structures.  That can really be confusing, since it's not obvious.

> 
> Thanks for re-considering this & any explanations.

Thanks for this comment.  I will reopen and fix this, you guided me to the desired solution here.

Comment 6

[Ben]

Thanks Honza.  This issue doesn't exactly fall in the execute malicious code remotely category, but it was confusing.
I was just concerned that users want to make sure all caches are cleared, like after banking activity.
And concerned / confused if users had critical data being cached in memory, that it wasn't being duplicated to disk - possibly requiring wiping free space after clearing disk cache.  You confirmed that's not the case.
 
I didn't mean to be harsh at all about your earlier comments, if that's how it sounded.  To be fair, programmers possibly have 100's of issues to investigate & quickly write opinions.  Where average users look at only a few per year & more time to dwell.

Comment 7

[Honza Bambas (:mayhemer)]

(In reply to Ben from comment #6)
> Thanks Honza.  This issue doesn't exactly fall in the execute malicious code
> remotely category, but it was confusing.
> I was just concerned that users want to make sure all caches are cleared,
> like after banking activity.
> And concerned / confused if users had critical data being cached in memory,
> that it wasn't being duplicated to disk - possibly requiring wiping free
> space after clearing disk cache.  You confirmed that's not the case.
>  
> I didn't mean to be harsh at all about your earlier comments, if that's how
> it sounded.  To be fair, programmers possibly have 100's of issues to
> investigate & quickly write opinions.  Where average users look at only a
> few per year & more time to dwell.

No, no offense at all :)  You put clear arguments.  Just the comment was a bit longer to read ;)

Comment 8

[Honza Bambas (:mayhemer)]

Not finding time to work on this bug.

Comment 9

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 10

[[:Harest]]

Unfortunate this issue (as minor as it is) has been there for so long. I should also have searched here on this subject before creating a thread in the support forums.

When i changed from disk to RAM because i saw 4.5 Mo/s being written on the SSD* and 90%+ of it was firefox cache related, i saw the changes were immediate but about:cache was still displaying false values since it should be at 0. At least now i know why but it's a nonsense to still be displayed like that imo.

*I feel like browser cache + SSD + video, especially streams, is a very bad combo. Even more on very low TBW disks.

Comment 11

[Peter Lyons Kehl]

Related issue: On about:cache page, Maximum storage size field for disk (that is, disk + memory, as explained above) cache ignores browser.cache.disk.capacity, unless you set browser.cache.disk.smart_size.enabled to false. See https://bugzilla.mozilla.org/show_bug.cgi?id=792248.