Preconditions:

• visit https://www.reddit.com/ and click "I agree" on the cookies banner at the bottom of the page, to have a considerable amount of cookies stored

Steps to reproduce:

1 Open Firefox and go to about:preferences, "Privacy & Security" section.
- Firefox is successfully opened.
- The about:preferences page is successfully loaded.
- The options for "Privacy & Security" section are displayed.

2 In the "Cookies and Site Data" set the "Delete cookies and site data when Firefox is closed".
- The option is selected.

3 Close Reddit or all the sites opened and reopen Firefox.
- All the sites are closed
- The browser opens without any issues.

4 Go to about:preferences#privacy -> Cookies and Site Data -> click on "Manage Data..."
- The "Manage Cookies and Site Data" subdialog is displayed.
- THERE ARE sites listed in the subdialog.

[Tracking Requested - why for this release]:
Not clearing data when we say we do

Indeed, ugh. I can reproduce this and the culprit seems to be bug 1529301.

Locally backing out https://searchfox.org/mozilla-central/diff/75324d92e38f69b12f47530c689a8c7ad7242872/browser/modules/Sanitizer.jsm#690 fixes the issue and correctly sanitizes all principals.

Tom, do you think this is a quick fix? Otherwise should we switch back to getUsage for now? Shutdown perf is nice and all but I would say that integrity of user privacy is more important.

Also it would be great to find out why this wasn't caught by tests and write some that would have caught this.

I need to look into the code more

If I click the "Delete cookies and site data when Firefox is closed" and then go to the website and I cannot reproduce the issue.
If I go to the website and then and click the "Delete cookies and site data when Firefox is closed". And I can reproduce the issue.

