1092393 - [Downloads] Restarting a download in progress, the failed download date will be changed

Status: RESOLVED FIXED

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

Description

[pete siphantong [:Petes]]

Attachment: dl.txt

Description:
When user restarts a download in progress, the failed download date will be changed in the download list

Setup Steps:
1) Flame device is set to 319mb

Repro Steps:
1) Update a Flame device to BuildID: 20141031001201
2) Download a file from  https://owd.tid.es/dm/ 
3) Tap home button > pull down the notification screen
4) Restart the phone
5) Go to settings > downloads


Actual:
the failed download date changed

Expected:
the failed download date display correctly

Flame 2.1 

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141031001201
Gaia: f89c7b12c36572262c9ea76058694a139b1a8634
Gecko: 50d48f8a04c7
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Repro frequency: 100%
See attached: screenshot, logcat

Comment 1

[pete siphantong [:Petes]]

This issue also reproduces on the Flame 2.2 and the Flame 2.0, the failed download date changed.

Flame 2.2

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141031061804
Gaia: a07994714f0552f89801d6097982308d8b0a1ee1
Gecko: 6bd2071b373f
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2) 
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0


Flame 2.0

Device: Flame 2.0 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141031000201
Gaia: 7b8df9941700c1f6d6d51ff464f0c8ae32008cd2
Gecko: 82a6ed695964
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 32.0 (2.0) 
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[Joshua Mitchell (Inactive)]

Pete - can you attach a video - triage will want to visually see what 'date changed' looks like to fully grasp this bug.

Comment 3

[pete siphantong [:Petes]]

Attachment: 2014-10-31-14-38-14.png

The date changed to 01/08/1970

Comment 4

[Joshua Mitchell (Inactive)]

NI to Settings owner for a blocking call

Comment 5

[Gerry Chang [:gchang]]

[Blocking Requested - why for this release]:
Hi Howie, 
This may impact user's experience. But, still need your help to determine if this is a blocker.

Comment 6

[howie [:howie]]

System FE team will help to triage this.

Comment 7

[Manuel Casas Barrado [:mancas]]

Hey, I've tried to reproduce it and I've noticed that the failed downloads are stored in |mozDownloadManager| rather than in |DownloadStore|. So it seems that the date is broken when the mozDownloadManager changes the |startTime| of a specific download.

Comment 8

[Gregor Wagner [:gwagner]]

Lets uplift if the fix is low risk.

Comment 9

[Michael Henretty [:mikehenrty][:mhenretty]]

Alberto, can you take a look here?

Comment 10

[Alberto Pastor [:albertopq]]

That date is coming invalid straight away from the API:

In here:

https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/downloads/download_api_manager.js#L88

apiDownloads[0].startTime is already from 1970.

It looks like a good first bug for the Backend. Aus, would you guide me a little bit on the process for fixing this? I guess the issue is around:

http://hg.mozilla.org/mozilla-central/file/72ae882fce1a/dom/downloads/DownloadsAPI.js#l182

Thanks!

Comment 11

[Aus Lacroix [:aus]]

I think this is happening because the underlying Gecko JS Downloads API never records to it's datastore the start time for this download because it's unaware of the shutdown that's about to happen. 

Asking QA to see if this bug occurs in Desktop Firefox? It should be similar to killing the process when there's an active download.

If we can catch at the gaia level the fact that we're restarting, we could simply pause all downloads before restarting the phone. This will allow everything to work as expected when the phone restarts.

Normal failed downloads (because of loss of connectivity or what not, should be recorded as expected in js downloads datastore and have a proper start time).

Comment 12

[Aus Lacroix [:aus]]

QAWanted, see comment #11.

Comment 13

[Alberto Pastor [:albertopq]]

