Refactored ExtensionUninstallDialog to take a NativeWindow instead of a Browser

chrome/browser/extensions/extension_uninstall_dialog.h

Index: chrome/browser/extensions/extension_uninstall_dialog.h
diff --git a/chrome/browser/extensions/extension_uninstall_dialog.h b/chrome/browser/extensions/extension_uninstall_dialog.h
index 0d2f112a1ad781feb8fb145e165efd98af28205a..3828338037920ef90ed780d050aae8fd22f71cd8 100644
--- a/chrome/browser/extensions/extension_uninstall_dialog.h
+++ b/chrome/browser/extensions/extension_uninstall_dialog.h
@@ -12,6 +12,7 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/native_widget_types.h"
 
 class Browser;
 class Profile;
@@ -43,14 +44,19 @@ class ExtensionUninstallDialog
     virtual ~Delegate() {}
   };
 
-  // Creates a platform specific implementation of ExtensionUninstallDialog.
-  // |profile| and |delegate| can never be NULL.
-  // |browser| can be NULL only for Ash when this is used with the applist
-  // window.
+  // Creates a platform specific implementation of ExtensionUninstallDialog. No
+  // parameters can be NULL.
   static ExtensionUninstallDialog* Create(Profile* profile,
                                           Browser* browser,
                                           Delegate* delegate);
 
+  // Creates a platform specific implementation of ExtensionUninstallDialog as
+  // a modal to the given parent window. If |parent| is NULL, creates a

[not at google - send to devlin, 2014/07/21 20:07:48]

I find this comment a bit confusing, since you say that it's modal to the given
parent window, but parent can be NULL. if parent is NULL does that mean that
it's modal to the entire desktop? does it mean it's not modal at all?

[sashab, 2014/07/22 00:30:43]

Yeah... It's a tradeoff between making another method, and keeping this one with
the nullable parameter. I've changed the comment for now, but I'm happy to make
CreateStandaloneDialog again, which just has the second parameter as NULL.

+  // standalone dialog.
+  static ExtensionUninstallDialog* CreateModal(Profile* profile,
+                                               gfx::NativeWindow parent,
+                                               Delegate* delegate);
+
   virtual ~ExtensionUninstallDialog();
 
   // This is called to verify whether the uninstallation should proceed.
@@ -69,12 +75,15 @@ class ExtensionUninstallDialog
  protected:
   // Constructor used by the derived classes.
   ExtensionUninstallDialog(Profile* profile,
-                           Browser* browser,
+                           gfx::NativeWindow parent,
                            Delegate* delegate);
 
+  // Registers this class as an observer for when the browser closes.
+  virtual void RegisterObserver(Browser* browser);

[not at google - send to devlin, 2014/07/21 20:07:48]

I prefer the Browser-in-constructor approach to needing this. The inheritance
and construction behaviour is pretty convoluted already.

[sashab, 2014/07/22 00:30:43]

Agreed. In that case, I also moved the GetParent() logic into the
ExtensionUninstallDialog constructor, but now the constructor is more
complicated. It's a tough call...

+
   Profile* const profile_;
 
-  Browser* browser_;
+  gfx::NativeWindow parent_;
 
   // The delegate we will call Accepted/Canceled on after confirmation dialog.
   Delegate* delegate_;