Build: Beta 53.0b7 (2017-28-03); Devices: -Asus ZenPad 8.0 Z380KL (Android 6.0.1) -Huawei Honor 5X (Android 5.1.1) Steps to reproduce: 1. Go to Settings>Privacy and tap on 'Clear private data on exit' 2. Select 'Downloads' and tap Set. 2. Download some files: e.g. images from Google (I did not find a relation between the issue and files types). 3. Quit Firefox. 4. Downloads are cleared on the first try. 5. Download again some files and quit. Repeat until you can still see the files in about:downloads. Expected results: The downloads are cleared on exit everytime. Actual results: Downloads are not always cleared from the download manager, but are removed from the internal storage. If you try to open the file from about:downloads it won't do anything.

This sounds like a fairly bad privacy bug.

Honor 6 - about:downloads not cleared but the downloaded files get deleted from storage, thats why i unticked this some time ago. Now i lost again plenty of saved files.

Aurora 54.0a2 (2017-03-28)

snorp, can you help investigate or find an owner for this bug? Thanks.

This is more front end than platform.

This almost sounds like it could be related to the problems we have with removing the history when quitting. @JanH: Quick question, from fixing the history bug, could we have the same problem with downloads or do you think it's unrelated?

If this doesn't happen when clearing downloads while Firefox is running via Settings -> Clear private data, then yes, this sounds like somehow somewhere we're doing something async and not waiting for its completion, although from looking at it, at least the sanitizer code itself (https://dxr.mozilla.org/mozilla-central/rev/272ce6c2572164f5f6a9fba2a980ba9ccf50770c/mobile/android/modules/Sanitizer.jsm#253) doesn't look immediately suspicious.

We did have problems in the sanitizer before, I remember. Maybe something regressed there.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #8) > We did have problems in the sanitizer before, I remember. Maybe something > regressed there. I fixed a similar problem in bug 1163937; but at first glance it doesn't look like there have been any significant changes to the patch I landed for that.

Regression window performed: Last good revision: 5e0140b6d11821e0c2a2de25bc5431783f03380a (2016-02-27) First bad revision: 5e0140b6d11821e0c2a2de25bc5431783f03380a (2016-02-28) Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=5e0140b6d11821e0c2a2de25bc5431783f03380a

[triage] Borderline critical privacy bug. To be clear, it looks like files are removed from disk but not in about:downloads (comment 2).

Mike what’s the attack surface to glean those items from about:downloads? If not very high I would keep it as typical P1.

tldr; low – a user would need access to the unlocked device to see this file but I'm most concerned about the use case like: 1) user downloads file they want to keep private 2) deletes downloads, and assumes this accident was erased 3) they hand their device to another user to use, assuming no private data will be exposed 4) Other user sees this data Full explanation: if about:downloads keeps the list of downloaded items, it's likely keeping this data in a file. Thus the data can be accessed from the UI (which would require another user to have access to the device) or from this file. Android devices are encrypted by default (starting with M (a little over 50%) and some L devices (75% of devices)) and sandboxes apps from accessing another app's data so these files should be protected from everyone without access to the unlocked device expect for users who root their device (and they're on their own). Susheel, what do you think about the priority here? Sounds like you think it should stay P1. Android version distribution: https://developer.android.com/about/dashboards/index.html

yes, stay as p1.

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195 Needinfo :susheel if you think this bug should be re-triaged.

I tend to think this should still be P1 since it is a clear privacy issue.

Added to work queue.

I've tried to reproduce this today but was not able to. Is this still an issue? What I've observed though is that canceled downloads (when Quit-ing) still remain both in about:downloads and on disk.

I was not able to reproduce either on several devices but not the reporting one. Petru, I think the huawei is with you guys. You might want to try with that. We did reproduce the canceling download issue - are you gonna report it or should we?

Tested this including on the Honor 5X for which it was reported and could not reproduce. Regarding the issue of not clearing partial downloads I've filed bug 1492404

Hello everyone, Following the steps provided, I was able to reproduce the issue today, on Nightly 64.0a1 (2018-09-27) on HTC Desire 820 (Android 6.0.1). Also, I checked all the boxes from Settings -> Privacy -> 'Clear private data on exit', tapped "Set" and quitted Firefox. The files were still in about:downloads, but could not be opened. Please see attached video. Thanks!

Logcat here,

Tested with Mira and saw that in the cases in which this fails the call to persist the current number of downloads (which after sanitizing should be 0) https://dxr.mozilla.org/mozilla-central/rev/7e9ab0e7b608ddb4d84f36194509f498c66e3449/toolkit/components/downloads/DownloadStore.jsm#140 is never called. This might be because the call to persist https://dxr.mozilla.org/mozilla-central/rev/7e9ab0e7b608ddb4d84f36194509f498c66e3449/mobile/android/modules/Sanitizer.jsm#358 is not blocking (in the case of downloadFiles `clear` is a generator function but it's not used as such) and the app closes before having the chance to save the current number of downloads.

There are 2 ways we ensure downloads are sanitized before shutdown: 1) DownloadIntegration.forceSave() Need to use the already initialized instance of DownloadIntegration, which has an already initialized DownloadStore to actually store the updated list of downloads. 2) A Blocker for AsyncShutdown.profileBeforeChange Will ensure DownloadStore will finish writing the updated list of downloads. Must use await to actually block until that operation completes.

Seems like the sanitization was broken for a long time. In the cases it worked we were just lucky to win the race from [1] > AsyncShutdown.profileBeforeChange.addBlocker("DownloadAutoSaveView: writing data", > () => this._writer.finalize()); which didn't actually block and the actual shutdown of the app. DownloadIntegration.forceSave() also didn't do anything as it would always return `Promise.resolve` [2] because we would use another instance of DownloadIntegration, not the one already initialized with a DownloadStore. The above patch ensures we use the global DownloadIntegration object so that forceSave() will block until actually saving the updated list of downloads. The Blocker for AsyncShutdown.profileBeforeChange will also block and ensure all writes (not necessarily the ones from Sanitizer) are completed before shutting down. [1] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/toolkit/components/downloads/DownloadIntegration.jsm#1050 [2] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/toolkit/components/downloads/DownloadIntegration.jsm#792

Updated my initial patch to remove the DownloadIntegration part. It was more of a speculative fix. Thanks :paolo for highlighting the incorrect usage of AsyncShutdown.profileBeforeChange()!

Pushed by ncsoregi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dbab4e3611fb Ensure sanitizing before shutdown; r=jchen

Verified as fixed on Nightly 64. LG Nexus 5 (Android 6.0.1) Mi 4i (Android 5.0.2) Google Pixel (Android 9)

Not a new issue and we're in RC week. Let's let this ride the trains.