Using native sheet to display modal dialogs for sign in

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

Index: chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
diff --git a/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm b/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
index 537b68bf3221a458be51c04d5f92d06083f582f5..70a82b7b574d6f1b9c58e82fdc435d2cb8e612f9 100644
--- a/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
+++ b/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
@@ -67,12 +67,6 @@ SigninViewControllerDelegateMac::SigninViewControllerDelegateMac(
 
 SigninViewControllerDelegateMac::~SigninViewControllerDelegateMac() {}
 
-void SigninViewControllerDelegateMac::OnConstrainedWindowClosed(
-    ConstrainedWindowMac* window) {
-  ResetSigninViewControllerDelegate();
-  delete this;
-}
-
 // static
 std::unique_ptr<content::WebContents>
 SigninViewControllerDelegateMac::CreateGaiaWebContents(
@@ -145,8 +139,12 @@ SigninViewControllerDelegateMac::CreateSigninErrorWebContents(
 }
 
 void SigninViewControllerDelegateMac::PerformClose() {
-  if (constrained_window_.get())
-    constrained_window_->CloseWebContentsModalDialog();
+  if (window_.get()) {
+    [host_web_contents_->GetTopLevelNativeWindow() endSheet:window_];
+    window_.reset(nil);
+    ResetSigninViewControllerDelegate();

[msarda, 2017/01/10 14:16:13]

I think it would be better to use a DeleteSoon here to avoid crashes if the code
calling PerformClose uses the object after this is called.

See:
https://cs.chromium.org/chromium/src/base/sequenced_task_runner.h?rcl=0&l=123
and
https://cs.chromium.org/chromium/src/ash/common/system/tray/system_tray_bubbl...

[jlebel, 2017/01/10 15:43:35]

As we talked offline, this would require to move the destructor method from
private to public. Plus it is better to be more restrictive by adding a comment
saying that as soon as PerformClose() is called, the current instance should be
considered as destroyed.

I've added some comments in signin_view_controller_delegate.h.

 

+    delete this;
+  }
 }
 
 void SigninViewControllerDelegateMac::ResizeNativeView(int height) {
@@ -167,8 +165,8 @@ void SigninViewControllerDelegateMac::DisplayModal() {
   window_.get().contentView = web_contents_->GetNativeView();
   base::scoped_nsobject<CustomConstrainedWindowSheet> sheet(
       [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window_]);
-  constrained_window_ =
-      CreateAndShowWebModalDialogMac(this, host_web_contents_, sheet);
+  [host_web_contents_->GetTopLevelNativeWindow() beginSheet:window_
+                                          completionHandler:nil];
 }
 
 void SigninViewControllerDelegateMac::HandleKeyboardEvent(