Closed Bug 1036143 Opened 5 years ago Closed 5 years ago

[MozApps] downloadapplied event not fired after pause/resume

Categories

(Core Graveyard :: DOM: Apps, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla33

People

(Reporter: kgrandon, Assigned: macajc)

References

Details

(Keywords: regression)

We have a few gaia intermittent failures that are occurring because mozApps is not firing a downloadapplied event after pausing and resuming the app download. I feel that this is potentially a recent regression, but am not 100% sure.

The downloadapplied event fires fine for a normal install, but only messes up when you pause/resume a download.

This issue reproduces on b2g desktop (where our tests are run), but I was not able to reproduce on a device. I'm not sure if these logs are relevant, but I'm seeing them in logcat: 

I/Gecko   (  177): *************************
I/Gecko   (  177): A coding exception was thrown and uncaught in a Task.
I/Gecko   (  177): 
I/Gecko   (  177): Full message: TypeError: undefined has no properties
I/Gecko   (  177): Full stack: this.DOMApplicationRegistry.startDownload<@resource://gre/modules/Webapps.jsm:1411:34
I/Gecko   (  177): TaskImpl_run@resource://gre/modules/Task.jsm:314:40
I/Gecko   (  177): Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:866:23
I/Gecko   (  177): this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7
I/Gecko   (  177): 
I/Gecko   (  177): *************************
I've confirmed that this was working properly on commit 1836f9b10537, but has recently broken.

Marco, Fernando - you guys were the main two who landed patches in Webapps.jsm since then. Any idea if something like this would ring a bell?
Flags: needinfo?(mar.castelluccio)
Flags: needinfo?(ferjmoreno)
Keywords: regression
I've confirmed that this is being caused by bug 1036143.
Blocks: 903291
Flags: needinfo?(mar.castelluccio)
Assignee: nobody → carmen.jimenezcabezas
Flags: needinfo?(ferjmoreno)
(In reply to Kevin Grandon :kgrandon from comment #2)
> I've confirmed that this is being caused by bug 1036143.

Sorry, being caused by bug 903291 I mean :) Copy/paste fail.
Bug 903291 was backed out, but hopefully we can find a way to test for this regression when it eventually re-lands?
Flags: in-testsuite?
Target Milestone: --- → mozilla34
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla34 → mozilla33
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #4)
> Bug 903291 was backed out, but hopefully we can find a way to test for this
> regression when it eventually re-lands?

Hoping to have Gi turned back on which should cover this, and I think they are working on a Mochitest as well. Thanks!
Created bug 1036847 to add mochitests for this.

Note that the behavior described in C0 is *not* a bug. downloadapplied should *not* be called unless you're calling also applyDownload() on the downloadsuccess handler. If you're doing something like:

installPackage()

oninstall() { cancel(); setTimeout(download()) }

then the ondownloadsuccess handler should fire after the install. ondownloadapplied should NOT fire (and if it does, that's a bug!). download() does not install the package, applyDownload does. And you should call applyDownload on the ondownloadsuccess handler.

NI'ng Kevin so he sees this before comitting the Gi, hopefully.
Flags: needinfo?(kgrandon)
Ok, nevermind me. If the app isn't installed previously then applyDownload should be called automatically, so in fact doing install->cancel->download should eventually fire the ondownloadapplied event.
Flags: needinfo?(kgrandon)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.