Closed Bug 959211 Opened 7 years ago Closed 7 years ago

[Download Manager] Manage stopped downloads and resuming them (offline -> online)

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(tracking-b2g:backlog)

VERIFIED FIXED
1.4 S4 (28mar)
tracking-b2g backlog

People

(Reporter: crdlc, Assigned: crdlc)

References

Details

(Whiteboard: [systemsfe])

Attachments

(1 file)

Use case:

1) Some downloads in progress
2) The device loses connection (3G or WIFI)
3) Downloads are shown as failed in notification bar and download list
4) The device is online after some seconds...

Should downloads be resumed AUTOMATICALLY or by means of the USER interaction?
QA Contact: rafael.marquez
Hi Francis, 

   From your point of view, how should it work?

Thanks a lot
Flags: needinfo?(fdjabri)
Whiteboard: [systemsfe]
Component: Gaia → Gaia::System
Hi Cristian, 

In the case that you mentioned, I would expect that the download should not fail in the first place. If the device loses connectivity, the download manager should be robust enough to keep trying the download  without failing and requiring user intervention. 

In fact, in the case of a device losing connection, I think the download should continue to finish the download until the user manually stops the download.  

Are there cases other than losing connection which would cause the download to fail? If not, then we could remove the failed state altogether. 

Francis
Flags: needinfo?(fdjabri)
Right now the download manager keeps the downloads as stopped when there is not connection and when it is recovered again, downloads are resumed automatically without user intervention, Is it ok right?
Flags: needinfo?(fdjabri)
Hi Cristian, 

I agree that the download should resume automatically without user intervention, but I don't think the download should go into "Stopped" state. The "Stopped" state should be for those downloads that are manually stopped by the user. So, if connection is lost, the downloads should remain in the "Downloading" state until the user manually stops it, or if it fails.
Flags: needinfo?(fdjabri)
Hi Francis, Aus