(In reply to Ghislain Aus Lacroix [:aus] from comment #11)
> I think this is happening because the underlying Gecko JS Downloads API
> never records to it's datastore the start time for this download because
> it's unaware of the shutdown that's about to happen. 
> 

I'm working on a Gaia patch for pausing downloads when the 'requestshutdown' event occurs, but why do we need to wait for the shutdown for storing the startTime? Can't we just set it when creating the download?

Comment 14

[Aus Lacroix [:aus]]

(In reply to Alberto Pastor [:albertopq] from comment #13)
> (In reply to Ghislain Aus Lacroix [:aus] from comment #11)
> > I think this is happening because the underlying Gecko JS Downloads API
> > never records to it's datastore the start time for this download because
> > it's unaware of the shutdown that's about to happen. 
> > 
> 
> I'm working on a Gaia patch for pausing downloads when the 'requestshutdown'
> event occurs, but why do we need to wait for the shutdown for storing the
> startTime? Can't we just set it when creating the download?

Absolutely, the underlying jsdownloads api that is used by the gecko dom downloads api is probably not saving this information when the download is created. Only when it is suspended or completed. We should probably fix this in the js downloads api itself if that's indeed the problem.

Flagging Paoli on this.

Comment 15

[Aus Lacroix [:aus]]

I mean Paolo, apologies :)

Comment 16

[:Paolo Amadini]

The startTime property is set by the JavaScript API when the download is started, and the change in the "stopped" property that happens when the download starts should trigger the serialization of the download to the "downloads.json" file, because the "serialization hash" changes:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadCore.jsm#943

When reloading "downloads.json", the startTime property should also be restored. Off the top of my head, I don't see why this would work on Desktop and not on B2G.

Comment 17

[Cody Roesch [:croesch]]

(In reply to Ghislain Aus Lacroix [:aus] from comment #11)
> I think this is happening because the underlying Gecko JS Downloads API
> never records to it's datastore the start time for this download because
> it's unaware of the shutdown that's about to happen. 
> 
> Asking QA to see if this bug occurs in Desktop Firefox? It should be similar
> to killing the process when there's an active download.
> 
> If we can catch at the gaia level the fact that we're restarting, we could
> simply pause all downloads before restarting the phone. This will allow
> everything to work as expected when the phone restarts.
> 
> Normal failed downloads (because of loss of connectivity or what not, should
> be recorded as expected in js downloads datastore and have a proper start
> time).



So I checked on Firefox browser for Unbuntu OS and found that if I manually close the process of a large download, the correct time will appear next to the download.

If instead I simply reboot the computer while the download is taking place, the download will just resume once the browser is started again after the reboot.

Hope this helps!

Comment 18

[Alberto Pastor [:albertopq]]

Alive, do you know if is there any event in the system we can listen to, in order to know when the phone is going to shutdown? I tried batteryshutdown and requestshutdown, and don't seem to work... Thanks!

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alberto Pastor [:albertopq] from comment #18)
> Alive, do you know if is there any event in the system we can listen to, in
> order to know when the phone is going to shutdown? I tried batteryshutdown
> and requestshutdown, and don't seem to work... Thanks!

No, there is no such event.
A workaround is dispatch event in sleep menu before it calls mozPower.restart(); BUT please note this does not work if the batter is really drained. We cannot ask the phone to wait us before shutdown.

Comment 20

[Alberto Pastor [:albertopq]]

Attachment: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/26289

Pausing downloads when shutting down

Comment 21

[Aus Lacroix [:aus]]

Comment on attachment 8525279 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/26289

LGTM, but, let's try and add a test or two for this to ensure that 'pause' gets called on the download as expected when receiving the event and that the sleep menu fires the event we require. They can be unit-tests, I don't feel like it's necessary to have integration tests in this case. I would also like to clone this bug so that we fix the API itself and get rid of this code later when this happens. :)

Comment 22

[Alberto Pastor [:albertopq]]

master: https://github.com/mozilla-b2g/gaia/commit/13567aaad7e92efd78daf2770460315cc402a9ca