Closed Bug 923238 Opened 7 years ago Closed 7 years ago

Swiping away updater download notification should cancel the download

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: snorp, Assigned: eduardn)

Details

(Whiteboard: [lang=java] [mentor=snorp@snorp.net])

Attachments

(1 file, 1 obsolete file)

Right now if you swipe the updater download notification away, the download continues. That's bad.
Mentor bug?
OS: Mac OS X → Android
Hardware: x86 → ARM
Could've sworn I set the mentor flags when I filed. Weird.

Assigned this to Michael Boon, our new updater maintainer.
Assignee: nobody → mtp.boon
Whiteboard: [lang=java] [mentor=snorp@snorp.net]
I'd like to work on this bug, could you give me some directions? Thanks!
Flags: needinfo?(snorp)
(In reply to Eduard Neculaesi (:eduardn) from comment #3)
> I'd like to work on this bug, could you give me some directions? Thanks!

Michael hasn't had time to work on it, and says he's fine giving it up, so it's all yours!

First you need to provide a 'deleteIntent' to the download progress notification. This is the intent that will be fired when the notification is swiped away (see here: http://developer.android.com/reference/android/app/Notification.html#deleteIntent). So you need to provide that intent and then just cancel the download in the handler. I think you'll need to do this in onStartCommand() instead of onPendingIntent(), similar to ACTION_APPLY_UPDATE.
Flags: needinfo?(snorp)
Assignee: mtp.boon → eduardnem
Attached patch Fix for 923238 (obsolete) — Splinter Review
I added a |deleteIntent| to the notification and when that gets called I stop the download and delete the package. The next step (if everything is ok with the patch) would be testing but I don't know where to add tests and how to run them.
Attachment #8355303 - Flags: review?(snorp)
Comment on attachment 8355303 [details] [diff] [review]
Fix for 923238

Review of attachment 8355303 [details] [diff] [review]:
-----------------------------------------------------------------

You need to reset mCancelDownload at the beginning of downloadUpdatePackage(), otherwise it will stay canceled forever (or until the service is recreated).

::: mobile/android/base/updater/UpdateServiceHelper.java
@@ +26,5 @@
>      public static final String ACTION_CHECK_FOR_UPDATE = AppConstants.ANDROID_PACKAGE_NAME + ".CHECK_FOR_UPDATE";
>      public static final String ACTION_CHECK_UPDATE_RESULT = AppConstants.ANDROID_PACKAGE_NAME + ".CHECK_UPDATE_RESULT";
>      public static final String ACTION_DOWNLOAD_UPDATE = AppConstants.ANDROID_PACKAGE_NAME + ".DOWNLOAD_UPDATE";
>      public static final String ACTION_APPLY_UPDATE = AppConstants.ANDROID_PACKAGE_NAME + ".APPLY_UPDATE";
> +    public static final String ACTION_CANCEL_UPDATE = AppConstants.ANDROID_PACKAGE_NAME + ".CANCEL_UPDATE";

How about CANCEL_DOWNLOAD, since that's the specific phase that we are talking about.
Attachment #8355303 - Flags: review?(snorp) → review-
Missed that! Renamed to CANCEL_DOWNLOAD and fixed the |mCancelDownload| problem.
Attachment #8355303 - Attachment is obsolete: true
Attachment #8355578 - Flags: review?(snorp)
Comment on attachment 8355578 [details] [diff] [review]
Fix for 923238 (2)

Review of attachment 8355578 [details] [diff] [review]:
-----------------------------------------------------------------

nice work!
Attachment #8355578 - Flags: review?(snorp) → review+
https://hg.mozilla.org/mozilla-central/rev/ce80d022ae3f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.