(In reply to Francis Djabri [:djabber] from comment #4)
> Hi Cristian, 
> 
> I agree that the download should resume automatically without user
> intervention, but I don't think the download should go into "Stopped" state.

If the connection is lost, the stopped event is sent from the API. So it means that if the current behavior is not ok, the API should be reviewed in that case. Although my understanding is that if there is not connection the downloads are stopped IMHO

> The "Stopped" state should be for those downloads that are manually stopped
> by the user. So, if connection is lost, the downloads should remain in the
> "Downloading" state until the user manually stops it, or if it fails.

If it fails, the state is failed, right?
Flags: needinfo?(fdjabri)
Flags: needinfo?(aus)
the state is "stopped" and error is set when an error occurs.
Flags: needinfo?(aus)
Aus,

   I know and I am agree with you but my concern is the Francis' comment 4. Could we close it?

Thanks
Flags: needinfo?(aus)
Hmmm, I do agree with Francis that the state of the UI should probably not go to stopped, but rather go to paused or something like that. As for the API itself, I'm not sure how well I can trap the fact that necko has gone offline and suspended http transactions but will resume them and give you a different state for those. 

I know it's a pain but for now, could we do it by looking at the state being stopped but the downloaded size not equaling the total size of the file to figure out that it's paused? That's the general intention of the API as it's designed.

I can file a bug to enhance the API to have an additional state for suspended downloads because of offline state.

Please note that there will never be a paused state as the user does indeed "stop" the download and it will automatically resume if it can, or restart if it has to. This is how firefox also functions.
Flags: needinfo?(aus)
Inline

(In reply to Ghislain Aus Lacroix [:aus] from comment #8)
> Hmmm, I do agree with Francis that the state of the UI should probably not
> go to stopped, but rather go to paused or something like that. As for the
> API itself, I'm not sure how well I can trap the fact that necko has gone
> offline and suspended http transactions but will resume them and give you a
> different state for those.
> 
> I know it's a pain but for now, could we do it by looking at the state being
> stopped but the downloaded size not equaling the total size of the file to
> figure out that it's paused? That's the general intention of the API as it's
> designed.

Do you mean that "stopped" state and bytes < totalBytes -> "downloading" state? 
How can I realize that the download was stopped by the user in settings apps from system? the state is "stopped" and bytes < totalBytes too, right?

> 
> I can file a bug to enhance the API to have an additional state for
> suspended downloads because of offline state.

I think so

> 
> Please note that there will never be a paused state as the user does indeed
> "stop" the download and it will automatically resume if it can, or restart
> if it has to. This is how firefox also functions.
BTW I think that if you don't change the "downloading" statue in the API when is offline, nothing should be changed in the UI and no new states should be added, right?
Hi guys, sorry, I'm not sure if there's any new UX that needs to be defined here. Based on my comment in comment #4 above at least, no new UI is needed, but please ni? me if there's anything you need. Francis
Flags: needinfo?(fdjabri)
(In reply to Cristian Rodriguez (:crdlc) from comment #9)
> Inline
> 
> (In reply to Ghislain Aus Lacroix [:aus] from comment #8)
> > Hmmm, I do agree with Francis that the state of the UI should probably not
> > go to stopped, but rather go to paused or something like that. As for the
> > API itself, I'm not sure how well I can trap the fact that necko has gone
> > offline and suspended http transactions but will resume them and give you a
> > different state for those.
> > 
> > I know it's a pain but for now, could we do it by looking at the state being
> > stopped but the downloaded size not equaling the total size of the file to
> > figure out that it's paused? That's the general intention of the API as it's
> > designed.
> 
> Do you mean that "stopped" state and bytes < totalBytes -> "downloading"
> state? 
> How can I realize that the download was stopped by the user in settings apps
> from system? the state is "stopped" and bytes < totalBytes too, right?
> 

You could tell that the user stopped the download manually because the method to stop it will get called from a UI event vs a DOM download status change. You can then set a flag on the download to remember that it was stopped by the user so that the subsequent download status change does not set the download into a 'paused' state.

I'm also a little confused, are you not getting an error along with the download status changing to stopped when you lose connectivity?
Flags: needinfo?(crdlc)
> I'm also a little confused, are you not getting an error along with the download status changing to stopped when you lose connectivity?

No idea, let me check it, thx
Flags: needinfo?(crdlc)
Aus,

   When the connectivity is lost, Gaia receives 'stopped' state without error (error is null).

1) API does not change the state from 'downloading' to 'stopped' (comment 4)

or

2) API provides with a new state called 'paused' (comment 8)

or

3) Gaia receives 'stopped' state and when there is not error and !window.navigator.onLine -> remain 'downloading' state (comment 4)

I will work on the third approach
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Summary: [Download Manager] Manage failed downloads and resuming them (offline -> online) → [Download Manager] Manage stopped downloads and resuming them (offline -> online)
Attached file Patch v1
The patch implements the behavior proposed in comment 4. When downloads are stopped because the device is offline, download notifications and items in the list should show them as "downloading" instead of "stopped"

Thanks
Attachment #8388454 - Flags: review?(kaze)
Attachment #8388454 - Flags: review?(francisco.jordano)
blocking-b2g: --- → 1.4?
Comment on attachment 8388454 [details]
Patch v1

LGTM, just left a comment on github just to understand the online/offline events and possible race conditions.
Attachment #8388454 - Flags: review?(francisco.jordano) → review+
Flags: in-testsuite+
You can land on master but we wouldn't hold the release for this.
blocking-b2g: 1.4? → backlog
Comment on attachment 8388454 [details]
Patch v1

Hi Evelyn, I've realized that Kaze has some days off so if you could take a look to this simple patch, it would be great! Thanks in advance
Attachment #8388454 - Flags: review?(kaze) → review?(ehung)
Comment on attachment 8388454 [details]
Patch v1

to me, it's weird to see "downloading" when network is lost, but UX said so in comment 4. r+.
Attachment #8388454 - Flags: review?(ehung) → review+
Merged in master:

https://github.com/mozilla-b2g/gaia/commit/5c9269724905add6dfb16a94e5eac70011fd6b42
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S4 (28mar)
Jason, what do you think about asking for approval here?
Flags: needinfo?(jsmith)
(In reply to Cristian Rodriguez (:crdlc) from comment #21)
> Jason, what do you think about asking for approval here?

It's probably worthwhile. To strengthen the case to get an approval here, we want to have QA verify this on trunk.
Flags: needinfo?(jsmith)
I just verify this bug in master(1.5) branch. It would be good idea to add this patch to v1.4
Status: RESOLVED → VERIFIED
Comment on attachment 8388454 [details]
Patch v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): no regression
[User impact] if declined: no high, just a UX decision regarding to show downloads as downloading when there is not connection
[Testing completed]: add unit test and verified by QA
[Risk to taking this patch] (and alternatives if risky): close to null
[String changes made]: no literals added
Attachment #8388454 - Flags: approval-gaia-v1.4?
I can think of one scenario where I don't want this feature. What about starting the download when I am on wifi and the connection changes to data where I have to pay for it. Should the download actually continue automatically?
Flags: needinfo?(fdjabri)
Waiting for comment #25 to be addressed here before uplifting.
I agree with Gregor. If the download was started on wifi, it should only resume automatically if a connection to wifi is re-established.
Flags: needinfo?(fdjabri)
Gregor, if you consider it as a bug, please file a new bug for the API which is the component that resume the downloads automatically

(In reply to Francis Djabri [:djabber] from comment #27)
> I agree with Gregor. If the download was started on wifi, it should only
> resume automatically if a connection to wifi is re-established.
Flags: needinfo?(anygregor)
Not sure if this should be part of the API or if the gaia app should take care of this. Aus, what do you think?
Flags: needinfo?(anygregor) → needinfo?(aus)
Hmmm, well, we could work around it in gaia by checking when the download resumes if we're on the same type of connection it was started and stop it (and resume it again when we have the same type of connectivity). This is actually the easiest way to go about it.

I do agree though that it would be nice to be able to tell the API how it should behave under these conditions and act accordingly.

The complexity would be mainly in jsdownloads as it's the one that actually resumes the downloads currently. We would then need to expose this as something that's configurable at runtime (maybe defaults to on for fxos and off for others?) when the correct privileges are requested by the application attempting to configure it.
Flags: needinfo?(aus)
I will check the jsdownloads code to see exactly how this is configured. I think there may be a pref for this already...
Gregor,

Please provide your assessment here. Can this be moved to backlog?
Flags: needinfo?(anygregor)
Sorry, meant can this wait or should it be taken in 1.4
Comment on attachment 8388454 [details]
Patch v1

We shouldn't make an exception if we are not 100% confident that we are doing the right thing here. It should ride the regular train.
Attachment #8388454 - Flags: approval-gaia-v1.4? → approval-gaia-v1.4-
Flags: needinfo?(anygregor)
.. what happens when I have WIFI and 3G and in some point I lose the WIFI connection and the download continues with 3G? how does Gaia know?

(In reply to Ghislain Aus Lacroix [:aus] from comment #30)
> Hmmm, well, we could work around it in gaia by checking when the download
> resumes if we're on the same type of connection it was started and stop it
> (and resume it again when we have the same type of connectivity). This is
> actually the easiest way to go about it.

I am not agree here what it is the the easiest way. You want to move some logic of the download manager to the UX. Do you mean that the UX should check always, when a download is resumed, if the connection is 3G or WIFI. Previously saving the kind of connection when it was started. Stop and resume depending on different scenarios, etc... Well, honestly, it could be done but I don't think that this is the correct way to do this.

Which component is resuming automatically? It is the API so I think that is simpler to do this new feature in the place where it is implemented. I mean, the device is online and the connection is wifi or the same when the download was started then resuming, otherwise continue stopped 

Another different stuff is if you say to me, Cristian, the download manager is a component shared between tons of platforms and it will be better if the new feature would be implemented in Gaia instead of touching more shared code in Gecko. Is it the real issue?

> 
> I do agree though that it would be nice to be able to tell the API how it
> should behave under these conditions and act accordingly.
> 
> The complexity would be mainly in jsdownloads as it's the one that actually
> resumes the downloads currently. We would then need to expose this as
> something that's configurable at runtime (maybe defaults to on for fxos and
> off for others?) when the correct privileges are requested by the
> application attempting to configure it.
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.