Checking 'clear history when minefield closes' is not clearing cache on shutdown but on startup

VERIFIED FIXED in Firefox 4.0b5

Status

()

Firefox
Private Browsing
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: Luke Iliffe (Harlequin99), Assigned: mak)

Tracking

({dev-doc-complete, privacy, regression})

Trunk
Firefox 4.0b5
dev-doc-complete, privacy, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2.7) Gecko/20100713 Firefox/3.6.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2.7) Gecko/20100713 Firefox/3.6.7

In a new profile, checking the option to 'clear history when minefield closes' in the privacy pref pane, and ensuring 'cache' is selected in settings, it appears the cache is not cleared when minefield exits, but when it starts up again.

Note history, cookies etc do get cleared on shutdown.

This would have privacy implications allowing someone's cache to be viewed after they existed minefield and walked away.

Discussion on Mozillazine: http://forums.mozillazine.org/viewtopic.php?f=23&t=1950401&start=0

They mention bug 579334 and bug 580374 but I am not technical enough to really know what those bugs are talking about.

Reproducible: Always

Steps to Reproduce:
1. New profile, Firefox button, Options, Privacy, check 'clear history when minefield closes'. Check settings button to ensure 'cache' is selected (default)
2. Open your OS file manager & go to the cache location (On XP %USERPROFILE%\Local Settings\Application Data\Mozilla\Firefox\Profiles\<PROFILENAME>\Cache). Verify the cache files present.
3.Browse to some sites, e.g. google, images, maps to generate some cache content.
4. Refresh your OS file manager and note the newly created cache files.
5. Close minefield. Verify the process has exited in task manager.
6. Cache files are still present in the OS file manager.
7. Restart minefield and as it opens watch the OS file manager, the cache files disappear!
Actual Results:  
Cache is cleared on startup.

Expected Results:  
Cache is cleared on shutdown.

I have tested on XP, Mozilla/5.0 (Windows; Windows NT 5.1; rv:2.0b3pre) Gecko/20100721 Minefield/4.0b3pre

Others on Mozillazine appear to be using Windows 7 and Linux
(Reporter)

Updated

7 years ago
OS: Windows XP → All
Hardware: x86 → All
Version: unspecified → Trunk
(Reporter)

Comment 1

7 years ago
Mozilla Central Regression Range:
Works 2010-04-21 http://hg.mozilla.org/mozilla-central/rev/cdc8bf25220e

Broken 2010-04-22 http://hg.mozilla.org/mozilla-central/rev/cb8f60ee7c23

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cdc8bf25220e&tochange=cb8f60ee7c23

There are a number of possible candidates in the list but I would be just guessing, will leave to others more knowledgeable to work out the offender.
Keywords: privacy, regression
Mak, could this be a Regression by Bug 529821?
blocking2.0: --- → ?
Blocking, and assigning to mak for investigation. Marco, if this needs to go elsewhere, feel free to reassign.
Assignee: nobody → mak77
blocking2.0: ? → betaN+
(In reply to comment #2)
> Mak, could this be a Regression by Bug 529821?

that bug regressed clear history and we fixed it in bug 562644.

Comment 0 has been posted with 3.6.7, but is it relative to latest nightly right?
I can exclude bug 580374.

History and cache are not cleared by the same code clearly, need to check if cache is cleared at the wrong time.
bug 562644 could be culprit by the fact it moved earlier some shutdown stuff, but has to be verified, even if at first look is the only valid candidate.

Will try to figure out something.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Reporter)

Comment 5

7 years ago
(In reply to comment #4)
> Comment 0 has been posted with 3.6.7, but is it relative to latest nightly
> right?

Yes I posted the bug using 3.6.7 but tested with latest nightly, and trunk builds for the regression range. This does NOT occur in 3.6.7 (I did check that too for sanity).
Marco, could bug 578892 also be a dupe of this one?

Comment 7

7 years ago
just a confirmation of what the OP of this thread observed. No clearing of private data when set to do so at closing time. Things weirdly happen when reopening the browser. This concerns cache, forms history, and cookies too, but not browsing history. Browsing history gets cleared when closing FF, as expected, if set to do so.
 The issue with cookies is a bit more complicated, because there's a separate setting interface >>> if cookies are set to be kept until Firefox closes, as opposed "when they expire", then they will be deleted when closing Firefox. But if they are set to be kept until expiration, and on the other hand they are set to be deleted from the general "clearing history when Firefox closes" interface, then they're not deleted at all when closing Firefox.

Comment 8

7 years ago
just to add that downloads and passwords if set to be cleared at closing time are deleted as expected, and when expected.Behavior is okay like for history.

Comment 9

7 years ago
(In reply to comment #8)
> just to add that downloads and passwords if set to be cleared at closing time
> are deleted as expected, and when expected.Behavior is okay like for history.

this was tested on my side with 4.0b2 build1
Whiteboard: [needs investigation]

Comment 10

7 years ago
I reverted to changeset 80e39b33fc3a in local win build, the problem still happens,
And reverted to changeset ab4c6ba5ff70, the problem does not happen.

So, landing of changeset 80e39b33fc3a(Bug 529821) seems to cause a problem.
in bug 588651 Neil noticed that there is no event loop spinning between profile-before-change and xpcom-shutdown, so I had some wrong assumption.
My current plan would be to shutdown Places synchronously at profile-change-teardown, and to finally close the connection (And do the final sync) at profile-before-change.
I also suspect fixing this will show Ts Shutdown regressions, but I'm not much worried about that because it's most likely previous improvements were partly lying to us.

My only doubts are regarding the Sanitizer, to work correctly with clear history it should run at profile-change-teardown, but there are other parties involved (like cache for example) that could try to add data after this notification, making it leaving some stuff behind.

Another possibility could involve adding an enqueueNotifyObservers() api to the observer service, that will notify a topic as soon as current one has finished before giving back control to the caller, so it would be possible to avoid having nested topics.
At first glance, looking at mxr for profile-before-change listeners does not look like there could be issues calling sanitize() at profile-change-teardown.
Created attachment 468563 [details] [diff] [review]
wip v1.0

something like this.

Updated

7 years ago
Attachment #468563 - Flags: feedback+

Updated

7 years ago
Blocks: 588651

Comment 14

7 years ago
just to mention that the issue is still unsolved in 4.0b4, but I suppose you guys know it and are working on it ;)
yes we are aware of the issue and it's a blocker, I'll be back on it as soon as I can.

Comment 16

7 years ago
okay thanks ;)
Whiteboard: [needs investigation]
Created attachment 469825 [details] [diff] [review]
patch v1.0
Attachment #468563 - Attachment is obsolete: true
Created attachment 469840 [details] [diff] [review]
patch v1.1

