The default bug view has changed. See this FAQ.

"Clear Cache Now" doesn't clear bfcache

RESOLVED FIXED in mozilla1.8beta4

Status

()

Core
Document Navigation
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: Jesse Ruderman, Assigned: Brian Ryner (not reading))

Tracking

({fixed1.8, privacy, testcase})

Trunk
mozilla1.8beta4
fixed1.8, privacy, testcase
Points:
---
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

12 years ago
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050821
Firefox/1.0+ ID:2005082116

"Clear Cache Now" doesn't clear bfcache.  I think it should, since bfcache
usually holds at least as much private information as the normal cache for the
same page.
(Reporter)

Updated

12 years ago
Flags: blocking1.8b4?
OS: MacOS X → All
Hardware: Macintosh → All
Version: 1.8 Branch → Trunk
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050821
Firefox/1.0+ ID:2005082121

I see this too.
(Reporter)

Comment 2

12 years ago
Created attachment 193423 [details]
testcase

Comment 3

12 years ago
bryner, could you take a look?  thanks
Assignee: nobody → bryner
Flags: blocking1.8b4? → blocking1.8b4+
Target Milestone: --- → mozilla1.8beta4
(Assignee)

Comment 4

12 years ago
This is going to be easier once all of the session histories can be enumerated,
which is going to be part of bug 292965.  Deferring this until that one is fixed.
(Reporter)

Updated

12 years ago
Depends on: 292965

Updated

12 years ago
Keywords: testcase
(Assignee)

Comment 5

12 years ago
Created attachment 197287 [details] [diff] [review]
patch
Attachment #197287 - Flags: superreview?(darin)
Attachment #197287 - Flags: review?(marria)

Comment 6

12 years ago
What about nsICacheSession::evictEntries?  It seems like this observer event is
only generated by a call to nsICacheService::evictEntries, which takes a
parameter to control the storage type to evict.  In other words, this API does
not map directly to the button in the preferences panel.  Perhaps it would be
better to generate the observer event from the prefs panel.  It would almost
make sense for the cache service to observe that event instead (but I might hold
off on that change for now).
(Assignee)

Comment 7

12 years ago
(In reply to comment #6)
> not map directly to the button in the preferences panel.  Perhaps it would be

browser/base/content/sanitize.js:

        cacheService.evictEntries(ci.nsICache.STORE_ON_DISK);
        cacheService.evictEntries(ci.nsICache.STORE_IN_MEMORY);

So it will get invoked twice, true... I don't really know a good way to avoid 
that though, since the content viewer caching doesn't correspond entirely to 
either memory or disk cache (it's an in-memory cache, to be sure, but should 
also go away if the disk cache is cleared, since it contains similar 
information).

> better to generate the observer event from the prefs panel.  It would almost

I specifically wanted to avoid that, because it would require changes to any 
Gecko client that has a "clear cache" option.  The back forward cache is a 
detail that Gecko clients shouldn't need to worry about.
(Assignee)

Updated

12 years ago
Attachment #197287 - Flags: superreview?(darin)
Attachment #197287 - Flags: review?(marria)
(Assignee)

Comment 8

12 years ago
Created attachment 197628 [details] [diff] [review]
patch

This patch is more inclusive about sending the notifications.  I changed the
call sites in browser and xpfe to avoid notifying more than once, however, a
client that does flush memory and disk separately will still empty the bfcache
as designed.
Attachment #197287 - Attachment is obsolete: true
Attachment #197628 - Flags: superreview?(darin)
Attachment #197628 - Flags: review?(marria)

Updated

12 years ago
Attachment #197628 - Flags: review?(marria) → review+

Comment 9

12 years ago
This looks good to me, except i would put some big disclaimer comment by
the EvictAllContentViewers function explaining how inefficient it is.
I marked as approved so you can go ahead and submit after adding the comment.

Comment 10

12 years ago
One more concern:  I believe that the nsICacheService is an API designed to be
called on any thread (hence mCacheServiceLock).  nsIObserverService is not
threadsafe, however.  So, if someone decided to trigger cache eviction on a
background thread, then suddenly lot's of other stuff would run on that thread.
 I think this means that you need to use NS_GetProxyForObject.  See
nsCategoryManager, which has to deal with exactly the same problem.
(Assignee)

Updated

12 years ago
Attachment #197628 - Flags: superreview?(darin)
(Assignee)

Comment 11

12 years ago
Created attachment 197804 [details] [diff] [review]
with comments addressed
Attachment #197628 - Attachment is obsolete: true
Attachment #197804 - Flags: superreview?(darin)

Comment 12

12 years ago
Comment on attachment 197804 [details] [diff] [review]
with comments addressed

s/objcets/objects/

please add a comment in nsICacheService.idl detailing the observer topic. 
someone should not have to look at the source code to see what this thing
means.	for example, you might want to follow the documentation found at the
bottom of nsIHttpProtocolHandler.idl.

otherwise, patch looks good to me.  sr=darin with those nits picked.
Attachment #197804 - Flags: superreview?(darin) → superreview+
(Assignee)

Comment 13

12 years ago
checked in.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

12 years ago
Created attachment 197908 [details] [diff] [review]
branch patch
Attachment #197908 - Flags: approval1.8b5?

Updated

12 years ago
Attachment #197908 - Flags: approval1.8b5? → approval1.8b5+
(Assignee)

Comment 15

12 years ago
Checked in on the branch
Keywords: fixed1.8

Updated

9 years ago
Component: History: Session → Document Navigation
QA Contact: history.session → docshell
You need to log in before you can comment on or make changes to this bug.