Make the MediaRouterDialogDelegate set initial dialog size in OSX

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

 // Copyright 2015 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/webui/media_router/media_router_dialog_controller_impl.h"
 
 #include <string>
 #include <utility>
 #include <vector>
 
@@ skipping 59 matching lines @@
 
   GURL GetDialogContentURL() const override {
     return GURL(chrome::kChromeUIMediaRouterURL);
   }
 
   void GetWebUIMessageHandlers(
       std::vector<WebUIMessageHandler*>* handlers) const override {
     // 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();
   }
 
   void OnDialogClosed(const std::string& json_retval) override {
     // We don't delete |this| here because this class is owned
     // by ConstrainedWebDialogDelegate.
     if (action_)
       action_->OnPopupHidden();
   }
 
   void OnCloseContents(WebContents* source, bool* out_close_dialog) override {
     if (out_close_dialog)
       *out_close_dialog = true;
   }
 
   bool ShouldShowDialogTitle() const override {
     return false;
   }
 
  private:
   base::WeakPtr<MediaRouterAction> action_;
   base::WeakPtr<MediaRouterDialogControllerImpl> controller_;
 
   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
 MediaRouterDialogControllerImpl*
 MediaRouterDialogControllerImpl::GetOrCreateForWebContents(
     WebContents* web_contents) {
   DCHECK(web_contents);
   // This call does nothing if the controller already exists.
   MediaRouterDialogControllerImpl::CreateForWebContents(web_contents);
   return MediaRouterDialogControllerImpl::FromWebContents(web_contents);
@@ skipping 192 matching lines @@
           ->GetWeakPtr();
   if (!create_connection_request.get()) {
     media_router_ui->InitWithDefaultMediaSource(delegate);
   } else {
     media_router_ui->InitWithPresentationSessionRequest(
         initiator(), delegate, std::move(create_connection_request));
   }
 }
 
 }  // namespace media_router