We need to use a watchdog thread with timer to force gecko to power off or reboot the device if the profile synchronizing hangs. Some codes can be refactored and reused from nsAppStartup.cpp.

This should be a basecamp-blocker because it blocks Bug 790527 which is also a basecamp-blocker.

Hi :cjones, I'd like to take this one if you don't mind. I believe most of the codes can be shared from the codes you made in nsAppStartup.cpp. Probabaly, make it a generic service? What do you suggest?

Please do! The best way to implement this may be to - a restart() method to the PowerManagerService - change b2g chrome to use this restart() method instead of nsIAppStartup.quit() - re-use your implementation of profile sync to implement Restart(), but _exit(0) after the process finishes - then move the watchdog code we added to nsAppStartup.cpp out of there and into PowerManagerService, and pass it a parameter letting it to know what to do when the watchdog timer expires (poweroff vs. reboot vs. quit) Does that make sense?

Sounds great! Should be able to come back with the patch tonight or tomorrow. Thanks for the direction!

(In reply to Chris Jones [:cjones] [:warhammer] from comment #3) > Please do! > > The best way to implement this may be to > - a restart() method to the PowerManagerService > - change b2g chrome to use this restart() method instead of > nsIAppStartup.quit() One quick question: is the point you mentioned to replace nsIAppStartup.quit() with PowerManagerService.restart() at http://mxr.mozilla.org/mozilla-central/source/b2g/components/UpdatePrompt.js#190? > - re-use your implementation of profile sync to implement Restart(), but > _exit(0) after the process finishes > - then move the watchdog code we added to nsAppStartup.cpp out of there and > into PowerManagerService, and pass it a parameter letting it to know what to > do when the watchdog timer expires (poweroff vs. reboot vs. quit) > > Does that make sense?

(In reply to Gene Lian [:gene] from comment #5) > (In reply to Chris Jones [:cjones] [:warhammer] from comment #3) > > Please do! > > > > The best way to implement this may be to > > - a restart() method to the PowerManagerService > > - change b2g chrome to use this restart() method instead of > > nsIAppStartup.quit() > > One quick question: is the point you mentioned to replace > nsIAppStartup.quit() with PowerManagerService.restart() at > http://mxr.mozilla.org/mozilla-central/source/b2g/components/UpdatePrompt. > js#190? > That's correct. Then we can keep all this code in one place.

And it would allow the gaia system app to request a b2g restart, which is useful for testing.

Got it! Thanks Chris! :)

Hi :cjones :) I've done a quick WIP patch. Could you please take a look and let me know if I'm on the right direction? I'll refine the codes and test them at the same time. Thank you very much! One question: how to deal the ANDROID platform when calling pmService.restart()?

Comment on attachment 665278 [details] [diff] [review] WIP Patch >diff --git a/b2g/components/UpdatePrompt.js b/b2g/components/UpdatePrompt.js > #ifndef ANDROID Let's use the existing nsIAppService here for non-android (which should be MOZ_WIDGET_GONK) ... > #else and use the nsIPowerManagerService interface for MOZ_WIDGET_GONK. > #endif The rest looks just fine. Nice work!

(In reply to Chris Jones [:cjones] [:warhammer] from comment #10) > Comment on attachment 665278 [details] [diff] [review] > WIP Patch > > >diff --git a/b2g/components/UpdatePrompt.js b/b2g/components/UpdatePrompt.js > > > #ifndef ANDROID > > Let's use the existing nsIAppService here for non-android (which > should be MOZ_WIDGET_GONK) ... > Sorry, this was unclear. What I meant was, this ifdef should really be #ifndef MOZ_WIDGET_GONK

Hi Cjones, The formal patch is done. One noticeable change is we need to carefully release the parameter pointer passed to pthread. Since the QuitHard() is considered, it's kind of hard to use the auto pointers; instead, we have to release it manually. Not sure if we could have other fancy ways to do that or it's just OK to you. ;) Btw, to test the performance I added some dummy codes like |while(1) {};| to simulate the hanging behaviors when shutting down. The watchdog can terminate everything as expected if timeout. Thanks for your review again! :)

Comment on attachment 666503 [details] [diff] [review] Patch >diff --git a/dom/power/PowerManagerService.cpp b/dom/power/PowerManagerService.cpp > NS_IMETHODIMP >+PowerManagerService::Restart() >+{ >+ StartForceQuitWatchdog(eHalShutdownMode_Restart, mWatchdogTimeoutSecs); >+ // To synchronize any unsaved user data before restarting. >+ SyncProfile(); >+ _exit(0); Leave a comment here that this implementation is currently gonk-specific, because it relies on the gonk init process to restart b2g. Let's file a followup bug on fixing this and add a FIXME/bug XXXXXX comment here. >diff --git a/dom/power/nsIPowerManagerService.idl b/dom/power/nsIPowerManagerService.idl > void powerOff(); > void reboot(); >+ void restart(); Document here that this interface restarts the Gecko processes, as opposed to restarting the machine. Looks good! r=me with those changes.

Addressing comment #13 and r=cjones. Thanks for the reviews!

Try: https://tbpl.mozilla.org/?tree=Try&rev=91d10ff414c2