The default bug view has changed. See this FAQ.

chrome.downloads.pause() should detect when a download cannot be resumed

NEW
Unassigned

Status

()

Toolkit
WebExtensions: Request Handling
P2
normal
6 months ago
3 months ago

People

(Reporter: Mostafa Elsaie, Unassigned)

Tracking

50 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [downloads] triaged)

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
Created attachment 8794121 [details]
Screen shot of error within console

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.79 Safari/537.36 Edge/14.14393

Steps to reproduce:

1) Register a listener for the chrome.downloads.onCreated event
2) Call the chrome.downloads.pause() within the event listener


Actual results:

- Download was canceled
- Every time you try to resume the paused download, you receive the following error:
Unchecked lastError value: Error: Download 1 cannot be resumed



Expected results:

The download should have been paused instead of being canceled when the chrome.downloads.pause() was called.
(Reporter)

Updated

6 months ago
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64

Comment 1

6 months ago
In Firefox,pause and cancel are implemented the same way. Canceled downloads can be resumed as long as the data is still available and the server supports partial requests. Perhaps we should try to detect whether reauming is possible before allowing the pause() call to succeed, though.
OS: Windows 10 → All
Hardware: x86_64 → All
(Reporter)

Comment 2

6 months ago
(In reply to Kris Maglione [:kmag] from comment #1)
> In Firefox,pause and cancel are implemented the same way. Canceled downloads
> can be resumed as long as the data is still available and the server
> supports partial requests. Perhaps we should try to detect whether reauming
> is possible before allowing the pause() call to succeed, though.

Actually whenever I had a bit of delay before calling the pause function(e.g. pausing the debugger before the pause call), everything seems to be just fine. However, if I let the code run through normally, the download is cancelled instead of being paused. This must be a timing issue within the implementation.

Comment 3

6 months ago
Yes, downloads can only be resumed if there's already partial data. If you pause before the transfer has started, there's nothing to resume.

Updated

6 months ago
Priority: -- → P2
Whiteboard: [downloads] triaged

Comment 4

6 months ago
It's still not clear what we should do if a caller tries to pause a download that can't be resumed, though...

Thoughts, Andrew?
Status: UNCONFIRMED → NEW
Component: WebExtensions: Untriaged → WebExtensions: Request Handling
Ever confirmed: true
Flags: needinfo?(aswan)
Summary: chrome.downloads.pause() cancels the download instead of pausing it → chrome.downloads.pause() should detect when a download cannot be resumed

Comment 5

6 months ago
(In reply to Kris Maglione [:kmag] from comment #4)
> It's still not clear what we should do if a caller tries to pause a download
> that can't be resumed, though...

This is already reasonably handled in the API -- each DownloadItem has a canResume property and if you call resume() on a download for which that property is false, it throws an error with a message that describes the situation.
The specific scenario described in this bug would be good to fix though (pause before any data is transferred should be resumable).  Any knowledge I had about downloads has all been paged out of my head, if somebody else wants to dive in and address that problem, I'd be happy to review patches.
Flags: needinfo?(aswan)
Blocks: 1313035
I looked into this, and the reason for the issue is that resuming is conditioned on weather a download .hasPartialData, since we can't actually "pause" downloads, but must cancel and restart them instead.

I understand the initial thought behind this, and it does make some logical sense: a download can only be "resumed" if it is already partially done.  But that is not exactly what happens with our current setup. A download can have both:
 - partial data (and .canResume true), and
 - have resume() actually start the download from beginning, if the server doesn't support the Range header for that resource [1].

Because of that, and for the sake of this bug, I believe that we should actually change this.  A download should be considered resumeable _iff_ it was paused and without errors.

What do you think Andrew and Kris?


1) https://developer.mozilla.org/en-US/docs/Mozilla/Implementing_download_resuming#Resuming_not_supported
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(aswan)

Comment 7

5 months ago
I'm not sure exactly what you're proposing to change, in what scenario do we have a download with partial data and no errors other than having a download that was paused?  Are you trying to cover the case where the download is paused before any data has been transferred to hasPartialData is false and the download ends up non-resumeable?

What we really want to know is whether the server will support Range headers but I don't think we have any way to figure that out ahead of time.
Flags: needinfo?(aswan)

Updated

5 months ago
Duplicate of this bug: 1313035
> I'm not sure exactly what you're proposing to change,
I'm proposing removal of any checks for .hasPartialData in our downloads code.

> in what scenario do we have a download with partial data and no errors
> other than having a download that was paused?  
None.

> Are you trying to cover the case where the download is paused before any data has
> been transferred to hasPartialData is false and the download ends up non-resumeable?
Yes, that's why I'm proposing it as a solution to this bug (among others).

> What we really want to know is whether the server will support Range headers
> but I don't think we have any way to figure that out ahead of time.
Agreed.


Our "pause" semantics are already very far from Chrome's.  We even allow "resuming" a canceled download (possibly because we can't know if it was "paused" in the previous session), but we don't allow resuming a paused one without partial data?  And even when we have partial data, "resuming" sometimes starts from the beginning.

In conclusion, I don't see a single benefit of checking .hasPartialData, and several downsides of leaving it in, one of which is this bug.

(An alternative fix for this bug would be messy + hard to make work across sessions).
I agree that we should support resuming when there's no partial data but resuming is possible.

Either way, I think we need to try to detect cases where a resume would not be possible, and prevent pause() in those cases.

You mentioned downloads with POST data as one obvious case. There are probably some other heuristics to detect some common/obvious cases.

For the other cases, maybe we should consider pausing the transfer and triggering a test request for partial content before actually canceling.
Flags: needinfo?(kmaglione+bmo)
Or maybe we should just go by whether the original request was GET and the original response has a valid Accept-Ranges header.
You need to log in before you can comment on or make changes to this bug.