Network > Clear Now button does not clear cache if cache size is set to 0

NEW
Unassigned

Status

()

Firefox
Preferences
9 years ago
3 years ago

People

(Reporter: Alan Baxter, Unassigned)

Tracking

unspecified
Points:
---
Bug Flags:
blocking-firefox3.6 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3

Not wanting a disk cache anymore, I set the cache size to 0 and clicked OK.  I noticed the cache had not been cleared yet, so I went to Options again and clicked Network > Clear Now, but that didn't clear the cache either.

Reproducible: Always

Steps to Reproduce:
1. Set Options > Advanced > Network to non-zero value.
2. Create some disk cache by browsing.
3. Set Options > Advanced > Network to 0 MB of cache and click OK.
4. Click Options > Advanced > Network > Clear Now
Actual Results:  
The cache is not cleared.

Expected Results:  
The cache is cleared.

Bug also present in current Namoroka and Minefield nightlies:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20091014 Namoroka/3.6b1pre
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091014 Minefield/3.7a1pre

Comment 1

9 years ago
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3

I experience the same behavior.
(Reporter)

Updated

9 years ago
Flags: blocking-firefox3.6?
Does setting the cache size to 0 not end up deleting the entire cache the next time you start up Firefox?

Blocking for now, though based on what the actual end state is, we might punt this off to a future release.
Component: General → Preferences
Flags: blocking-firefox3.6? → blocking-firefox3.6+
QA Contact: general → preferences
Assignee: nobody → robert.bugzilla
(Reporter)

Comment 3

9 years ago
(In reply to comment #2)
> Does setting the cache size to 0 not end up deleting the entire cache the next
> time you start up Firefox?

It does not delete the entire cache even after a restart.  There are still about a dozen or so files left in the Cache directory.  To clear the entire cache I have to set the space allowed for the cache to a non-zero amount and click Clear Now.  Then and only then is the entire cache removed.  Just tested in Namoroka Alpha 1 and the most recent nightly: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b2pre) Gecko/20091026 Namoroka/3.6b2pre
Created attachment 409039 [details] [diff] [review]
patch rev1

Simple fix... I'm going to look at the history of nsCacheService.cpp to see if this has always been the case which so far appears to be the case.
Created attachment 409041 [details] [diff] [review]
patch rev2 - only clear the disk cache
Attachment #409039 - Attachment is obsolete: true
Comment on attachment 409041 [details] [diff] [review]
patch rev2 - only clear the disk cache

>diff --git a/browser/components/preferences/advanced.js b/browser/components/preferences/advanced.js
>--- a/browser/components/preferences/advanced.js
>+++ b/browser/components/preferences/advanced.js
>@@ -169,12 +169,20 @@ var gAdvancedPane = {
> 
>   /**
>    * Converts the cache size as specified in UI (in MB) to KB and returns that
>-   * value.
>+   * value and clears the disk cache when it has been set to 0.
>    */
>   writeCacheSize: function ()
>   {
>     var cacheSize = document.getElementById("cacheSize");
>     var intValue = parseInt(cacheSize.value, 10);
>+    if (intValue == 0) {
It might be a good idea to change this check to the following to be consistent with the check that follows it.
if (isNaN(intValue) || intValue == 0) {

>+      // Clear the disk cache since it has been set to 0.
>+      var cacheService = Components.classes["@mozilla.org/network/cache-service;1"]
>+                           	       .getService(Components.interfaces.nsICacheService);
>+      try {
>+        cacheService.evictEntries(Components.interfaces.nsICache.STORE_ON_DISK);
>+      } catch(ex) {}
>+    }
>     return isNaN(intValue) ? 0 : intValue * 1024;
>   },
>
Rob, please pick another reviewer; Gavin's slammed with Fennec atm.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-firefox3.6+ → blocking-firefox3.6-
Attachment #409041 - Flags: review?(gavin.sharp) → review?(dao)
(In reply to comment #6)
> It might be a good idea to change this check to the following to be consistent
> with the check that follows it.
> if (isNaN(intValue) || intValue == 0) {

isNaN is probably a leftover from former days. Since we have a number box, it's not needed. This code should use .valueNumber instead of .value.

As far as the actual fix is concerned, it doesn't seem quite right to me. On Windows, pref changes need to be confirmed with the OK button, otherwise they don't apply. So I don't think the cache should be cleared right away when the user modifies the value.
I was thinking along the same lines but couldn't think of a good solution for this. I considered changing the behavior of the backend so Clear Now always cleared the disk cache but I think that is just as bad as this patch since setting the cache to zero should just clear it.

What do you think about only clearing the cache when zero and setting the pref value when the cacheSize element receives blur? It isn't perfect but I think it is better.
Attachment #409041 - Attachment is obsolete: true
Attachment #409041 - Flags: review?(dao)
An after 3.6 solution might be having a checkbox for disabling the disk cache instead of using 0 MB as being the trigger to disable it.
Blur will happen even if you hit the Cancel button, so I don't think this is substantially better. I think fixing the Clear Now button would be better for now... certainly not a complete fix, but at least no new unexpected behavior.
hmmm... I really don't like the idea of making clear now be the only way to clear the cache when the user sets the disk cache to 0 since I believe the expectation of setting it to 0 is to not store the cache on disk and the only way they would know their cache is still on the disk would be to inspect the file system. Also, making clear now clear after the disk cache is set to 0 would require either setting the disk cache to a greater than zero value, calling evictEntries, and resetting it back to zero or changing the backend code to create the disk device when it isn't present, evicting the entries, and deleting the disk device which is more likely to introduce unexpected behavior than using blur. btw: if the implementation were to use blur it would of course respect cancel and not set the value.

I'll see how well making clear works via the js method described above.
Maybe nsCacheService::SetDiskCacheCapacity, which is called by the browser.cache.disk.capacity observer, should call evictEntries?
Wouldn't that also affect the case where instantApply is true?
Sure, I think that's expected behavior if instantApply is true.
note: I highly suspect that bug 329260 caused this.
Bug 395627 is also of note and is hit in a debug build when setting the cache size to 0.
(In reply to comment #13)
> Maybe nsCacheService::SetDiskCacheCapacity, which is called by the
> browser.cache.disk.capacity observer, should call evictEntries?

This sounds ideal, indeed.
OS: Windows XP → All
Hardware: x86 → All
I agree though I don't think this type of change is appropriate for 1.9.2 so after talking with beltzner and johnathan I'm targeting this for 1.9.3.

Comment 20

4 years ago
I have the same behavior, but even more strange.

When I press 'Clear now' button for the first time, it does nothing, but on second time, it actually clears the cache. So I need to press button twice.

Always.

Firefox 33.1 / Windows 8.1
Assignee: robert.strong.bugs → nobody
You need to log in before you can comment on or make changes to this bug.