Destroy the UserManager after ChromeBrowserMainPartsLinux::PostMainMessageLoopRun

Destroy the UserManager after ChromeBrowserMainPartsLinux::PostMainMessageLoopRun

UserManager is needed during MergeSessionThrottle::ShouldShowMergeSessionPage, which appears sometimes to be triggered during ChromeBrowserMainPartsLinux::PostMainMessageLoopRun.

I observed this during the test TrayAccessibilityTest.ShowMenu, as described in https://crbug.com/243364#c12. However, at most occasions this was masked by ShouldShowMergeSessionPage being actually triggered earlier.

This is restoring the order of destroying UserManager and calling ChromeBrowserMainPartsLinux::PostMainMessageLoopRun to the state before the CL https://chromiumcodereview.appspot.com/15702004/.

BUG=243364
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202617

[vabr (Chromium), 2013-05-28 13:46:42]

Hi Steven,

Could you please take a look?

The fix seems to work.
I'm not sure when is ShouldShowMergeSessionPage supposed to get triggered,
however, so I'm not 100% confident this is the best thing to do.

Thanks,
Vaclav

[Nikita (slow), 2013-05-28 14:03:17]

lgtm

[vabr (Chromium), 2013-05-28 14:19:27]

Thanks, Nikita!

Based on your LGTM and the passing trybots, I'm checking the CQ bit.

Vaclav

[commit-bot: I haz the power, 2013-05-28 14:19:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/15769004/1

[Nikita (slow), 2013-05-28 14:58:03]

thanks for quick fix and investigation!

[stevenjb, 2013-05-28 16:11:53]

Please update the comment, otherwise lgtm.

https://chromiumcodereview.appspot.com/15769004/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://chromiumcodereview.appspot.com/15769004/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:877: // Destroy the
UserManager after ash has been destroyed.
We should update this comment if there is an additional dependency.

[vabr (Chromium), 2013-05-28 16:44:39]

On 2013/05/28 16:11:53, stevenjb (chromium) wrote:
> Please update the comment, otherwise lgtm.
> 
>
https://chromiumcodereview.appspot.com/15769004/diff/1/chrome/browser/chromeo...
> File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):
> 
>
https://chromiumcodereview.appspot.com/15769004/diff/1/chrome/browser/chromeo...
> chrome/browser/chromeos/chrome_browser_main_chromeos.cc:877: // Destroy the
> UserManager after ash has been destroyed.
> We should update this comment if there is an additional dependency.

Good idea, thanks!

I updated he comment and am sending back to CQ.

Vaclav

[commit-bot: I haz the power, 2013-05-28 16:44:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/15769004/16001

[commit-bot: I haz the power, 2013-05-28 18:56:34]

Change committed as 202617