Using native sheet to display modal dialogs for sign in

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h"
 
 #import <Cocoa/Cocoa.h>
 
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 49 matching lines @@
       web_contents_(std::move(web_contents)),
       wait_for_size_(wait_for_size),
       host_web_contents_(host_web_contents),
       window_frame_(frame) {
   if (!wait_for_size_)
     DisplayModal();
 }
 
 SigninViewControllerDelegateMac::~SigninViewControllerDelegateMac() {}
 
-void SigninViewControllerDelegateMac::OnConstrainedWindowClosed(
-    ConstrainedWindowMac* window) {
-  ResetSigninViewControllerDelegate();
-  delete this;
-}
-
 // static
 std::unique_ptr<content::WebContents>
 SigninViewControllerDelegateMac::CreateGaiaWebContents(
     content::WebContentsDelegate* delegate,
     profiles::BubbleViewMode mode,
     Profile* profile,
     signin_metrics::AccessPoint access_point) {
   GURL url =
       signin::GetSigninURLFromBubbleViewMode(profile, mode, access_point);
 
@@ skipping 52 matching lines @@
       ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
 
   NSView* webview = web_contents->GetNativeView();
   [webview
       setFrameSize:NSMakeSize(kModalDialogWidth, kSigninErrorDialogHeight)];
 
   return web_contents;
 }
 
 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) {
   if (wait_for_size_) {
     [window_.get().contentView
         setFrameSize:NSMakeSize(kModalDialogWidth,
                                 height)];
     window_frame_.size = NSMakeSize(kModalDialogWidth, height);
     DisplayModal();
   }
 }
 
 void SigninViewControllerDelegateMac::DisplayModal() {
   window_.reset(
       [[ConstrainedWindowCustomWindow alloc]
           initWithContentRect:window_frame_]);
 
   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(
     content::WebContents* source,
     const content::NativeWebKeyboardEvent& event) {
   int chrome_command_id = [BrowserWindowUtils getCommandId:event];
   bool can_handle_command = [BrowserWindowUtils isTextEditingEvent:event] ||
                             chrome_command_id == IDC_CLOSE_WINDOW ||
                             chrome_command_id == IDC_EXIT;
   if ([BrowserWindowUtils shouldHandleKeyboardEvent:event] &&
@@ skipping 38 matching lines @@
 SigninViewControllerDelegate::CreateSigninErrorDelegate(
     SigninViewController* signin_view_controller,
     Browser* browser) {
   return new SigninViewControllerDelegateMac(
       signin_view_controller,
       SigninViewControllerDelegateMac::CreateSigninErrorWebContents(
           browser->profile()),
       browser->tab_strip_model()->GetActiveWebContents(),
       NSMakeRect(0, 0, kModalDialogWidth, kSigninErrorDialogHeight), true);
 }