Added a test for cache too.
Locally all tests are passing, but I'll do a tryserver run to check them and Talos since changing shutdown is always challenging. As soon as tryserver reopens.
Attachment #469825 - Attachment is obsolete: true
Attachment #469840 - Flags: review?(sdwilsh)
once fixed, should update Places page on mdc that reports notifications.
Keywords: dev-doc-needed
this should probably land sooner than later, beta5 would be perfect. While there should be no risk since we were already shutting down even earlier (quit-application) as Neil pointed out on IRC, shutdown changes could always bring some unwanted effect.

Updated

7 years ago
Attachment #469840 - Flags: review?(sdwilsh) → review+
Created attachment 469881 [details] [diff] [review]
patch v1.2

ready to qimport
Attachment #469840 - Attachment is obsolete: true
Flags: in-testsuite?
This could possibly show a fake Ts shutdown regression, I see something on Windows on tryserver even if it's a bit oscillating. this is not changing amount of work we do, but it's moving stuff that could have been ignored (was after xpcom-shutdown) to a more decent and correct position. So the regression, if present, is ignorable.
Flags: in-testsuite? → in-testsuite+
This was pushed a few days ago:
http://hg.mozilla.org/mozilla-central/rev/7557b96e9e02

I haven't heard of any shutdown regressions (but I'm behind on mail).
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b5
(Reporter)

Comment 24

7 years ago
Verified fixed on XP based on my STR in comment 0. Sanity checked common things like cookies and browsing history still behave as expected and they do.

Would be nice if someone could verify on linux , OSX and Win7
Status: RESOLVED → VERIFIED
I've documented the places-connection-closed, places-will-close-connection, and places-connection-closing notifications:

https://developer.mozilla.org/en/Observer_Notifications#Places
Keywords: dev-doc-needed → dev-doc-complete

Comment 26

7 years ago
(In reply to comment #24)
> Verified fixed on XP based on my STR in comment 0. Sanity checked common things
> like cookies and browsing history still behave as expected and they do.
> 
> Would be nice if someone could verify on linux , OSX and Win7

fixed on Windows 7 also.
Blocks: 595633

Comment 27

7 years ago
yeah confirming this is fixed since beta5; I'm running beta6pre now and it works just...well as fine as it always use to since I use Firefox (W7/64 here)
Blocks: 521166

Comment 28

6 years ago
Is this actually fixed?  I don't see any evidence of approval or checkin, and a year later, there is a credible report of an incompletely deleted cache, here:  http://forums.mozillazine.org/viewtopic.php?f=38&t=2376317 .

I can't reproduce the problem, but the user reports that _CACHE_001,CACHE_002, _CACHE_003, and _CACHE_MAP are not deleted.

Comment 29

6 years ago
(In reply to VanillaMozilla from comment #28)
> Is this actually fixed?  I don't see any evidence of approval or checkin,

It was, see comment 23 (and several comments verifying the fix)

> and a year later, there is a credible report [...]

Given that this was fixed and it is a year later, whether it is the same issue or a different issue, it should be reported as a new bug.

Comment 30

6 years ago
(In reply to Michael Lefevre from comment #29)
> Given that this was fixed and it is a year later, whether it is the same
> issue or a different issue, it should be reported as a new bug.

This is bug 706224.

Comment 31

6 years ago
I forgot to mention, I think Preferences is the wrong component for that bug, but I don't want to change it because I'm not entirely sure about Private Browsing either (does it cover just Private Browsing Mode or all privacy matters?).
You need to log in before you can comment on or make changes to this bug.