Closed Bug 1027458 Opened 6 years ago Closed 6 years ago

On an individual app .ondownloaderror (after a downloadCancel) emits both DOWNLOAD_CANCELED and NETWORK_ERROR

Categories

(Core Graveyard :: DOM: Apps, defect)

defect
Not set

Tracking

(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
mozilla33
blocking-b2g 2.0+
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: jlal, Assigned: jlal)

References

Details

Attachments

(1 file, 3 obsolete files)

https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#3467 is effectively dead code (unless I am really missing something) at somepoint between v1.2 and now we removed the addition of the flag but this logic is still needed so we can correctly detect when an app download was canceled vs some other network error
Assignee: nobody → jlal
Blocks: 1023950
Comment on attachment 8442652 [details] [diff] [review]
Correctly cleanup after canceling a mozapp download

I came up with this from a combination of reading what the offline (app cache) case does and digging up the past functionality from v1.2.
Attachment #8442652 - Flags: feedback?(fabrice)
Attachment #8442654 - Flags: feedback?(fabrice)
Attachment #8442652 - Attachment is obsolete: true
Attachment #8442652 - Flags: feedback?(fabrice)
Comment on attachment 8442654 [details] [diff] [review]
Correctly cleanup after canceling a mozapp download

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

Hm... so I'm pretty sure we had something similar in the past and ferjm removed it. CC-ing fernando.

::: dom/apps/src/Webapps.jsm
@@ +2779,5 @@
>  
> +      // save the current state of the app to handle cases where we may be
> +      // retrying a past download.
> +      yield DOMApplicationRegistry._saveApps();
> +      yield DOMApplicationRegistry.broadcastMessage("Webapps:UpdateState", {

no yield here.
Attachment #8442654 - Flags: feedback?(fabrice) → feedback+
Flags: needinfo?(ferjmoreno)
QA Whiteboard: [VH-FL-blocking+][VH-FC-blocking+]
More complete try run https://tbpl.mozilla.org/?tree=Try&rev=8e8352871d35 (looks like some of the apps tests do not run on b2g?)
Attachment #8443659 - Flags: review?(fabrice)
Attachment #8442654 - Attachment is obsolete: true
Comment on attachment 8443659 [details] [diff] [review]
Correctly cleanup after canceling a mozapp download

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

r=me with nit fixed.

::: dom/apps/src/Webapps.jsm
@@ +2776,5 @@
>  
>        // initialize the progress to 0 right now
>        oldApp.progress = 0;
>  
> +      // save the current state of the app to handle cases where we may be

nit: Save
Attachment #8443659 - Flags: review?(fabrice) → review+
Flags: needinfo?(ferjmoreno)
Attachment #8443659 - Attachment is obsolete: true
Attachment #8443776 - Flags: review?(fabrice)
Comment on attachment 8443776 [details] [diff] [review]
Correctly cleanup after canceling a mozapp download

carying r+ over from fabrice... this fixes the review comment.
Attachment #8443776 - Flags: review?(fabrice) → review+
Keywords: checkin-needed
Pushed to b2g-inbound (with fix to the commit message with r=fabrice).

https://hg.mozilla.org/integration/b2g-inbound/rev/59fda077a2ab
Status: NEW → ASSIGNED
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/59fda077a2ab
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Blocks: 1025782
Comment on attachment 8443776 [details] [diff] [review]
Correctly cleanup after canceling a mozapp download

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Not able to correctly resume app download in gaia (FxOS)
Testing completed (on m-c, etc.): tests written (live in gaia)
Risk to taking this patch (and alternatives if risky): low risk mostly adding back some removed code
String or IDL/UUID changes made by this patch: none
Attachment #8443776 - Flags: approval-mozilla-aurora?
Blocks a blocker (bug 1025782).
blocking-b2g: --- → 2.0?
blocking-b2g: 2.0? → 2.0+
Attachment #8443776 - Flags: approval-mozilla-aurora?
Sorry, that follow-up was meant for the blocking bug, bug 1023950.
Flags: in-moztrap-
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.