MacViews: Unify web contents modal dialog types

chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_cocoa.mm

Index: chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_cocoa.mm b/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_cocoa.mm
index 5d025f6c8f6e5d865c7591cc6d5070712107f76d..22e12543a832ae36f9b32525eae8ebb1b4baaef8 100644
--- a/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_cocoa.mm
+++ b/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_cocoa.mm
@@ -4,10 +4,12 @@
 
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 
+#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_sheet.h"
 #include "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.h"
 #include "components/web_modal/single_web_contents_dialog_manager.h"
 
 using web_modal::NativeWebContentsModalDialog;
+using web_modal::SingleWebContentsDialogManagerDelegate;
 
 namespace {
 
@@ -15,29 +17,35 @@ class NativeWebContentsModalDialogManagerCocoa
     : public web_modal::SingleWebContentsDialogManager {
  public:
   NativeWebContentsModalDialogManagerCocoa(
-      NativeWebContentsModalDialog dialog)
-      : dialog_(dialog) {
+      NativeWebContentsModalDialog dialog,
+      SingleWebContentsDialogManagerDelegate* native_delegate)
+      : native_delegate_(native_delegate),
+        dialog_(dialog),
+        sheet_([[CustomConstrainedWindowSheet alloc]
+            initWithCustomWindow:dialog]) {}
+
+  ~NativeWebContentsModalDialogManagerCocoa() override {
+    Close();
   }
 
-  ~NativeWebContentsModalDialogManagerCocoa() override {}
-
-  // SingleWebContentsDialogManager overrides
+  // SingleWebContentsDialogManager overrides.
   void Show() override {
-    GetConstrainedWindowMac(dialog())->ShowWebContentsModalDialog();
+    window_.reset(new ConstrainedWindowMac(

[Mike Wittman, 2015/02/25 20:40:53]

I think this preserves the existing creation/show/hide behavior so should be OK
for now.

If you're moving towards a Mac implementation more in line with the spirit of
the desired interfaces, you may want/need to address https://crbug.com/264032 at
some point. This would use the cross-platform show/hide logic in the WCMDM for
Mac. Without that change it may be difficult to reason about the show/hide
behavior due to the two different implementations.

+        nullptr, native_delegate_->GetWebContents(), sheet_));
   }
 
   void Hide() override {}
 
   void Close() override {
-    GetConstrainedWindowMac(dialog())->CloseWebContentsModalDialog();
+    window_->CloseWebContentsModalDialog();

[Mike Wittman, 2015/02/25 20:40:53]

Add null check. window_ can be null here if the dialog is created but not shown
before being destroyed. This occurs e.g. when another WCMD is already being
displayed when this one is created, then the tab is closed.

   }
 
   void Focus() override {
-    GetConstrainedWindowMac(dialog())->FocusWebContentsModalDialog();
+    window_->FocusWebContentsModalDialog();

[Mike Wittman, 2015/02/25 20:40:53]

DCHECK(window_.get())

This shouldn't get called unless this window has been shown.

   }
 
   void Pulse() override {
-    GetConstrainedWindowMac(dialog())->PulseWebContentsModalDialog();
+    window_->PulseWebContentsModalDialog();

[Mike Wittman, 2015/02/25 20:40:53]

Ditto.

   }
 
   void HostChanged(web_modal::WebContentsModalDialogHost* new_host) override {}
@@ -45,14 +53,10 @@ class NativeWebContentsModalDialogManagerCocoa
   NativeWebContentsModalDialog dialog() override { return dialog_; }
 
  private:
-  static ConstrainedWindowMac* GetConstrainedWindowMac(
-      NativeWebContentsModalDialog dialog) {
-    return static_cast<ConstrainedWindowMac*>(dialog);
-  }
-
-  // In mac this is a pointer to a ConstrainedWindowMac.
-  // TODO(gbillock): Replace this casting system with a more typesafe call path.
+  SingleWebContentsDialogManagerDelegate* native_delegate_;

[tapted, 2015/02/25 06:23:28]

Lifetime comment?

   NativeWebContentsModalDialog dialog_;
+  base::scoped_nsobject<CustomConstrainedWindowSheet> sheet_;
+  scoped_ptr<ConstrainedWindowMac> window_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeWebContentsModalDialogManagerCocoa);
 };
@@ -65,7 +69,7 @@ SingleWebContentsDialogManager*
     WebContentsModalDialogManager::CreateNativeWebModalManager(
         NativeWebContentsModalDialog dialog,
         SingleWebContentsDialogManagerDelegate* native_delegate) {
-  return new NativeWebContentsModalDialogManagerCocoa(dialog);
+  return new NativeWebContentsModalDialogManagerCocoa(dialog, native_delegate);
 }
 
 }  // namespace web_modal