821523 - During a system update (stalled or not) forcing an update check from the settings app the status gets stuck at "Checking for updates..."

Status: RESOLVED DUPLICATE of bug 831701

(Firefox OS Graveyard :: General, defect, P4)

Description

[Daniel Holbert [:dholbert]]

This morning, I was on the build ID 20121206062128.  I received an update notification for a ~52 meg update, and tried to update a few times, but the update stalled at some point during the download.  I rebooted my phone and clicked "Check for updates" in Settings, and it just stayed stuck at "Checking for updates..."

Meanwhile, from watching logcat, I could see that the update actually _had resumed on its own_ (after the reboot), and it ultimately downloaded & extracted itself in the background (so that when I rebooted _again_, I was updated).  That's cool, but it didn't update the UI at all to indicate this -- there was nothing in the notification screen the whole time, and the Settings app stayed at "Checking for updates..."

Filing this bug on the UI inconsistency there.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: logcat after phone-reboot (w/ update resuming in background)

Here's the logcat output, from when I rebooted the phone & tried to check for updates (and the update resumed in the background, with no UI indication at all).

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

Triage: BB+, P3, C3 - confuse users if the update is done or not or are they waiting for new updates

Comment 3

[Julien Wajsberg [:julienw]]

This looks like a platform bug but I'll dig further just to be sure. I'll also check what happens for app updates.

However there is a UX decision to be made here. I think this is wrong that we automatically resume the update. We should rather not resume and show to the user that there is an update available and propose him to update, just as if nothing was done before.

Asking for UX decision here.

Comment 4

[Marshall Culpepper [:marshall_law]]

I guess platform should be re-sending the update-apply-prompt here for a resume?

Comment 5

[Etienne Segonzac (:etienne)]

(In reply to Marshall Culpepper [:marshall_law] from comment #4)
> I guess platform should be re-sending the update-apply-prompt here for a
> resume?

Gaia will be listening :)

Comment 6

[Julien Wajsberg [:julienw]]

I think we should even restart at update-available.

We don't want a download to happen without the user consent.

Gaia is already listening to update-available anyway so this would work without a change in gaia.

Étienne, what do you think ?

Comment 7

[Etienne Segonzac (:etienne)]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> I think we should even restart at update-available.
> 
> We don't want a download to happen without the user consent.
> 
> Gaia is already listening to update-available anyway so this would work
> without a change in gaia.
> 
> Étienne, what do you think ?

Makes sense.

Comment 8

[Julien Wajsberg [:julienw]]

I don't know if this could be fixed in the same bug, but I noticed if I don't start the update download, I still don't get the system update notification when I reboot the phone. However I get it when I force "check for update" from Settings.

Comment 9

[Etienne Segonzac (:etienne)]

Cleaning up this bug to focus on the Settings app part, the rest is tracked in bug 819548.

Comment 10

[Josh Carpenter [:jcarpenter]]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> I think we should even restart at update-available.
> 
> We don't want a download to happen without the user consent.

I agree. If the device restarts we must prompt the user to resume, _never_ automatically resume, given the cost of bandwidth in the target markets.

Comment 11

[Josh Carpenter [:jcarpenter]]

Oops, I see the scope of this bug has changed. Am copying my comments over to #819548.

Comment 12

[Julien Wajsberg [:julienw]]

I've seen when working on Bug 819548 that if we have an activeUpdate (as in UpdateManager.activeUpdate), we never get the message back from the platform.

Worth investigating, but I'm not sure this should be blocking basecamp, so renominating.

Comment 13

[Julien Wajsberg [:julienw]]

Note that in Bug 819548's resolution we should clear UpdateManager.activeUpdate if present at startup.

So when Bug 819548 is fixed, then this should only happen if we force an update when there is already an update happening. That's why I'm renominating.

Comment 14

[Joe Cheng [:jcheng] (please needinfo)]

Triage: BB-. tracking-b2g18+, p4 - with stricter criteria for traige now, move it to BB-

Comment 16

[Daniel Holbert [:dholbert]]

This is still an issue w/ 20130104070203 (the most recent build, up until this afternoon) -- I reproduced this while updating from that build to today's beta release.  (i.e. while today's update was downloading, I hit the "Check now" button again, just for fun, and the text below it said "Checking for updates..." and it stayed stuck there until I rebooted my phone.)

Comment 17

[Julien Wajsberg [:julienw]]

this makes sense, as we didn't fix this bug yet :-)

Comment 18

[Dietrich Ayala (:dietrich)]

Still seeing this. Requesting blocking-b2g:tef+ because update-related failure, and that flow *must* be bulletproof.

Today I checked, and got the notification, but the Settings UI was still stuck on "Checking for updates...".

Comment 19

[Julien Wajsberg [:julienw]]

Dietrich, we currently have Bug 829522 producing what I think you're seeing. I plan to work on this tomorrow.

This is not related to the system update so this is not this bug.

Comment 20

[Julien Wajsberg [:julienw]]

To be clear, this bug is about the following STR:
* check for update (from settings)
* there is a system update
* begin the system update download
* check for update again
=> it never returns

To me, this occurs because of [1] :

2248   _selectAndInstallUpdate: function AUS__selectAndInstallUpdate(updates) {
2249     // Return early if there's an active update. The user is already aware and
2250     // is downloading or performed some user action to prevent notification.
2251     var um = Cc["@mozilla.org/updates/update-manager;1"].
2252              getService(Ci.nsIUpdateManager);
2253     if (um.activeUpdate)
2254       return;

and/or [2] :

55   onCheckComplete: function UCL_onCheckComplete(request, updates, updateCount) {
56     if (Services.um.activeUpdate) {
57       return;
58     }

[1] https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#2253
[2] https://mxr.mozilla.org/mozilla-central/source/b2g/components/UpdatePrompt.js#55

Comment 21

[Julien Wajsberg [:julienw]]

Dietrich, also, Bug 813770 just got fixed yesterday and maybe is not arrived in a build near you yet.

Comment 23

[Julien Wajsberg [:julienw]]

Assigning this to Dave, he may infirm or confirm what I said in Comment 20, and maybe give us an indication of the complexity for fixing this. Then we could maybe renom depending on that.

Comment 24

[Dave Hylands [:dhylands]]

I'll take a look at this. I haven't yet really looked at this aspect of the update.

I can say that with bug 813770 landed you should never see a download resume on its own (as mentioned in comment 1).

Comment 25

[Julien Wajsberg [:julienw]]

Yep Dave, the history of this bug is complicated as it was really several bugs :)

But the last bug remaining (imho) is the one with STR in comment 20.

Comment 26

[Julien Wajsberg [:julienw]]

Dave said that the fix in Bug 831701 will fix these last STR.

Houra !

Comment 27

[Dave Hylands [:dhylands]]

Yes - all paths through onCheckComplete now call setUpdateStatus, so you shouldn't see this any more. Marking as a dup of bug 831701.