(In reply to Tom Tung [:tt, :ttung] from comment #2)

I need to look into the code more

To elaborate the biggest difference is that listInitializedOrigins will have early return is the temporary storage has been initialized. And, it doesn't return the usages of the origins which are unnecessary if we are going to delete them.

Note that the temporary is initialized when the first time we use quota storage (e.g. DOM Cache, IndexedDB, .. etc)

Johann, one thing I want to check if my understanding of the usage for the function is correct:
if the storage is from the previous session, should we remove them as well (by clicking the box)? When I implemented that, my impression is that the function is used to clean the storage in that session so that I made optimization on it.

(In reply to pciapa from comment #0)
A few initial questions and would be great if we can have your replies so that they would help us to narrow down the issue. Thanks in advance!
(To elaborate more, I want to ensure if your quota storage has been initialized. If it's not, then not cleaning the quota storage, in that case, meets my expectation [Note that it could be wrong behavior from the perspective of Johann. If he thinks we should remove the storage no matter whether it's stored in this session])

1. Would you mind going to the website "https://firefox-storage-test.glitch.me/" and share if the results are all green?
2. In your STR, were you in a fresh profile?

For the comment #4

(In reply to Tom Tung [:tt, :ttung] from comment #4)

Johann, one thing I want to check if my understanding of the usage for the function is correct:
if the storage is from the previous session, should we remove them as well (by clicking the box)? When I implemented that, my impression is that the function is used to clean the storage in that session so that I made optimization on it.

It should clear all storage, not just from the last session. But to be clear, in my STR not even the reddit data from the recent session is cleared.

(In reply to pciapa from comment #0)
A few initial questions and would be great if we can have your replies so that they would help us to narrow down the issue. Thanks in advance!
(To elaborate more, I want to ensure if your quota storage has been initialized. If it's not, then not cleaning the quota storage, in that case, meets my expectation [Note that it could be wrong behavior from the perspective of Johann. If he thinks we should remove the storage no matter whether it's stored in this session])

1. Would you mind going to the website "https://firefox-storage-test.glitch.me/" and share if the results are all green?
2. In your STR, were you in a fresh profile?

I can reproduce in an entirely fresh profile, just by going to reddit.com (and accepting the cookies prompt, not sure if that's necessary, the point is that reddit adds some quota storage).

Thanks for taking a look! :)

(In reply to Johann Hofmann [:johannh] from comment #6)

(In reply to Tom Tung [:tt, :ttung] from comment #4)

Johann, one thing I want to check if my understanding of the usage for the function is correct:
if the storage is from the previous session, should we remove them as well (by clicking the box)? When I implemented that, my impression is that the function is used to clean the storage in that session so that I made optimization on it.

It should clear all storage, not just from the last session. But to be clear, in my STR not even the reddit data from the recent session is cleared.

Thanks for the reply!

I see, then I can remove that early return (if the storage hasn't been initialized, quota manager won't return any origin and consequently not remove any storage). I guess that should fix the issue.

Remove the ni since I get enough information.

Depends on D47740

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=19440bf7bc166960e84a677d6de7d7ffb8e628ce

(In reply to Johann Hofmann [:johannh] from comment #6)

I can reproduce in an entirely fresh profile, just by going to reddit.com (and accepting the cookies prompt, not sure if that's necessary, the point is that reddit adds some quota storage).

Some extra info: I didn't accept cookies promt and bug reproduced.

Backed out 3 changesets (bug 1584986) for xpcshell failures in dom/quota/test/unit/test_listOrigins.js

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=269364763&resultStatus=pending%2Crunning%2Csuccess%2Csuperseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=linux%2Cx64%2Cquantumrender%2Cdebug%2Cxpcshell%2Ctests%2Ctest-linux64-qr%2Fdebug-xpcshell-e10s-2%2Cx%28x2%29&revision=8fc63769342ee83654bc9c22ae305915d55af5b2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=269364763&repo=autoland&lineNumber=2582

Backout: https://hg.mozilla.org/integration/autoland/rev/f5d54343a43b9b12145e3ad1cdce0f00af9ce0b9

Ah, I didn't rename the function in the helper of the unit tests... And, I don't understand how could I pass the test locally...

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9282958bed7849a8e800255c159e5968f93c88e6

https://hg.mozilla.org/mozilla-central/rev/5db52cdeeabf
https://hg.mozilla.org/mozilla-central/rev/3cb9575de89a
https://hg.mozilla.org/mozilla-central/rev/62c234fbfdce

Hi Johann,

Could you help me to verify the result is fixed on the latest Nightly? If so, then I will ask for uplifting into beta. Thanks in advance!

Hi pciapa, could you help me to verify the result is fixed on the latest Nightly as well? Thanks in advance!

It wasn't fixed for Nightly

So I had some weird results in the beginning that I think are due to a bug when clearing local file data on shutdown, but the bug as described doesn't reproduce anymore for me in the latest Nightly. So yes, this is fixed for me. pciapa, can you describe more? What steps are you taking? Is this still the exact same issue or something else?

Confirm that for beta is fixed.

Can't check Nightly as it is out of my scope at the moment

(In reply to pciapa from comment #23)

Confirm that for beta is fixed.

Can't check Nightly as it is out of my scope at the moment

The patches haven't been uplifted to beta, so it seems to be another issue if you can still reproduce it on Nightly. Please feel free to reopen this bug or file another bug to track that when you have time to test that on Nightly. Thanks!

(In reply to Johann Hofmann [:johannh] from comment #22)

So I had some weird results in the beginning that I think are due to a bug when clearing local file data on shutdown, but the bug as described doesn't reproduce anymore for me in the latest Nightly. So yes, this is fixed for me. pciapa, can you describe more? What steps are you taking? Is this still the exact same issue or something else?

Based on this comment, at least the issue for only removing the storage in the previous session has been fixed. I am going to uplift patches here to beta.

Comment on attachment 9097610 [details]
Bug 1584986 - P1 - Remove the early return for listInitializedOrigins;

Beta/Release Uplift Approval Request

• User impact if declined: ClearSiteData will only remove the quota storage in the previous session. It should clean all the quota storage.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: As comment #0
  Steps to reproduce:

1 Open Firefox and go to about:preferences, "Privacy & Security" section.

• Firefox is successfully opened.
• The about:preferences page is successfully loaded.
• The options for "Privacy & Security" section are displayed.

2 In the "Cookies and Site Data" set the "Delete cookies and site data when Firefox is closed".

• The option is selected.

3 Close Reddit or all the sites opened and reopen Firefox.

• All the sites are closed
• The browser opens without any issues.

4 Go to about:preferences#privacy -> Cookies and Site Data -> click on "Manage Data..."

• The "Manage Cookies and Site Data" subdialog is displayed.
• THERE ARE sites listed in the subdialog.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): So, P1 only removes the early return check and it should be safe.
  P2 and P3 look a lot, but they only rename the function.

The safest way is only uplifting the P1. But, it would be great if we can also uplift P2 and P3 since the function name should follow its behavior.

• String changes made/needed:

(In reply to Tom Tung [:tt, :ttung] from comment #24)

(In reply to pciapa from comment #23)

Confirm that for beta is fixed.

Can't check Nightly as it is out of my scope at the moment

The patches haven't been uplifted to beta, so it seems to be another issue if you can still reproduce it on Nightly. Please feel free to reopen this bug or file another bug to track that when you have time to test that on Nightly. Thanks!

Tested on Nightly on OSX, when I close Firefox and it stays in Dock, data keeps, so bug exists.
When I Quit Firefox, data is deleted as expected

(In reply to pciapa from comment #27)

Tested on Nightly on OSX, when I close Firefox and it stays in Dock, data keeps, so bug exists.
When I Quit Firefox, data is deleted as expected

Ah, assuming you expect data should be deleted while the window is closed rather than waiting until Firefox quits.

Johann, do you think if we need to update the wording for "Delete cookies and site data when Firefox is closed"? Or, that option should apply to clean quota storage while a window is close? Thanks in advance!

To elaborate more: my understanding is that "Delete cookies and site data when Firefox is closed" refers to quit Firefox. But, we might need to change to wording to reduce confusion. Or, if my understanding is wrong, then we should call another function. Note that if the use (remove storage while closing tab) is needed, we can consider having a function to do that.

I assume deleting cookies and site data happens when you quit the application. Not when a window is closed. We could change the wording at some point, but not for 70 release.

Do you feel sure enough about this fix for beta uplift? Asking Tom and Johann both.

Let me make that more clear. We have only one beta build left (Thursday's) and then, Friday and the weekend to notice and fix any resulting problems. Is this definitely an improvement over what we have in 69? If so then I'm happy to take a chance on it in beta 14.

(In reply to Liz Henry (:lizzard) from comment #30)

Do you feel sure enough about this fix for beta uplift? Asking Tom and Johann both.

If we only have one beta build left, personally, I think P1 is simple enough to uplift. And, it's covered by the unit test inside. From my perspective, the risk of having issues in quota storage is low.

The impact is that:
If that option is set,

• Before the change in this bug, Firefox would only clean the quota storage after the temporary storage is initialized. In other words, if there is no any quota storage being used during the session, then no any quota storage would be removed when closing.
• After the change, Firefox would clean the quota storage when closing.

Note: Quota storage here refers to Localstorage, indexedDB, DOM Cache/ServiceWorker

I'll let Johann decide if it's definitely an improvement that we should uplift to 69.

I would like to uplift this. We're breaking privacy guarantees right now.

Also, feel free to file a bug for the wording, though I wouldn't think it's a big concern.

Comment on attachment 9097610 [details]
Bug 1584986 - P1 - Remove the early return for listInitializedOrigins;

Fix for privacy issue, OK for beta 14 uplift.

Thank you! That seems clear. Let's see how it does in beta.

Hi team,

I reproduced the bug on Release 68.0 and 69.0.2 (64-bit) Build ID 20191001234643 on Windows 10, Mac OSX 10.14 and Ubuntu 18.04.
Verified as fixed on Nightly 71.0a1 (2019-10-08) (64-bit) Build ID 20191008093420 on Windows 10, Mac OSX 10.14 and Ubuntu 18.04.
I will also verify this on beta when the fix lands.

https://hg.mozilla.org/releases/mozilla-beta/rev/73bfb831fd79
https://hg.mozilla.org/releases/mozilla-beta/rev/6ab7b61ef80c
https://hg.mozilla.org/releases/mozilla-beta/rev/860f8d63f7c3

I was able to verify the issue was fixed on beta 70.0b14 (Build ID 20191010142853) for Windows 10 and Ubuntu 18.04. However, on Mac OSX10.14 the cookies for reddit.com were deleted only after I closed the Firefox 70.0b14 app twice:

1. I opened FF beta app, went to reddit.com
2. Went to about:preferences --> "Cookies and Site Data" set the "Delete cookies and site data when Firefox is closed".
3. Closed Reddit page.
4. quit the application from Firefox menu --> Quit Firefox.
5. Open Firefox Beta again, checked "Cookies and Site Data", Reddit cookies were still there.
6. Quit Firefox app again
7. Open Firefox, check "Cookies and Site Data", Reddit cookies were successfully deleted.

I tried this both by closing app from Firefox app menu--> Quit Firefox, and repeated the steps with a new profile, this time Force Quitting app from Dock, the results were the same, I had to quit Firefox 70b14 twice for Reddit cookies to be deleted.

Any thoughts about this? Thanks!

Virginia, can you file a followup bug as a dependent bug on this one?
It's too late to do anything about it for 70 but we could still take a look for 72 or 71.

Thank you Liz, I opened bug https://bugzilla.mozilla.org/show_bug.cgi?id=1588887 to discuss the issue on Mac OS.

Sorry for the delay here, thanks for the follow-up :)

Did you want to nominate P1 for ESR68 uplift too? It grafts cleanly as-landed. P2 would need rebasing if that was also wanted, but not sure that'd be worth the effort.

Based on the previous comments, the issue on Mac is being tracked at https://bugzilla.mozilla.org/show_bug.cgi?id=1588887.
I've updated the qe verify flag and whiteboard.

Comment on attachment 9097610 [details]
Bug 1584986 - P1 - Remove the early return for listInitializedOrigins;

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: The patch corrects the behavior of the "Delete cookies and site data when Firefox is close" in about:preference.
  Before this patch, it would only remove the quota storage when the quota storage has been used successfully.
  After this patch, it would remove the quota storage no matter if the quota storage has been used successfully.
• User impact if declined: When users choose that option and their quota storage hasn't been used successfully while closing the Firefox, the quota storage won't be removed
• Fix Landed on Version: 70, 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch is simple and only remove an early return.

Note that at this moment, I don't want to uplift P2 and P3 to ESR 68 because it might take some time to rebase these two patches. (And P2 is for renaming the function to align the behavior; P3 is for correcting the unit test)

• String or UUID changes made by this patch:

(In reply to Ryan VanderMeulen [:RyanVM] from comment #43)

Did you want to nominate P1 for ESR68 uplift too? It grafts cleanly as-landed. P2 would need rebasing if that was also wanted, but not sure that'd be worth the effort.

Thanks for the notice! I decide not to uplift P2 and P3 since it might take some time to rebase them and I guess the priority for correcting the class name and function for ESR68 probably not so high.

Comment on attachment 9097610 [details]
Bug 1584986 - P1 - Remove the early return for listInitializedOrigins;

Fix for cookie deletion, good for user privacy, let's uplift for 68.3esr.

https://hg.mozilla.org/releases/mozilla-esr68/rev/81742a07a1b3

Hi team,

I was unable to verify the fix in Windows 10 and Mac OSX 10.14 on the following Release versions:

• 70.0.1 (64-bit)
• 70.0 (64-bit)

Any updates on when would this be ready to test?

Thanks!

(In reply to Virginia Balducci from comment #49)

I was unable to verify the fix in Windows 10 and Mac OSX 10.14 on the following Release versions:

• 70.0.1 (64-bit)
• 70.0 (64-bit)

Not sure about the cases in Windows, but there is a bug (bug 1588887) for Mac. (Maybe the issue on the Window is the same issue on Mac).

There is another issue is that QM creates another directory with the same name for the same origin right after the deletion for the original one based on the requests from the website/service worker.

If you want to ensure the issue in this bug is fixed, you could verify the result by:

1. Open FF beta app, go to reddit.com
2. Go to about:preferences --> "Cookies and Site Data" set the "Delete cookies and site data when Firefox is closed".
3. Closed Reddit page.
4. Go the profile (you can find the path in about:profiles) and add one more new file (says, foo.txt) in the directory for Reddit ($profile/storage/default/https+++www.reddit.com)
5. Quit Firefox.
6. Open Firefox Beta again.
7. Check the directory for Reddit the added file (foo.txt) should be removed. (That indicates the new origin directory for Reddit is created after the deletion)

Hi Tom,

I followed the steps you described for Mac OSX 10.14 on Firefox beta 71 (Build ID 20191125204040):
1- Created a txt file on the Reddit directory
2- Quit Firefox,
3- Open Firefox Beta
4- Checked Reddit folder: txt file was deleted.
5- Checked about:preferences -> Cookies and Site Data -> Manage Data
6- "www.reddit.com" cookie was not deleted.

After that, I quit Firefox Beta for the second time and verified that the Reddit cookie was deleted. But, I had to quit Firefox Beta twice. Based on this, the issue still persists.

Also, I tested this on Windows 10 using the steps of comment #26, and reddit cookie was not deleted, issue persists. I couldn't follow Step 4 described in your previous comment (Go the profile (you can find the path in about:profiles) and add one more new file (says, foo.txt) in the directory for Reddit ($profile/storage/default/https+++www.reddit.com) as it appears that the Profile directory on Windows has different folders than Mac OSX.

Any thoughts on this?

Thanks!

(In reply to Virginia Balducci from comment #51)

Hi Tom,

I followed the steps you described for Mac OSX 10.14 on Firefox beta 71 (Build ID 20191125204040):
1- Created a txt file on the Reddit directory
2- Quit Firefox,
3- Open Firefox Beta
4- Checked Reddit folder: txt file was deleted.
5- Checked about:preferences -> Cookies and Site Data -> Manage Data
6- "www.reddit.com" cookie was not deleted.

After that, I quit Firefox Beta for the second time and verified that the Reddit cookie was deleted. But, I had to quit Firefox Beta twice. Based on this, the issue still persists.

So, my understanding is that the issue in this bug is the storage wasn't removed after quitting the Firefox (with choosing the "Delete cookies and site data when Firefox is closed" option). And I fixed the issue.

However, after that, we found another issue. That issue is that Firefox creates the new storage for that origin right after deletion. We filed a bug for that and that bug is bug 1588887. To elaborate more, we suspect that Firefox creates the new storage for that origin because the service worker for the website requests that. Thus, it looks like a timing issue for me at this moment. We definitely want to fix that and it's tracked by bug 1588887.

Therefore, I would say the issue in this bug is fixed and the remaining issue in on bug 1588887 because the quota storage is removed after quitting. What you saw is the newly created one and the STR verifies that because the random file created by us is removed.

If the created file is removed, then it verifies that the storage is removed (which is the issue in this bug). What you found on the about:perference page is another storage created by Firefox after the deletion and that is tracked by bug 1588887. Is that clear to you?

Also, I tested this on Windows 10 using the steps of comment #26, and reddit cookie was not deleted, issue persists. I couldn't follow Step 4 described in your previous comment (Go the profile (you can find the path in about:profiles) and add one more new file (says, foo.txt) in the directory for Reddit ($profile/storage/default/https+++www.reddit.com) as it appears that the Profile directory on Windows has different folders than Mac OSX.

Any thoughts on this?

I checked my Firefox on the Windows machine. There also has the "about:profiles" page. And I can open the profile directory by clicking the "Open Folder" icon. There is a "storage" folder. And so on.

Could you elaborate more on why you couldn't find the directory for Reddit on Windows?

Hi Tom,

I repeated the above-mentioned steps with two new profiles, and I was able to see the "storage" folder on Windows 10, on 71.0 (64-bit) Beta (Build ID 20191128221751).

• The sample txt file is correctly deleted when Firefox is closed, but the "www.reddit.com" cookies remain when checking about:preferences.

If this issue will be fixed on 1588887 based on Comment #52, I can verify this on Beta. Will this require a validation on Release channel, then the steps to verify this bug will include creating a sample txt file on the reddit directory inside the profile and verifying that it has been correctly deleted after reopening Firefox.

(In reply to Virginia Balducci from comment #53)

Hi Tom,

I repeated the above-mentioned steps with two new profiles, and I was able to see the "storage" folder on Windows 10, on 71.0 (64-bit) Beta (Build ID 20191128221751).

• The sample txt file is correctly deleted when Firefox is closed, but the "www.reddit.com" cookies remain when checking about:preferences.

If this issue will be fixed on 1588887 based on Comment #52, I can verify this on Beta. Will this require a validation on Release channel, then the steps to verify this bug will include creating a sample txt file on the reddit directory inside the profile and verifying that it has been correctly deleted after reopening Firefox.

Yes, that is expected behavior and will be investigated(for the causes)/fixed on bug 1588887. Thanks for verifying that!

This was also verified as fixed in ESR 68.3.0 Windows 10 and Ubuntu 18.0.4

Verified as fixed in Beta 75.0b5 (64-bit) and Nightly 76.0a1 (2020-03-18) (64-bit) on Windows 10 and Mac OSX 10.14.

Unless we have some reason to believe this re-broke in 74 (and then re-fixed itself in 75), we shouldn't be setting the status to 74 to affected here. As a more general point, I'm not sure why this bug was re-verified given that it was fixed 3 months ago and verified at the time.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #57)

Unless we have some reason to believe this re-broke in 74 (and then re-fixed itself in 75), we shouldn't be setting the status to 74 to affected here. As a more general point, I'm not sure why this bug was re-verified given that it was fixed 3 months ago and verified at the time.

Hi Virginia, Kindly see the above comment and let us know.

Hi Ryan,

Somehow I looked over, and I did a double-check to make sure it hasn't regressed since the last check.
Sorry about the firefox74 flag, the issue is no longer happening in 74.

Thanks,
Virginia

I am noticing that in Gmail sometimes it has not deleted my user, I have it set to always delete cookies when I log out.

103.0.2 64-Bit Linux Mint 21