Create the UserManager's Dialog Manager during init.

Create the UserManager's Dialog Manager during init.

BUG=520456
TEST=
1. Launch chrome, Sign in to chrome and  add/import a supervised user.
2. In the signed in user, click on avatar icon in top R.H.S and select 'Exit and childlock' option (Switch person window opens).
3. In Switch person window, double click on locked user (Sign in overlay opens).
4. Click on close button of Sign in overlay and observe.
5. The browser should not crash.

Committed: https://crrev.com/68fc33f97b520ec7dd44a87fb5acc39718172f15
Cr-Commit-Position: refs/heads/master@{#348791}

[anthonyvd, 2015-09-01 17:25:53]

anthonyvd@chromium.org changed reviewers:
+ groby@chromium.org

[anthonyvd, 2015-09-01 17:25:53]

Hi groby@,

Could you please take a quick look at this fix for a crash when showing the
reauth window in the User Manager?

Thanks a lot for your time!

[groby-ooo-7-16, 2015-09-01 18:25:20]

syntactically OK, but would you mind adding a quick paragraph why it fixes the
issue to the description? It's not immediately obvious to me :)

[anthonyvd, 2015-09-01 20:28:48]

On 2015/09/01 18:25:20, groby wrote:
> syntactically OK, but would you mind adding a quick paragraph why it fixes the
> issue to the description? It's not immediately obvious to me :)

On 2015/09/01 18:25:20, groby wrote:
> syntactically OK, but would you mind adding a quick paragraph why it fixes the
> issue to the description? It's not immediately obvious to me :)

Sure! On my local builds, the crash in the bug report is preceded by this stack
trace: 

[5311:1299:0901/125028:FATAL:single_web_contents_dialog_manager_cocoa.mm(80)]
Check failed: !!host_ != !!new_host (1 vs. 1)
0   Chromium Framework                  0x000000010a7df31f
base::debug::StackTrace::StackTrace() + 47
1   Chromium Framework                  0x000000010a7df373
base::debug::StackTrace::StackTrace() + 35
2   Chromium Framework                  0x000000010a8316c3
logging::LogMessage::~LogMessage() + 67
3   Chromium Framework                  0x000000010a830603
logging::LogMessage::~LogMessage() + 35
4   Chromium Framework                  0x000000011223a104
SingleWebContentsDialogManagerCocoa::HostChanged(web_modal::WebContentsModalDialogHost*)
+ 276
5   Chromium Framework                  0x0000000114b260ad
web_modal::WebContentsModalDialogManager::SetDelegate(web_modal::WebContentsModalDialogManagerDelegate*)
+ 237
6   Chromium Framework                  0x00000001124a9f6f
-[UserManagerWindowController showReauthDialogWithProfile:email:] + 271

It looks like it's possible to click on the pod a second time before the
constrained window becomes modal, which would set the delegate again and hit
that CHECK.

The stack trace in the bug report is different (but patchsets #1 and #2 made it
un-reproduceable): the onClose selector is sent to the window after it was
closed. I realized this after sending the review out but the latest patchset
ensures the window is properly closed when the window controller is deallocated
(so it shouldn't be possible to manually close a window attached to a zombie
window controller).

[anthonyvd, 2015-09-10 20:38:59]

On 2015/09/01 20:28:48, anthonyvd wrote:
> On 2015/09/01 18:25:20, groby wrote:
> > syntactically OK, but would you mind adding a quick paragraph why it fixes
the
> > issue to the description? It's not immediately obvious to me :)
> 
> On 2015/09/01 18:25:20, groby wrote:
> > syntactically OK, but would you mind adding a quick paragraph why it fixes
the
> > issue to the description? It's not immediately obvious to me :)
> 
> Sure! On my local builds, the crash in the bug report is preceded by this
stack
> trace: 
> 
> [5311:1299:0901/125028:FATAL:single_web_contents_dialog_manager_cocoa.mm(80)]
> Check failed: !!host_ != !!new_host (1 vs. 1)
> 0   Chromium Framework                  0x000000010a7df31f
> base::debug::StackTrace::StackTrace() + 47
> 1   Chromium Framework                  0x000000010a7df373
> base::debug::StackTrace::StackTrace() + 35
> 2   Chromium Framework                  0x000000010a8316c3
> logging::LogMessage::~LogMessage() + 67
> 3   Chromium Framework                  0x000000010a830603
> logging::LogMessage::~LogMessage() + 35
> 4   Chromium Framework                  0x000000011223a104
>
SingleWebContentsDialogManagerCocoa::HostChanged(web_modal::WebContentsModalDialogHost*)
> + 276
> 5   Chromium Framework                  0x0000000114b260ad
>
web_modal::WebContentsModalDialogManager::SetDelegate(web_modal::WebContentsModalDialogManagerDelegate*)
> + 237
> 6   Chromium Framework                  0x00000001124a9f6f
> -[UserManagerWindowController showReauthDialogWithProfile:email:] + 271
> 
> It looks like it's possible to click on the pod a second time before the
> constrained window becomes modal, which would set the delegate again and hit
> that CHECK.
> 
> The stack trace in the bug report is different (but patchsets #1 and #2 made
it
> un-reproduceable): the onClose selector is sent to the window after it was
> closed. I realized this after sending the review out but the latest patchset
> ensures the window is properly closed when the window controller is
deallocated
> (so it shouldn't be possible to manually close a window attached to a zombie
> window controller).

Hi groby@, did you have a chance to look at the latest patchset? :)

[groby-ooo-7-16, 2015-09-12 00:42:16]

Sorry, was OOO

LGTM w/ nit

https://codereview.chromium.org/1320893003/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/user_manager_mac.mm (right):

https://codereview.chromium.org/1320893003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/user_manager_mac.mm:237: //[closeButton
setTarget:nil];
Remove commented out code

[anthonyvd, 2015-09-14 23:59:19]

https://codereview.chromium.org/1320893003/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/user_manager_mac.mm (right):

https://codereview.chromium.org/1320893003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/user_manager_mac.mm:237: //[closeButton
setTarget:nil];
On 2015/09/12 00:42:16, groby wrote:
> Remove commented out code

oops, missed this. Thanks!

[anthonyvd, 2015-09-14 23:59:22]

The CQ bit was checked by anthonyvd@chromium.org

[anthonyvd, 2015-09-14 23:59:23]

The patchset sent to the CQ was uploaded after l-g-t-m from groby@chromium.org
Link to the patchset: https://codereview.chromium.org/1320893003/#ps60001
(title: "Remove commented code.")

[commit-bot: I haz the power, 2015-09-15 00:01:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1320893003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1320893003/60001

[commit-bot: I haz the power, 2015-09-15 00:38:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-15 00:39:04]

Patchset 4 (id:??) landed as
https://crrev.com/68fc33f97b520ec7dd44a87fb5acc39718172f15
Cr-Commit-Position: refs/heads/master@{#348791}

[commit-bot: I haz the power, 2015-09-23 12:40:18]

Patchset 4 (id:??) landed as
https://crrev.com/68fc33f97b520ec7dd44a87fb5acc39718172f15
Cr-Commit-Position: refs/heads/master@{#348791}