1036143 - [MozApps] downloadapplied event not fired after pause/resume

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Kevin Grandon :kgrandon]

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): *************************

Comment 1

[Kevin Grandon :kgrandon]

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?

Comment 2

[Kevin Grandon :kgrandon]

I've confirmed that this is being caused by bug 1036143.

Comment 3

[Kevin Grandon :kgrandon]

(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.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Bug 903291 was backed out, but hopefully we can find a way to test for this regression when it eventually re-lands?

Comment 5

[Kevin Grandon :kgrandon]

(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!

Comment 6

[Antonio Manuel Amaya Calvo (:amac)]

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.

Comment 7

[Antonio Manuel Amaya Calvo (:amac)]

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.