588351 - 'Pause' button doesn't work in Downloads Manager

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chwiej Pawel]

User-Agent:       Opera/9.80 (Windows NT 5.1; U; pl) Presto/2.6.30 Version/10.61
Build Identifier: Mozilla/5.0 (X11; Linux armv7l; rv:2.0b5pre) Gecko/20100818 Namoroka/4.0b5pre Fennec/2.0a1pre

Maemo 5
fennec nightly (20100818)


Reproducible: Always

Steps to Reproduce:
1. Go to http://pidgin.im
2. Tap 'Download Pidgin' button
3. On the next page tap again on 'Download Pidgin' link
4. Tap 'Save' button
5. Open browser preferences and tap on downloads icon
6. Tap 'Pause' button

Actual Results:  
It's not possible to pause the download, tapping on 'Pause' button has no effect


Expected Results:  
Downloading should be paused

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We need to support nsIResumableChannel in e10s.

Comment 2

[Josh Matthews [:jdm]]

nsIResumableChannel should be fully supported from the suspend/resume work I did already.  test_resumable_channel_wrap.js passes completely, at least.

Comment 3

[Brian Crowder]

We need to support nsIResumableChannel in ExternalHelperAppParent, is what mfinkle means.   jdm:  Feel free to drop a patch on me, if you like.  If not I'll probably be harassing you for review of my own in the next few days.

Comment 4

[Josh Matthews [:jdm]]

Attachment: Patch

Piece of cake.  Pausing and resuming downloads works like a charm.

Comment 6

[Aakash Desai [:aakashd]]

Some Notes:

- Pause does work if you Cancel/Retry the download first.
- This is seen on Android and Maemo on 8/23 nightly builds.

Comment 7

[[e-mail]]

Comment on attachment 468248 [details] [diff] [review]
Patch

r=dwitte if you want it!

Comment 8

[Brian Crowder]

Wow, thanks jdm!

Comment 9

[Josh Matthews [:jdm]]

http://hg.mozilla.org/mozilla-central/rev/cb00bc18c846

Comment 10

[Tony Chung [:tchung]]

Reopening bug.   This is still broken when trying to pause directly.   I can confirm it pauses only after you cancel/retry as aakash noted.   

Tested against:
Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:2.0b4pre) Gecko/201000901 Namoroka/4.04pre Fennec/2.0a1pre

Mozilla/5.0 (Android; Linux armv7l; rv:2.0b5pre) Gecko/20100901 Firefox/4.0b5pre Fennec/2.0a1pre

Comment 11

[Brian Crowder]

If you cancel/resume, you're actually restarting the download on the chrome side, so that basically means pause doesn't work right for the e10s case.  jdm, do you have thoughts on this?

Comment 12

[Josh Matthews [:jdm]]

I definitely remember it working when testing my changes; I'll see if I can reproduce the problem on my local desktop.

Comment 13

[Josh Matthews [:jdm]]

Hmm, it most definitely doesn't work. I'm seeing this error in the console:

JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDownloadManager.pauseDownload]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://browser/content/downloads.js :: dv_pauseDownload :: line 381"  data: no]

I'm on it.

Comment 14

[Josh Matthews [:jdm]]

Attachment: Add missing interface reference to ExternalHelperAppParent.

Comment 15

[Josh Matthews [:jdm]]

Comment on attachment 471910 [details] [diff] [review]
Add missing interface reference to ExternalHelperAppParent.

I feel pretty dumb.  Apparently I wasn't actually testing it properly, because it couldn't have worked without this change.  Anyways, we should be golden with this patch.

Comment 16

[[e-mail]]

Comment on attachment 471910 [details] [diff] [review]
Add missing interface reference to ExternalHelperAppParent.

Gee gee.

Comment 17

[Josh Matthews [:jdm]]

http://hg.mozilla.org/mozilla-central/rev/5ec06024c508

Comment 18

[Aakash Desai [:aakashd]]

We should have unit tests for this bug, so flagging in-testsuite?. Otherwise, verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:2.0b6pre) Gecko/20100907 Namoroka/4.0b6pre Fennec/2.0b1pre

and

Mozilla/5.0 (Android; Linux armv71; Nokia N900; en-US; rv:2.0b6pre) Gecko/20100907 Namoroka/4.0b6pre Fennec/2.0b1pre