Closed Bug 1180250 Opened 4 years ago Closed 4 years ago

No download listed in Downloads section

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.5+)

RESOLVED FIXED
FxOS-S7 (18Sep)
blocking-b2g 2.5+

People

(Reporter: gerard-majax, Assigned: tedders1)

References

Details

(Keywords: dataloss, foxfood, regression, Whiteboard: [systemsfe])

Attachments

(1 file)

STR:
 0. Download a file
 1. Open Settings, go to Downloads

Expected:
 File is listed

Actual:
 Downloads sections is empty

Logcat shows:
> 07-03 18:03:25.992  1987  1987 E Settings: [JavaScript Error: "TypeError: download.path is undefined" {file: "app://settings.gaiamobile.org/shared/js/download/download_formatter.js" line: 7}]
FYI, to Yura and Michael we probably should get this fixed if this is the case.

QAWanted for a check to see if it happens still.
Flags: needinfo?(yzenevich)
Flags: needinfo?(mhenretty)
Keywords: qawanted
Tried reproducing this a couple of times but it seems to work fine for me (files downloaded and shown in Settings > Downloads just fine)
Flags: needinfo?(yzenevich)
[Blocking Requested - why for this release]:

Let's see if this still reproduces and make a blocking call.
blocking-b2g: --- → 2.5?
Flags: needinfo?(mhenretty)
Whiteboard: [systemsfe]
(In reply to Yura Zenevich [:yzen] from comment #2)
> Tried reproducing this a couple of times but it seems to work fine for me
> (files downloaded and shown in Settings > Downloads just fine)

I'm still reproducing this ... It's likely that the problem is in the data of my profile?
Fabrice, you worked on the Gecko side, do you think it could be a symptom that my device picked up some bug at some point and I have bogus data that is making Gaia side to choke?
Flags: needinfo?(fabrice)
(In reply to Alexandre LISSY :gerard-majax from comment #5)
> Fabrice, you worked on the Gecko side, do you think it could be a symptom
> that my device picked up some bug at some point and I have bogus data that
> is making Gaia side to choke?

That's likely, yes. But we should recover gracefully.
Flags: needinfo?(fabrice)
I wasn't able to reproduce this issue on the latest Flame and Aries builds.  Downloads showed up in the download list as expected.  Leaving the tag for someone else to try.

Environmental Variables:
Device: Flame 2.5
BuildID: 20150806103226
Gaia: 497fe3f938722b0aa49c93f975fad5d9ed3b0a82
Gecko: 2fad09a0540d
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Environmental Variables:
Device: Aries 2.5
BuildID: 20150806201613
Gaia: 497fe3f938722b0aa49c93f975fad5d9ed3b0a82
Gecko: 9bdbdad287e0
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
Flags: needinfo?(ktucker)
I was also not able to reproduce on Aries. Downloaded various files like pdf, jpg, png, gif... all appeared in my Settings > Downloads list. Removing qawanted due to inability to reproduce on 3 different people.

Device: Aries 2.5
BuildID: 20150806111958
Gaia: 497fe3f938722b0aa49c93f975fad5d9ed3b0a82
Gecko: 22476236b3e1
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
QA Whiteboard: [QAnalyst-Triage?]
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Lets re-open if it happens again.
Status: NEW → RESOLVED
blocking-b2g: 2.5? → 2.5+
Closed: 4 years ago
Resolution: --- → WORKSFORME
Can someone from Settings have a look at this? QA is not able to reproduce because of comment 5 and  comment 6
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Component: Gaia::Settings → Gaia::System::Download
Maybe the root cause is same. When the Download process is not launched or it is killed, the downloaded files will not appear or be deleted on the Settings-->Dowload page.
See Also: → 1194127
(In reply to Norry.L.F from comment #11)
> Maybe the root cause is same. When the Download process is not launched or
> it is killed, the downloaded files will not appear or be deleted on the
> Settings-->Dowload page.

I doubt. Can't someone just have a look at the code that is triggering the TypeError mentionned in comment 0 ?!
1194127 is unrelated to this bug. The particular typeerror at comment 0 of this bug is not in the log of bug 1194127. We were able to repro 1194127 but not this bug. I'll be working on getting the window for 1194127.
See Also: 1194127
Tim, can you help me or find someone who can help me? I can provide the parts of my profile that are needed in private if anyone can tell me what part is needed :)
Flags: needinfo?(timdream)
I am not unfamiliar with this feature. Sorry about that.
Flags: needinfo?(timdream)
Blocks: 1194642
Even with WebIDE it's painful to track ... download_formatter is loaded lazily so I cannot put a breakpoint ...
triggered from:

08-19 13:31:13.610  8919  8919 D Settings: Content JS DEBUG: FROM:DownloadFormatter.getFileName@app://settings.gaiamobile.org/shared/js/download/download_formatter.js:56:32
08-19 13:31:13.610  8919  8919 D Settings: DownloadFormatter.getUUID@app://settings.gaiamobile.org/shared/js/download/download_formatter.js:86:29
08-19 13:31:13.610  8919  8919 D Settings: getDownloadId@app://settings.gaiamobile.org/js/downloads/download_api_manager.js:70:12
08-19 13:31:13.610  8919  8919 D Settings: _setDownload@app://settings.gaiamobile.org/js/downloads/download_api_manager.js:20:20
08-19 13:31:13.610  8919  8919 D Settings: _appendDownloadsToCache@app://settings.gaiamobile.org/js/downloads/download_api_manager.js:15:7
08-19 13:31:13.610  8919  8919 D Settings: DownloadApiManager.getDownloads/</request.onsuccess@app://settings.gaiamobile.org/js/downloads/download_api_manager.js:182:13
08-19 13:31:13.610  8919  8919 D Settings: Request/this.done/<@app://settings.gaiamobile.org/shared/js/download/download_store.js:92:1
Content of the download variable passed to this formatter stuff is:
> 08-19 13:31:13.610  8919  8919 D Settings: Content JS DEBUG: download:{"byTimestamp":[]}
dumping "downloads" from apps/settings/js/downloads/download_api_manager.js:
> 08-19 13:39:31.430  9081  9081 D Settings: Content JS DEBUG: downloads === [{"byTimestamp":[]},{"url":"http://autourduciel.blog.lemonde.fr/files/2015/03/20150320-388-ECLIPSE-CANNAT-1500.jpg","path":"/storage/sdcard1/downloads/20150320-388-ECLIPSE-CANNAT-1500.jpg.jpeg","totalBytes":962763,"contentType":"image/jpeg","startTime":"2015-03-21T20:51:12.912Z","state":"succeeded","storageName":"sdcard1","storagePath":"downloads/20150320-388-ECLIPSE-CANNAT-1500.jpg.jpeg","finalizeTime":"2015-03-21T20:51:13.295Z","id":3},{"url":"https://www.cgrcinemas.fr/tours/reserver/Billet/2015-03/2015-03-24/0010386806.pdf","path":"/storage/sdcard1/downloads/0010386806.pdf","totalBytes":164576,"contentType":"application/pdf","startTime":"2015-03-24T16:40:07.875Z","state":"succeeded","storageName":"sdcard1","storagePath":"downloads/0010386806.pdf","finalizeTime":"2015-03-24T16:40:08.282Z","id":4},{"url":"","path":"/storage/sdcard1/0010386806.pdf","totalBytes":164576,"contentType":"application/pdf","startTime":"2015-03-24T17:10:28.063Z","state":"succeeded","storageName":"sdca
The bogus "{"byTimestamp":[]}" seems to be something that has been removed by bug 981938.
Depends on: 981938
Flags: needinfo?(tclancy)
Keywords: dataloss, regression
Looks like https://github.com/mozilla-b2g/gaia/pull/18192 removed the INDEX_ID handling but never sanitized properly the Datastore.

Then the bad entry is used in the Downloads section which chokes because of that.

There are 2 ways of fixing this IMO:
* getting the INDEX_ID, looking at its format, and remove()ing it if it looks like the old index.
* merely filtering it when getting the list. (easiest)
shared/js/download/download_store.js previously contained the line:

> var INDEX_ID = 1;

My code contains:

> const REVISION_KEY = 1;

I guess it was a mistake to re-use the value "1" for a different purpose.

Alexandre, if you replace the above line with 

> const REVISION_KEY = 2;

does it start working again? Perhaps that's the easiest fix.
Flags: needinfo?(tclancy) → needinfo?(lissyx+mozillians)
Sure, keeping the needinfo and I'll check this tomorrow
Ted, this is not working :(
Flags: needinfo?(lissyx+mozillians) → needinfo?(tclancy)
D'oh.

Alexandre, is there any way I can reproduce this for myself? Or would I need to use your phone?
Flags: needinfo?(tclancy) → needinfo?(lissyx+mozillians)
Ted, I think this is not the same store we're talking about.

There is an issue in the data stored in the DataStore, not the data stored in the IDB Store.

Easiest to reproduce is likely to introduce the issue in the first place, something like:

navigator.getDataStores(DATASTORE_NAME).then(function(ds) {
  ds[0].put(1, {"byTimestamp":[]});
});

(maybe you need to use `.add({"byTimestamp":[]}, 1)` if there is nothing in the DS yet, but I'm not sure).

Be careful as this will overwrite the data at your current index 1 :)
Flags: needinfo?(tclancy)
(In reply to Ted Clancy [:tedders1] from comment #25)
> D'oh.
> 
> Alexandre, is there any way I can reproduce this for myself? Or would I need
> to use your phone?

You have the sqlite file, I hope it will help reproduce :)
Flags: needinfo?(lissyx+mozillians)
Priority: -- → P2
So ?
Flags: needinfo?(tclancy)
Assignee: nobody → tclancy
Attachment #8658518 - Flags: review?(felash)
Attachment #8658518 - Flags: review?(dflanagan)
Alexandre, does the attached patch work for you?
Comment on attachment 8658518 [details] [review]
[gaia] tedders1:bug-1180250-fix > mozilla-b2g:master

Downloads working again \o/
Attachment #8658518 - Flags: feedback+
Blocks: 981938
No longer depends on: 981938
Comment on attachment 8658518 [details] [review]
[gaia] tedders1:bug-1180250-fix > mozilla-b2g:master

Hey Fred,

I'm redirecting this review to you as I feel you're more legitimate than me :)
Please look at the bug history to have more information, but otherwise feel free to ping me on IRC to get more context.

Giving a feedback+ but I'd like a unit test for this change...
Attachment #8658518 - Flags: review?(gasolin)
Attachment #8658518 - Flags: review?(felash)
Attachment #8658518 - Flags: feedback+
Comment on attachment 8658518 [details] [review]
[gaia] tedders1:bug-1180250-fix > mozilla-b2g:master

I'm not qualified to review this since I know nothing at all about the download manager or its datastore schema. As module owner for shared/js, I do like to give feedback on additions to shared/js, but I don't feel like I need to review bug fixes to shared code.
Attachment #8658518 - Flags: review?(dflanagan)
Comment on attachment 8658518 [details] [review]
[gaia] tedders1:bug-1180250-fix > mozilla-b2g:master

According to commit history of shared/js/download/donwload_store.js, Aus is more familiar with download manager than me
Attachment #8658518 - Flags: review?(gasolin) → review?(aus)
Comment on attachment 8658518 [details] [review]
[gaia] tedders1:bug-1180250-fix > mozilla-b2g:master

download_store changes are good. :)
Attachment #8658518 - Flags: review?(aus) → review+
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/5d45548bcc274d55dacf796b96a06bf6c9c75418
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S7 (18Sep)
You need to log in before you can comment on or make changes to this bug.