My update froze, so i restarted my device. fortunately, i can tell that the update is resuming via logcat. however, there's no indication in the UI that update is resuming. instead, you get a "Checking for updates.." logo, and nothing in notifications appear. Logcat of resuming download: 12-07 10:54:41.400: E/GeckoConsole(108): AUS:SVC Downloader:onProgress - progress: 9500702/48119660 12-07 10:54:41.981: I/Gecko(108): *** AUS:SVC Downloader:onProgress - progress: 9511694/48119660 12-07 10:54:41.981: E/GeckoConsole(108): AUS:SVC Downloader:onProgress - progress: 9511694/48119660 12-07 10:54:42.581: I/Gecko(108): *** AUS:SVC Downloader:onProgress - progress: 9529556/48119660 12-07 10:54:42.581: E/GeckoConsole(108): AUS:SVC Downloader:onProgress - progress: 9529556/48119660 12-07 10:54:43.212: I/Gecko(108): *** AUS:SVC Downloader:onProgress - progress: 9550166/48119660 12-07 10:54:43.212: E/GeckoConsole(108): AUS:SVC Downloader:onProgress - progress: 9550166/48119660 12-07 10:54:50.529: I/Gecko(108): *** AUS:SVC Downloader:onProgress - progress: 9561158/48119660 12-07 10:54:50.529: E/GeckoConsole(108): AUS:SVC Downloader:onProgress - progress: 9561158/48119660 12-07 10:55:25.653: I/Gecko(108): *** AUS:SVC Downloader:onProgress - progress: 9562532/48119660 12-07 10:55:25.653: E/GeckoConsole(108): AUS:SVC Downloader:onProgress - progress: 9562532/48119660 12-07 10:55:26.294: I/Gecko(108): *** AUS:SVC Downloader:onProgress - progress: 9563906/48119660 12-07 10:55:26.294: E/GeckoConsole(108): AUS:SVC Downloader:onProgress - progress: 9563906/48119660 See screenshots, no progress UI. REpro: 1) install 12-3 beta build 2) check for updates , and find a newer build available. 3) apply the update 4) if the update stalls, reboot the phone 5) after reconnecting, verify the update is still resuming (via logcat), but nothing on the UI indicates it so. Expected: - UI shows update resumes Actual: - update is still in progress (via logcat) ,but UI says checking... and the notification is empty.

BB+, P3 - OTA is an imporant feature. It needs to be stable.

This sounds eerily similar to Bug 821523. Tony can you check that out and mark this as a duplicate if it matches your testcase?

(In reply to Marshall Culpepper [:marshall_law] from comment #3) > This sounds eerily similar to Bug 821523. Tony can you check that out and > mark this as a duplicate if it matches your testcase? Did some cleanup. Bug 821523 refers to a piece of UI in the Settings app (updated by gecko.updateStatus mozSetting), this bug refers to a piece of UI in the System app (the fake notification in the tray updated through mozChromeEvents).

Per the thread with Julie in bug 821523, I agree that if the device restarts we _must_ prompt the user to resume, and never resume, given the cost of bandwidth in the target markets.

Per the thread with Julie in bug 821523, I agree that if the device restarts we must prompt the user to manually resume the interrupted update. Automatically resuming is never acceptable, owing to target market costs of bandwidth.

So in Bug 821523 comment 6 I said that we need some platform work, and the Gaia part will follow without a change. Agreed ?

I'll try do produce a first patch.

First try. The only thing I'm not ok with this is that I think we have to force an update to get the notification, or wait for whatever delay is configured in the settings. Any idea on how a notification could be shown as soon as gaia starts up ?

Comment on attachment 694925 [details] [diff] [review] patch v1 Ok, I have some idea and I'll try them later today (I'm traveling today, will work on this after). So removing the review request.

requesting review from Dave as he's working on this stuff for Bug 785124. After trying various things and with limited success, I decided to keep it simple, so it's my first patch except I added the function names in the LOG. I think that we should have a patch in Gaia so that Gaia's Update Manager would know that an update (either system or app) was not finished, and would check for an update asap after the startup.

What I tried instead of only setting um.activeUpdate to null: * in UpdateService, call |this._displayPrompt| * in B2G's UpdatePrompt, in the method "showUpdateAvailable", instead of calling "downloadUpdate" when sending an event fails (which it does at startup), I wanted to set the setting "gaia.system.checkForUpdates" to true, so that it forces an update check as soon as gaia is ready. I stopped here because I realized that Gecko doesn't use SettingsManager for now, and I thought that Gaia could manage to keep the information that the update hasn't finished.

