851056 - Downloads can't be canceled if the user closes Firefox while the download is in progress

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Download Manager, defect)

Description

[Adrian Tamas (:AdrianT)]

Attachment: log from Firefox Beta

Nightly 22.0a1 2013-03-14 / Firefox Mobile 20 beta 5 / Aurora 21.0a2 2013-03-14
Samsung Galaxy Tab 2 (Android 4.1.1)

Steps to reproduce:
1) Start a large download - try a nightly build for e.g.
2) Minimize the app and close it from the Task Manager
3) Pull down the notification bar and tap on the Download Notification

Expected results:
The user is asked if he wants to cancel the download just like when Firefox is opened

Actual results:
On Beta and Aurora the app is restarted but the user is not prompted if he wants to cancel the download. Tapping the notification dismisses it but the download is still active in the background and will eventualy display a Download Complete notification

On Nightly the notification is instantly dismissed and the download continues in the background.

Comment 1

[Adrian Tamas (:AdrianT)]

Attachment: log from Nightly

Comment 2

[Brian Nicholson (:bnicholson)]

(In reply to adrian tamas from comment #0)
> Created attachment 724868 [details]
> log from Firefox Beta
> 
> Actual results:
> On Beta and Aurora the app is restarted but the user is not prompted if he
> wants to cancel the download. Tapping the notification dismisses it but the
> download is still active in the background and will eventualy display a
> Download Complete notification
> 
> On Nightly the notification is instantly dismissed and the download
> continues in the background.

Weird - on my Droid RAZR, clicking the notification reopens Fennec and shows the prompt, as expected.

Attaching the Fennec application context to the notification more of a hack than a fix. We should implement a real native download manager that won't require Gecko to stay alive during downloads, and I imagine it may also fix issues like this one.

Comment 3

[Brian Nicholson (:bnicholson)]

Attachment: Remove NotificationHandler

Something is broken about the way we broadcast intents to NotificationHandler. Following the STR for Nightly, I never see the intent being sent to or handled by NotificationHandler; the notification immediately dies when tapped. This looks like some side effect of setForeground() since this only happens for foreground in-progress notifications.

I found that opening the Activity directly fixes the issue, so we might as well just remove NotificationHandler since its only purpose is to forward the intent to GeckoApp anyway. I also removed the clearIntent code because we weren't actually doing anything with it.

Finally, I moved super.onDestroy() to the end of onDestroy() in BrowserApp because I was seeing an IllegalStateException in logcat for using the nfc API after being destroyed.

Comment 4

[Brian Nicholson (:bnicholson)]

Attachment: Remove NotificationHandler, v2

Removed some fixes from local testing.

Comment 5

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/42caf4845677

Comment 6

[Brian Nicholson (:bnicholson)]

Bug 823285 didn't land on beta since the risk was too high, so it won't be fixed in Fx20.

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/42caf4845677

Comment 8

[Brian Nicholson (:bnicholson)]

Comment on attachment 725095 [details] [diff] [review]
Remove NotificationHandler, v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 823285
User impact if declined: the fix for bug 823285 won't work on some phones; also, this bug is needed to land bug 850693
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none

Comment 9

[bhavana bajaj [:bajaj]]

Comment on attachment 725095 [details] [diff] [review]
Remove NotificationHandler, v2

low risk uplift, aids fix a top-crasher . Approving for uplift.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/10bbfae8f5a9

Comment 11

[Paul Feher]

Verified on Firefox for Android 21.0b7 using: Nexus 4 (4.2.2)