Last Comment Bug 305462 - "Clear Cache Now" doesn't clear bfcache
: "Clear Cache Now" doesn't clear bfcache
: fixed1.8, privacy, testcase
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: All All
-- normal with 1 vote (vote)
: mozilla1.8beta4
Assigned To: Brian Ryner (not reading)
: Andrew Overholt [:overholt]
Depends on: 292965
  Show dependency treegraph
Reported: 2005-08-21 19:33 PDT by Jesse Ruderman
Modified: 2008-07-31 02:48 PDT (History)
14 users (show)
cbeard: blocking1.8b5+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (341 bytes, text/html)
2005-08-22 03:17 PDT, Jesse Ruderman
no flags Details
patch (6.23 KB, patch)
2005-09-24 14:09 PDT, Brian Ryner (not reading)
no flags Details | Diff | Splinter Review
patch (7.69 KB, patch)
2005-09-27 15:58 PDT, Brian Ryner (not reading)
marria: review+
Details | Diff | Splinter Review
with comments addressed (8.52 KB, patch)
2005-09-28 21:09 PDT, Brian Ryner (not reading)
darin.moz: superreview+
Details | Diff | Splinter Review
branch patch (8.66 KB, patch)
2005-09-29 14:29 PDT, Brian Ryner (not reading)
asa: approval1.8b5+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2005-08-21 19:33:43 PDT
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.
Comment 1 User image Ria Klaassen (not reading all bugmail) 2005-08-22 03:10:25 PDT
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.
Comment 2 User image Jesse Ruderman 2005-08-22 03:17:41 PDT
Created attachment 193423 [details]
Comment 3 User image Chris Beard 2005-08-22 15:01:06 PDT
bryner, could you take a look?  thanks
Comment 4 User image Brian Ryner (not reading) 2005-08-22 15:33:55 PDT
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.
Comment 5 User image Brian Ryner (not reading) 2005-09-24 14:09:07 PDT
Created attachment 197287 [details] [diff] [review]
Comment 6 User image Darin Fisher 2005-09-26 12:03:58 PDT
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).
Comment 7 User image Brian Ryner (not reading) 2005-09-26 14:37:08 PDT
(In reply to comment #6)
> not map directly to the button in the preferences panel.  Perhaps it would be



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 

> 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.
Comment 8 User image Brian Ryner (not reading) 2005-09-27 15:58:14 PDT
Created attachment 197628 [details] [diff] [review]

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.
Comment 9 User image Marria Nazif 2005-09-28 14:01:44 PDT
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 User image Darin Fisher 2005-09-28 19:17:21 PDT
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.
Comment 11 User image Brian Ryner (not reading) 2005-09-28 21:09:10 PDT
Created attachment 197804 [details] [diff] [review]
with comments addressed
Comment 12 User image Darin Fisher 2005-09-29 13:01:31 PDT
Comment on attachment 197804 [details] [diff] [review]
with comments addressed


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.
Comment 13 User image Brian Ryner (not reading) 2005-09-29 13:25:22 PDT
checked in.
Comment 14 User image Brian Ryner (not reading) 2005-09-29 14:29:12 PDT
Created attachment 197908 [details] [diff] [review]
branch patch
Comment 15 User image Brian Ryner (not reading) 2005-09-29 19:15:16 PDT
Checked in on the branch

Note You need to log in before you can comment on or make changes to this bug.