Comment on attachment 695326 [details] [diff] [review] patch v2 Review of attachment 695326 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure that I'm the best person to do this review, as I'm not fully aware of of the consequeces of even this simple change. ::: toolkit/mozapps/update/nsUpdateService.js @@ +1616,3 @@ > if (status == STATE_DOWNLOADING) { > LOG("UpdateService:_postUpdateProcessing - patch found in downloading " + > "state"); Patches should have 8 lines of context (so use the -U8 option if you're using git to create the patch). @@ +1616,5 @@ > if (status == STATE_DOWNLOADING) { > LOG("UpdateService:_postUpdateProcessing - patch found in downloading " + > "state"); > +#ifdef MOZ_WIDGET_GONK > + LOG("UpdateService:_postUpdateProcessing - We're in Gonk, returning."); To me, the really important thing that you haven't mentioned is that we need to wait for user input before resuming the download. I would fix the LOG statement and the comment below. It's not that b2g is not ready yet, it's that we want explicit user interaction before continuing. @@ +1619,5 @@ > +#ifdef MOZ_WIDGET_GONK > + LOG("UpdateService:_postUpdateProcessing - We're in Gonk, returning."); > + // we can't call _selectAndInstallUpdate yet because b2g is not ready yet > + // and that would trigger the download. > + um.activeUpdate = null; Should we also call cleanupActiveUpdate? @@ +1620,5 @@ > + LOG("UpdateService:_postUpdateProcessing - We're in Gonk, returning."); > + // we can't call _selectAndInstallUpdate yet because b2g is not ready yet > + // and that would trigger the download. > + um.activeUpdate = null; > + return; Since you're doing a return here, doesn't that imply that the rest of the code (between the #endif and the end of the if statement) will never be executed? Presumably, then you should remove this return, #else the rest of the if statement and use the return at the end of the #if statement to make it a little more obvious what's going on.

(In reply to Dave Hylands [:dhylands] from comment #13) > Comment on attachment 695326 [details] [diff] [review] > patch v2 > > Review of attachment 695326 [details] [diff] [review]: > ----------------------------------------------------------------- > > I'm not sure that I'm the best person to do this review, as I'm not fully > aware of of the consequeces of even this simple change. Who do you think would be a good person ? :marshall ? > ::: toolkit/mozapps/update/nsUpdateService.js > @@ +1616,3 @@ > > if (status == STATE_DOWNLOADING) { > > LOG("UpdateService:_postUpdateProcessing - patch found in downloading " + > > "state"); > > Patches should have 8 lines of context (so use the -U8 option if you're > using git to create the patch). Ok. I used |hg diff| but I will find a way :) > > @@ +1616,5 @@ > > if (status == STATE_DOWNLOADING) { > > LOG("UpdateService:_postUpdateProcessing - patch found in downloading " + > > "state"); > > +#ifdef MOZ_WIDGET_GONK > > + LOG("UpdateService:_postUpdateProcessing - We're in Gonk, returning."); > > To me, the really important thing that you haven't mentioned is that we need > to wait for user input before resuming the download. > > I would fix the LOG statement and the comment below. It's not that b2g is > not ready yet, it's that we want explicit user interaction before continuing. Ok. > @@ +1619,5 @@ > > +#ifdef MOZ_WIDGET_GONK > > + LOG("UpdateService:_postUpdateProcessing - We're in Gonk, returning."); > > + // we can't call _selectAndInstallUpdate yet because b2g is not ready yet > > + // and that would trigger the download. > > + um.activeUpdate = null; > > Should we also call cleanupActiveUpdate? Nope because that would prevent us from resuming the download. Calling cleanupActiveUpdate works but it makes the download restarting from the start. > @@ +1620,5 @@ > > + LOG("UpdateService:_postUpdateProcessing - We're in Gonk, returning."); > > + // we can't call _selectAndInstallUpdate yet because b2g is not ready yet > > + // and that would trigger the download. > > + um.activeUpdate = null; > > + return; > > Since you're doing a return here, doesn't that imply that the rest of the > code (between the #endif and the end of the if statement) will never be > executed? > > Presumably, then you should remove this return, #else the rest of the if > statement and use the return at the end of the #if statement to make it a > little more obvious what's going on. Good idea, thanks.

(In reply to Julien Wajsberg [:julienw] from comment #14) > (In reply to Dave Hylands [:dhylands] from comment #13) > > Comment on attachment 695326 [details] [diff] [review] > > patch v2 > > > > Review of attachment 695326 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > I'm not sure that I'm the best person to do this review, as I'm not fully > > aware of of the consequeces of even this simple change. > > Who do you think would be a good person ? :marshall ? marshall would be a good place to start. If push comes to shove, I can probably do it since it appears to be a B2G only patch. > > ::: toolkit/mozapps/update/nsUpdateService.js > > @@ +1616,3 @@ > > > if (status == STATE_DOWNLOADING) { > > > LOG("UpdateService:_postUpdateProcessing - patch found in downloading " + > > > "state"); > > > > Patches should have 8 lines of context (so use the -U8 option if you're > > using git to create the patch). > > Ok. I used |hg diff| but I will find a way :) See: https://developer.mozilla.org/en-US/docs/Installing_Mercurial#Configuration The unified = 8 line in the [diff] section will tell diff to provide 8 lines of context.

Addressed comments from :dhyllands. Thanks !

(In reply to Julien Wajsberg [:julienw] from comment #12) > * in B2G's UpdatePrompt, in the method "showUpdateAvailable", instead of > calling "downloadUpdate" when sending an event fails (which it does at > startup), I wanted to set the setting "gaia.system.checkForUpdates" to true, > so that it forces an update check as soon as gaia is ready. > > I stopped here because I realized that Gecko doesn't use SettingsManager for > now, and I thought that Gaia could manage to keep the information that the > update hasn't finished. We don't use the settings manager, but we do use the settings service for "gecko.updateStatus". See here: https://mxr.mozilla.org/mozilla-central/source/b2g/components/UpdatePrompt.js#176

Comment on attachment 695779 [details] [diff] [review] patch v3 Review of attachment 695779 [details] [diff] [review]: ----------------------------------------------------------------- This code probably works for stopping the automatic resume, but how is Gaia going to manage to prompt to resume the download? Do you have a gaia patch in progress for that? r- for now until we can make sure the path from Gecko -> Gaia will prompt correctly for download resuming. On a side note: I can see this prompt-for-every-resume being incredibly annoying for a spotty network connection. Do we have any kind of plan to allow these resumes to be auto-accepted? ::: toolkit/mozapps/update/nsUpdateService.js @@ +1622,5 @@ > + "without doing anything."); > + // we can't call _selectAndInstallUpdate yet because b2g is not ready yet > + // and that would trigger the download whereas we want the user consent > + // before downloading anything > + um.activeUpdate = null; One way we might let Gaia know of the update resume is to reuse nsIUpdatePrompt.showUpdateAvailable here..

(In reply to Marshall Culpepper [:marshall_law] from comment #19) > Comment on attachment 695779 [details] [diff] [review] > patch v3 > > Review of attachment 695779 [details] [diff] [review]: > ----------------------------------------------------------------- > > This code probably works for stopping the automatic resume, but how is Gaia > going to manage to prompt to resume the download? Do you have a gaia patch > in progress for that? r- for now until we can make sure the path from Gecko > -> Gaia will prompt correctly for download resuming. This actually works when the user checks for update again, or at the next automatic check (that is daily by default). I assumed it was enough for v1. I can work on something Gaia-side if you think that's necessary. I could try to hack something around the settings service but I'd prefer to do this right and keep this in Gaia. > On a side note: I can see this prompt-for-every-resume being incredibly > annoying for a spotty network connection. Do we have any kind of plan to > allow these resumes to be auto-accepted? I agree but I guess this is not for v1... > ::: toolkit/mozapps/update/nsUpdateService.js > @@ +1622,5 @@ > > + "without doing anything."); > > + // we can't call _selectAndInstallUpdate yet because b2g is not ready yet > > + // and that would trigger the download whereas we want the user consent > > + // before downloading anything > > + um.activeUpdate = null; > > One way we might let Gaia know of the update resume is to reuse > nsIUpdatePrompt.showUpdateAvailable here.. No we can't because Gaia is not ready yet, and then won't get the event, and |sendUpdateEvent| would return false, and the download would be triggered. That's actually what's happening in the current code and what we want to avoid.

Marking blocking bug 813770 which basically does what I was doing here. Therefore in this patch I'll now concentrate on getting an UI ASAP. After a night of thinking ;) I think using the settings service is not so hacky and I'll try something around this today.

Yeah - I think that if we set the setting thhat "Force Update Now" sets once we detect an interrupted download, then that would cause the update available notification to occur fairly soon after booting.

New proposition. This should be applied after fix for Bug 813770. Sorry for the space changes, my vim configuration changes this automatically and I don't know how to remove these from the diff with |hg export|. I tried the cases of rebooting during download and during uncompressing. By rebooting, I mean pulling the battery, in order to fail in a dirty way. I'm still concerned about the last following case : - the user gets the system update notification in Gaia, but he doesn't apply it and reboots the phone -> the notification is not shown unless he force checks or wait for whatever delay is configured (default is 1 day) I don't know how this could be handled in a cleanly way, maybe this should be handled in another bug.

Comment on attachment 697993 [details] [diff] [review] patch v4 Review of attachment 697993 [details] [diff] [review]: ----------------------------------------------------------------- Yeah - that should work.

> Created attachment 697993 [details] [diff] [review] > patch v4 > > New proposition. This should be applied after fix for Bug 813770. Sorry for > the space changes, my vim configuration changes this automatically and I > don't know how to remove these from the diff with |hg export|. > > I tried the cases of rebooting during download and during uncompressing. By > rebooting, I mean pulling the battery, in order to fail in a dirty way. > > I'm still concerned about the last following case : > - the user gets the system update notification in Gaia, but he doesn't apply > it and reboots the phone > -> the notification is not shown unless he force checks or wait for whatever > delay is configured (default is 1 day) We could have the code which posts an update notification set a setting, which gets cleared once the user starts the download. Then the startup code could check that settings and set the same forceCheck flag (in the same place we check the interrupted download). If the phone doesn't reboot, then the extra setting doesn't do anything, but if it reboots, then its the same thing as the user choosing checkForUpdates if they've been notitifed of an update.

Yep this should work, and I'd say that this could even not be b2g-specific. Let's open a new bug for this though.

Filed Bug 827090

Comment on attachment 697993 [details] [diff] [review] patch v4 we don't want to do this because we want to keep this worst case scenario handling.

spoke with Marshall and we'll handle this in gaia instead.

Is there still something specific we want QA to do, re: qawanted added at 12/20?

nope thanks (and sorry)

No problem, just wanted to make sure we weren't leaving something on the floor. Thanks for removing!

New thinking : We'll set a bool value (maybe with asyncStorage if this happens to be already in the system app) when we first get the system update notification, we'll remove it when we apply the update notification, and at init we'll automatically trigger a "check for update" if we happen to have this bool value set to true. This would fix Bug 827090 in the process too. I think michal will try to work on this today.

Taking this as I'm starving for bugs. I might pair with Michal.

Clarifying a bit the STR for this bug: 1) Flash with a "user" build (VARIANT=user in .userconfig) 2) check for updates , and find a newer build available. 3) click in notifications and apply the update 4) When rebooting, the update will continue to download 5) after reconnecting, verify the update is still resuming (via logcat), but nothing on the UI indicates it so. If on step 4, you cleanly reboot via the UI, you get bug 827896 which is different.

bug 827896 is different but could be fixed here ;)

Forgot to mention that we paired with Michal to fix this.

Comment on attachment 700446 [details] [diff] [review] Review pointer to https://github.com/mozilla-b2g/gaia/pull/7504 comments on the pull request

Comment on attachment 700446 [details] [diff] [review] Review pointer to https://github.com/mozilla-b2g/gaia/pull/7504 We adressed most of the comments of the first review. Asking Etienne for a new review.

This one is ready to land.

Comment on attachment 700446 [details] [diff] [review] Review pointer to https://github.com/mozilla-b2g/gaia/pull/7504 Epic follow up Rick :)

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.

Unagi Build ID: 20130313070202 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/e74dafa6b2d9 Gaia: b34e726147f8e671ad8c538b50900ccfbffcb084 Kernel: Dec 5th This bug has been verified fixed. There is no longer hidden update activity after an OTA stall and a phone reboot. The update availability will be checked once the phone is booted and ready and an update will be shown as available in notificaitons for the user to choose what to do.