Implement online reauth UI for Locked Profiles on Mac

chrome/browser/ui/cocoa/profiles/user_manager_mac.mm

Index: chrome/browser/ui/cocoa/profiles/user_manager_mac.mm
diff --git a/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm b/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm
index 5c8b1ae7c3db6f0d4fb83d4fff9db488fd38ed7f..71d89f9e8ff48e1efa62c2828bd45bf76b0a08ea 100644
--- a/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm
+++ b/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm
@@ -12,6 +12,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/signin/signin_promo.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #import "chrome/browser/ui/cocoa/browser_window_utils.h"
 #include "chrome/browser/ui/cocoa/chrome_event_processing_window.h"
@@ -25,7 +26,6 @@
 #include "ui/events/keycodes/keyboard_codes.h"
 
 namespace {
-

[groby-ooo-7-16, 2015/08/05 21:16:38]

Either keep this blank line or remove the one at the end of the namespace. (I'm
in favor of keeping the blank line)

[anthonyvd, 2015/08/06 19:08:04]

Done.

 // Update the App Controller with a new Profile. Used when a Profile is locked
 // to set the Controller to the Guest profile so the old Profile's bookmarks,
 // etc... cannot be accessed.
@@ -45,6 +45,11 @@ void ChangeAppControllerForProfile(Profile* profile,
 UserManagerMac* instance_ = NULL;  // Weak.
 BOOL instance_under_construction_ = NO;
 
+void CloseInstanceReauthDialog() {
+  DCHECK(instance_);
+  instance_->CloseReauthDialog();
+}
+
 // Custom WebContentsDelegate that allows handling of hotkeys.
 class UserManagerWebContentsDelegate : public content::WebContentsDelegate {
  public:
@@ -75,6 +80,95 @@ class UserManagerWebContentsDelegate : public content::WebContentsDelegate {
   }
 };
 
+class ReauthDialogDelegate : public profiles::ReauthDialogObserver,
+                             public UserManagerWebContentsDelegate {
+ public:
+   ReauthDialogDelegate(content::WebContents* web_contents, std::string email)
+      : profiles::ReauthDialogObserver(web_contents, email) {}
+
+  // profiles::ReauthDialogObserver:
+  void CloseReauthDialog() override {
+    CloseInstanceReauthDialog();
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ReauthDialogDelegate);
+};
+
+@interface ReauthDialogWindowController
+    : NSWindowController <NSWindowDelegate> {
+ @private
+  std::string emailAddress_;
+  scoped_ptr<content::WebContents> webContents_;
+  scoped_ptr<ReauthDialogDelegate> webContentsDelegate_;
+  NSModalSession modalSession_;
+}
+- (void)windowWillClose:(NSNotification*)notification;
+- (id)initWithProfile:(Profile*)profile email:(std::string)email;
+@end
+
+@implementation ReauthDialogWindowController
+- (void)windowWillClose:(NSNotification*)notification {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];

[groby-ooo-7-16, 2015/08/05 21:16:38]

removeObserver should probably be handled in -dealloc

(This is _probably_ safe, but I have no idea if windowWillClose is guaranteed in
all cases, so let's be safe)

[anthonyvd, 2015/08/06 19:08:04]

Done.

+  [NSApp endModalSession:modalSession_];
+}
+
+- (id)initWithProfile:(Profile*)profile email:(std::string)email {
+  emailAddress_ = email;
+  // Center the window on the screen that currently has focus.
+  NSScreen* mainScreen = [NSScreen mainScreen];
+  CGFloat screenHeight = [mainScreen frame].size.height;
+  CGFloat screenWidth = [mainScreen frame].size.width;
+  NSRect contentRect =
+      NSMakeRect((screenWidth - UserManager::kReauthDialogWidth) / 2,
+                 (screenHeight - UserManager::kReauthDialogHeight) / 2,
+                 UserManager::kReauthDialogWidth,
+                 UserManager::kReauthDialogHeight);

[groby-ooo-7-16, 2015/08/05 21:16:38]

You can save yourself quite a few of these lines by just using NSWindow's
-center method and using a contentRect with a 0,0 origin.

[anthonyvd, 2015/08/06 19:08:04]

Done.

+  ChromeEventProcessingWindow* window = [[ChromeEventProcessingWindow alloc]
+      initWithContentRect:contentRect
+                styleMask:NSTitledWindowMask |
+                          NSClosableWindowMask |
+                          NSResizableWindowMask
+                  backing:NSBackingStoreBuffered
+                    defer:NO
+                   screen:mainScreen];
+  [window setTitle:l10n_util::GetNSString(IDS_PRODUCT_NAME)];
+  [window setMinSize:NSMakeSize(UserManager::kReauthDialogWidth,

[groby-ooo-7-16, 2015/08/05 21:16:38]

That's the minimum size for the frame, including window chrome. Are you sure you
don't want

[window setMinSize:[window frame].size]

[anthonyvd, 2015/08/06 19:08:04]

Done.

+                                UserManager::kReauthDialogHeight)];
+
+  if ((self = [super initWithWindow:window])) {
+    // Initialize the web view.
+    webContents_.reset(content::WebContents::Create(
+        content::WebContents::CreateParams(profile)));
+    window.contentView = webContents_->GetNativeView();
+    webContentsDelegate_.reset(
+       new ReauthDialogDelegate(webContents_.get(), emailAddress_));
+    webContents_->SetDelegate(webContentsDelegate_.get());
+    DCHECK(window.contentView);
+
+    [[NSNotificationCenter defaultCenter]
+        addObserver:self
+           selector:@selector(windowWillClose:)
+               name:NSWindowWillCloseNotification
+             object:self.window];
+    [self show];
+  }
+  return self;
+}
+
+- (void)show {
+  GURL url = signin::GetReauthURLWithEmail(emailAddress_);
+  webContents_->GetController().LoadURL(url, content::Referrer(),
+                                        ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                        std::string());
+  modalSession_ = [NSApp beginModalSessionForWindow:[self window]];

[groby-ooo-7-16, 2015/08/05 21:16:38]

IIRC, we usually run modal things as sheets, not as a modal session.
(Specifically, via ConstrainedWindow)

Is there a specific reason why this is introducing an entirely new UI mode?

[anthonyvd, 2015/08/06 19:08:04]

To be honest it's only because I'm very unfamiliar with the way we do things on
mac. Because of this, I based this class on the User Manager itself (which isn't
a ConstrainedWindow) but wanted it to stay on top so that's what I figured
worked.

I'd be happy to change it to use sheets and ConstrainedWindows but I'd need a
bit of guidance w.r.t doing so. I've tried using ShowConstrainedWebDialog:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

as well as creating the ConstrainedWindowMac manually with no success. In both
cases, Chrome crashes when checking (if (delegate_)) in
WebContentsModalDialogManager::ShowDialogWithManager.

Do you have any guidance as to what I'm doing wrong?

[groby-ooo-7-16, 2015/08/06 19:49:25]

First, thank you for willing to make this change, even though it's a large
modification to this CL
Second: Do you have a CL I can take a look at? Also, happy to have a quick VC
chat if you think that'll help

[groby-ooo-7-16, 2015/08/06 19:49:25]

That seems to indicate a NULL WebContentsDialogManager, which is... odd.
General guidance:
If you create ConstrainedWindowMac manually, AutofillDialogCocoa::Show would be
the example. (Mostly because I wrote it, and so am familiar with it :)

You'll need a (subclassed) NSWindowController to drive window behavior and that
allocates a ConstrainedWindowCustomWindowMac, you need a
CustomConstrainedWIndowSheet that's bound to the window controller's window, and
you need a ConstrainedWindowMac that takes the sheet and the web contents. It's
a bit of an elaborate dance. AutofillSignInContainer shows how to load an actual
web page in there.

Assuming you want to avoid that and stick with ShowConstrainedWebDialog, that
does take care of this whole dance. As said above, your crash is likely a bad
manager, which - first guess - is caused by a bad BrowserContext. What are you
using as BrowserContext?

[anthonyvd, 2015/08/06 20:07:19]

I thought so too :)
Thanks a ton for this info, I'll try it out!

The BrowserContext I'm using is the UserManager's web_ui's BrowserContext. If I
read everything correctly, that BrowserContext would actually be the System
Profile (which could be an issue since it's special in many ways). Maybe it
doesn't have a WebContentsDialogManager?

[anthonyvd, 2015/08/06 20:07:19]

I don't have a CL but I'll create one from what you described in your next
comment and send it to you if I'm still having issues. :)

+  [NSApp runModalSession:modalSession_];
+}
+
+- (void)close {
+  [[self window] close];
+}
+@end
+
 // Window controller for the User Manager view.
 @interface UserManagerWindowController : NSWindowController <NSWindowDelegate> {
  @private
@@ -238,7 +332,25 @@ void UserManager::OnUserManagerShown() {
 // static
 void UserManager::ShowReauthDialog(content::BrowserContext* browser_context,
                                    const std::string& email) {
-  // TODO(rogerta): See equivalent views implementation in user_manager_view.cc.
+  DCHECK(instance_);
+  instance_->ShowReauthDialog(browser_context, email);
+}
+
+void UserManagerMac::ShowReauthDialog(content::BrowserContext* browser_context,
+                                      const std::string& email) {
+  if (reauthWindow_) {

[groby-ooo-7-16, 2015/08/05 21:16:38]

No need to check - send to nil always works. 

[anthonyvd, 2015/08/06 19:08:04]

Done.

+    [reauthWindow_ close];
+  }
+
+  reauthWindow_.reset(
+      [[ReauthDialogWindowController alloc]
+          initWithProfile:Profile::FromBrowserContext(browser_context)
+                    email:email]);
+}
+
+void UserManagerMac::CloseReauthDialog() {
+  [reauthWindow_ close];
+  reauthWindow_.reset();
 }
 
 UserManagerMac::UserManagerMac(Profile* profile) {
@@ -270,6 +382,9 @@ void UserManagerMac::LogTimeToOpen() {
 }
 
 void UserManagerMac::WindowWasClosed() {
+  if (reauthWindow_) {
+    CloseReauthDialog();

[groby-ooo-7-16, 2015/08/05 21:16:38]

Just call CloseReauthDialog - it's perfectly fine with a nil reauthWindow_

[anthonyvd, 2015/08/06 19:08:04]

Done.

+  }
   instance_ = NULL;
   delete this;
 }