Make the MediaRouterDialogDelegate set initial dialog size in OSX

chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.cc

Index: chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.cc
diff --git a/chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.cc b/chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.cc
index ad69808749ce7dd72bfd8aedcdc801c2c8f82e9b..399856de301c94525ccaf0e7c3607ac5a55d5a86 100644
--- a/chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.cc
+++ b/chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.cc
@@ -77,7 +77,20 @@ class MediaRouterDialogDelegate : public WebDialogDelegate {
     // MediaRouterUI adds its own message handlers.
   }
 
-  void GetDialogSize(gfx::Size* size) const override;
+  void GetDialogSize(gfx::Size* size) const override {
+    DCHECK(size);
+#if defined(OS_MACOSX)
+    // We need to set the initial size to be positive for OSX, otherwise the
+    // dialog may fail to show.

[apacible, 2016/09/06 17:24:53]

Add a comment mentioning this is done because specifically because
WebDialogConstrainedWindowSheet's |size| cannot be empty.

[takumif, 2016/09/06 19:01:37]

Done.

+    if (size->IsEmpty())

[apacible, 2016/08/31 18:19:12]

I would prefer handling everything on the sheets side rather than requiring
future autosizing dialogs to send an arbitrary size for mac as a workaround.

[takumif, 2016/08/31 22:44:30]

I don't think the sheet has enough information to be able to set the initial
window size.

I looked a bit more into where else GetDialogSize() gets called, and it seems
that on other platforms we also expect it to modify |size| when it's empty [1],
in a similar manner as my change in WebDialogConstrainedWindowSheet above. I'm
not seeing any change in behavior on Linux regardless of whether or not we set
the |size| here though.

So perhaps we should set the |size| for OSX for now, and also set for other
platforms later if that's preferable?

[1]
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/constrained_web_...

[apacible, 2016/09/06 17:24:53]

Acknowledged. I don't think having an arbitrary size in updateSheetsPosition()
would work well if the dialog doesn't immediately sized. This is fine, don't set
the size with other platforms if not needed.

+      *size = gfx::Size(kWidth, kMinHeight);
+#endif
+    // GetDialogSize() is called when the dialog is created or the browser
+    // window resizes. We may want to update the maximum height of the dialog
+    // and scale the WebUI to the new height. |size| is not set (except for in
+    // OSX) because the dialog is auto-resizeable.
+    controller_->UpdateMaxDialogSize();
+  }
 
   std::string GetDialogArgs() const override {
     return std::string();
@@ -106,14 +119,6 @@ class MediaRouterDialogDelegate : public WebDialogDelegate {
   DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogDelegate);
 };
 
-void MediaRouterDialogDelegate::GetDialogSize(gfx::Size* size) const {
-  DCHECK(size);
-  // GetDialogSize() is called when the browser window resizes. We may want to
-  // update the maximum height of the dialog and scale the WebUI to the new
-  // height. |size| is not set because the dialog is auto-resizeable.
-  controller_->UpdateMaxDialogSize();
-}
-
 }  // namespace
 
 // static