828267 - Installing a packaged app, lose connection during download, remove it, install it again - network error doesn't fire and app infinitely spins trying to download resources

Status: VERIFIED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Jason Smith [:jsmith]]

Build: B2G 18 1/9/2013
Device: Unagi

Steps:

1. Start a download of a packaged app that's large (http://mozqa.com/webapi-permissions-tests/ Packaged App Test Case 5)
2. Kill the network connection
3. Remove that app from the homescreen
4. Try installing that app again

Expected:

A network error should immediately be thrown - we don't have a network connection to install the app.

Actual:

We start downloading the app resources even though we don't have a network connection. The app starts spinning forever in a downloading state. No error is fired to the user.

Error Console:

01-09 14:08:22.379: E/GeckoConsole(109): [JavaScript Error: "[Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIHttpChannel.responseStatus]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: resource://gre/modules/Webapps.jsm :: <TOP_LEVEL> :: line 1854"  data: no]" {file: "resource://gre/modules/Webapps.jsm" line: 1854}]

My hunch is that we're blowing up on the etag check.

Comment 1

[Jason Smith [:jsmith]]

Another STR that reproduces this behavior:

1. Install a packaged app
2. Kill the network connection
3. Try restarting the download without a network connection

Comment 2

[[:fabrice] Fabrice Desré]

We fail at https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#1879
I think that just moving the block at https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#1900 before the responseStatus block will fix that. Patch coming asap.

Comment 3

[[:fabrice] Fabrice Desré]

Attachment: patch

Jason verified on a build with this patch that this is fixing the issue.

Comment 4

[Lucas Adamski [:ladamski]]

Comment on attachment 699825 [details] [diff] [review]
patch

JST says r=JST

Comment 5

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4eb969119718

Comment 6

[[:fabrice] Fabrice Desré]

Comment on attachment 699825 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: network errors are not reported properly during package download, leading to a confusing UI.
Testing completed: yes
Risk to taking this patch (and alternatives if risky): no risk
String or UUID changes made by this patch: none

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4eb969119718

Comment 8

[Jason Smith [:jsmith]]

Verified when I tested Fabrice's patch directly.

Ed - Can you land this on b2g18?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/c0b05b8ea81c