932843 - Cannot preinstall a packaged app without network connection in a SIM customization variant

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Antonio Manuel Amaya Calvo (:amac)]

+++ This bug was initially created as a clone of Bug #927959 +++

Bug 927959 broke the installation of packaged apps when the device doesn't have network at boot-up time.

STR

1. Follow https://wiki.mozilla.org/B2G/QA/Customizations#Build_Setup to setup your build with a AT&T Mobile US SIM included
2. Complete the FTE

Expected

Packaged apps should be installed and useable.

Actual

Packaged apps get the grey rocket icon, and are on pending state.

I think the problem is on:

    if (Services.io.offline) {
      let onlineWrapper = {

at the start of onInstallSuccessAck. I'll upload a proposed patch in a few minutes (as soon as my compilation ends and I can try it out)

Comment 1

[Antonio Manuel Amaya Calvo (:amac)]

Requesting koi since apps should be installable without having network connection (and this was introduced as a regression from 927959).

Comment 2

[Jason Smith [:jsmith]]

Obvious 1.2 blocker

Comment 3

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: bug932843.patch

Skips the online validation for local installed apps (which actually don't need to be online to install).

Comment 4

[[:fabrice] Fabrice Desré]

You're only talking about the operator variant packages, right? not packaged apps from the store?

Comment 5

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Fabrice Desré [:fabrice] from comment #4)
> You're only talking about the operator variant packages, right? not packaged
> apps from the store?

Yeah. Packaged apps from the store can't be installed when not online anyway, so they're not affected by this. But operator variant packaged apps can (and should) be installed when not online.

Comment 6

[[:fabrice] Fabrice Desré]

Comment on attachment 824700 [details] [diff] [review]
bug932843.patch

Review of attachment 824700 [details] [diff] [review]:
-----------------------------------------------------------------

I don't like that we pass aData.app.localInstallPath here, since you don't use the path. Change that for something more semantic like:
onInstallSuccessAck(aManifestURL, aDontNeedNetwork)

Comment 7

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V2, includes review comments

Changed the parameter passed as requested on the previous review.

Comment 8

[[:fabrice] Fabrice Desré]

Comment on attachment 825667 [details] [diff] [review]
V2, includes review comments

Review of attachment 825667 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nit fixed

::: dom/apps/src/Webapps.jsm
@@ +2159,2 @@
>      // If we are offline, register to run when we'll be online.
> +    if ((Services.io.offline) && (!aDontNeedNetwork)) {

nit: we don't need all these parentheses:
if (Services.io.offline && !aDontNeedNetwork) { ...

Comment 9

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V3, with nits addressed

r=fabrice
Try run at https://tbpl.mozilla.org/?tree=Try&rev=2add0a42d731

Comment 10

[Antonio Manuel Amaya Calvo (:amac)]

Try run looks good (or as good as it's going to get :P), requesting checkin

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/9a123f6acb3d

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9a123f6acb3d

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/5777596fa906