Closed Bug 1114506 Opened 6 years ago Closed 6 years ago

Downloads are not cleared after clearing private data

Categories

(Firefox for Android :: Download Manager, defect)

ARM
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 37
Tracking Status
firefox34 --- unaffected
firefox35 --- unaffected
firefox36 --- unaffected
firefox37 + verified
fennec 37+ ---

People

(Reporter: CristinaM, Assigned: Margaret)

References

Details

(Keywords: regression, reproducible)

Attachments

(1 file, 1 obsolete file)

Environment: 
Device: Asus Transformer TF101 (Android 4.0.3);
Build: Nightly 37.0a1 (2014-12-21);

Steps to reproduce:
1. Download a file (e.g.: 1.usa.gov/deeXKM);
2. Go to Settings -> Privacy -> Clear now;
3. Make sure that all the options are checked and tap on 'Clear data' button;
4. Go to Tools - > Downloads.

Expected result:
The downloads list is empty.

Actual result:
The downloaded file is present in the downloads list.

Regression window:
Last good: 2014-12-16
First bad: 2014-12-17

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b836016d82b5&tochange=b7eb1ce0237d

Probably Bug 901360 caused the regression.
[Tracking Requested - why for this release]: Regressions from bug 901360

I was just looking into some remaining nsIDownloadManager uses in the "mobile/android" folder, and I expected this regression from code inspection:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/Sanitizer.jsm#205

On Desktop, this now uses DownloadList.removeFinished:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/sanitize.js#341

It seems on Android you want to remove files on disk though, so you might need your own implementation. It would require just a few lines, you can look at the current removeFinished implementation for reference:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadList.jsm#235
Depends on: 901360
Is this actually a regression? Bug 1107510 seems to indicate that this was an existing issue.
Bug 1107510 was reproducible only in private browsing. 
This issue is reproducible starting from 2014-12-17, I am not able to reproduce it on previous builds, so I guess is a regression.
I can look into this.
Assignee: nobody → margaret.leibovic
tracking-fennec: --- → ?
Blocks: 901360
No longer depends on: 901360
Looks like Sanitizer.jsm still uses the old downloads API:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/Sanitizer.jsm#22
Attachment #8540335 - Flags: review?(paolo.mozmail)
Attachment #8540335 - Flags: review?(mark.finkle)
/r/1649 - Bug 1114506 - Update Sanitizer.jsm to use Downloads.jsm to clear downloads

Pull down this commit:

hg pull review -r 622e088926a9e57c78bbd81563fc370fe127785d
Attachment #8540335 - Flags: review?(mark.finkle) → review+
https://reviewboard.mozilla.org/r/1647/#review1067

::: mobile/android/modules/Sanitizer.jsm
(Diff revision 1)
> +            // Delete the downloaded files themselves.
> +            let file = new FileUtils.File(download.target.path);
> +            if (file.exists()) {
> +              file.remove(false);
>              }

You should use OS.File.remove in new code. For efficiency, we recommend just invoking remove and catching exceptions, rather than checking for existence first. I'd also report real errors but continue with the next download, something similar to this:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadCore.jsm#1995

(You might not need the becauseAccessDenied check and may want to report those errors as well.)
Attachment #8540335 - Flags: review?(paolo.mozmail)
https://hg.mozilla.org/mozilla-central/rev/d6fb576da01d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Verified as fixed on latest Nightly on Nexus 7(Android 5.0.1)
Status: RESOLVED → VERIFIED
Blocks: 1107510
Duplicate of this bug: 1107510
tracking-fennec: ? → 37+
Attachment #8540335 - Attachment is obsolete: true
Attachment #8618945 - Flags: review+
You need to log in before you can comment on or make changes to this bug.