Enable AutoResize for Constrained Web Dialogs

chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc

Index: chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc
diff --git a/chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc b/chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc
index c98917173b820a6ed5319febb73e00ea237ce188..395c3919b304ac68bb8b94b4736a1d49a891a220 100644
--- a/chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc
+++ b/chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc
@@ -9,10 +9,13 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/webui/chrome_web_contents_handler.h"
 #include "components/constrained_window/constrained_window_views.h"
+#include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/native_web_keyboard_event.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/views/controls/webview/unhandled_keyboard_event_handler.h"
 #include "ui/views/controls/webview/webview.h"
+#include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_delegate.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
@@ -20,15 +23,23 @@
 
 namespace {
 
+class InitiatorWebContentsObserver

[msw, 2014/12/04 19:27:19]

Why do you need to subclass content::WebContentsObserver?

[apacible, 2014/12/09 17:07:58]

The WebContentsObserver constructor is protected. To create a new
WebContentsObserver, I subclassed it so I can call a public constructor.

+    : public content::WebContentsObserver {
+  public:
+   explicit InitiatorWebContentsObserver(content::WebContents* web_contents)
+      : content::WebContentsObserver(web_contents) {
+   }
+};

[msw, 2014/12/04 19:27:19]

nit: DISALLOW_COPY_AND_ASSIGN

[apacible, 2014/12/09 17:07:58]

Done.

+
 class WebDialogWebContentsDelegateViews
     : public ui::WebDialogWebContentsDelegate {
  public:
   WebDialogWebContentsDelegateViews(content::BrowserContext* browser_context,
-                                    content::WebContents* initiator,
+                                    InitiatorWebContentsObserver* observer,
                                     views::WebView* web_view)
       : ui::WebDialogWebContentsDelegate(browser_context,
                                          new ChromeWebContentsHandler()),
-        initiator_(initiator),
+        observer_(observer),
         web_view_(web_view) {
   }
   ~WebDialogWebContentsDelegateViews() override {}
@@ -45,15 +56,32 @@ class WebDialogWebContentsDelegateViews
     // http://crbug.com/104586
     // Disabled on Mac due to http://crbug.com/112173
 #if !defined(OS_MACOSX)
-    auto delegate = initiator_->GetDelegate();
+    if (!observer_->web_contents())
+      return;
+
+    auto delegate = observer_->web_contents()->GetDelegate();
     if (!delegate)
       return;
-    delegate->HandleKeyboardEvent(initiator_, event);
+    delegate->HandleKeyboardEvent(observer_->web_contents(), event);
 #endif
   }
 
+  void ResizeDueToAutoResize(content::WebContents* source,
+                             const gfx::Size& preferred_size) override {
+    if (!observer_->web_contents())
+      return;
+
+    web_view_->SetPreferredSize(preferred_size);
+
+    constrained_window::UpdateWebContentsModalDialogPosition(
+        web_view_->GetWidget(),
+        web_modal::WebContentsModalDialogManager::FromWebContents(
+            observer_->web_contents())->delegate()->
+                GetWebContentsModalDialogHost());
+  }
+
  private:
-  content::WebContents* initiator_;
+  InitiatorWebContentsObserver* const observer_;

[msw, 2014/12/04 19:27:19]

nit: Use a more descriptive name, like |initiator_observer_|.
(ditto for all the argument plumbing for this member)

[apacible, 2014/12/09 17:07:58]

Done.

   views::WebView* web_view_;
 
   DISALLOW_COPY_AND_ASSIGN(WebDialogWebContentsDelegateViews);
@@ -64,10 +92,10 @@ class ConstrainedWebDialogDelegateViews
  public:
   ConstrainedWebDialogDelegateViews(content::BrowserContext* context,
                                     ui::WebDialogDelegate* delegate,
-                                    content::WebContents* web_contents,
+                                    InitiatorWebContentsObserver* observer,
                                     views::WebView* view)
       : ConstrainedWebDialogDelegateBase(context, delegate,
-            new WebDialogWebContentsDelegateViews(context, web_contents, view)),
+            new WebDialogWebContentsDelegateViews(context, observer, view)),
         view_(view) {}
 
   ~ConstrainedWebDialogDelegateViews() override {}
@@ -107,13 +135,18 @@ class ConstrainedWebDialogDelegateViewViews
   ConstrainedWebDialogDelegateViewViews(
       content::BrowserContext* browser_context,
       ui::WebDialogDelegate* delegate,
-      content::WebContents* web_contents)
+      content::WebContents* web_contents,
+      const gfx::Size& min_size,
+      const gfx::Size& max_size)
       : views::WebView(browser_context),
+        observer_(web_contents),
         impl_(new ConstrainedWebDialogDelegateViews(browser_context, delegate,
-                                                    web_contents, this)) {
+                                                    &observer_, this)),
+        min_size_(min_size),
+        max_size_(max_size) {
     SetWebContents(GetWebContents());
     AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
-  }
+   }

[msw, 2014/12/04 19:27:19]

Remove this incorrect indentation change.

[apacible, 2014/12/09 17:07:58]

Done.

   ~ConstrainedWebDialogDelegateViewViews() override {}
 
   // ConstrainedWebDialogDelegate:
@@ -168,30 +201,80 @@ class ConstrainedWebDialogDelegateViewViews
   }
   gfx::Size GetPreferredSize() const override {
     gfx::Size size;
-    if (!impl_->closed_via_webui())
-      GetWebDialogDelegate()->GetDialogSize(&size);
+    if (!impl_->closed_via_webui()) {
+      size = WebView::GetPreferredSize();

[msw, 2014/12/04 19:27:19]

I'm worried that this might be conflating the sizes of the web content and the
dialog hosting the web content, it'd be nice if you added some comments to make
the meaning of all the sizes used in this CL more explicit.

[apacible, 2014/12/09 17:07:58]

Done.

+      if (size.IsEmpty())
+        GetWebDialogDelegate()->GetDialogSize(&size);
+    }
     return size;
   }
   gfx::Size GetMinimumSize() const override {
-    // Return an empty size so that we can be made smaller.
-    return gfx::Size();
+    return min_size_;
+  }
+  gfx::Size GetMaximumSize() const override {
+    return !max_size_.IsEmpty() ? max_size_ : WebView::GetMaximumSize();
+  }
+  void RenderViewCreated(content::RenderViewHost* render_view_host) override {
+    if (!max_size_.IsEmpty())
+      EnableAutoResize();
+  }
+  void RenderViewHostChanged(content::RenderViewHost* old_host,
+                             content::RenderViewHost* new_host) override {
+    if (!max_size_.IsEmpty())
+      EnableAutoResize();
+  }
+  void DocumentOnLoadCompletedInMainFrame() override {
+    if (!max_size_.IsEmpty()) {
+      EnableAutoResize();
+      if (observer_.web_contents())
+          constrained_window::ShowWebModalDialogViews(this,
+              observer_.web_contents());
+    }
   }
 
  private:
+  void EnableAutoResize() {
+    content::RenderViewHost* render_view_host =
+        GetWebContents()->GetRenderViewHost();
+    render_view_host->EnableAutoResize(min_size_, max_size_);
+  }
+
+  InitiatorWebContentsObserver observer_;

[msw, 2014/12/04 19:27:19]

ditto nit: Use a more descriptive name, like |initiator_observer_|.
(ditto for all the argument plumbing for this member)

[apacible, 2014/12/09 17:07:58]

Done.

+
   scoped_ptr<ConstrainedWebDialogDelegateViews> impl_;
 
+  const gfx::Size min_size_;
+  const gfx::Size max_size_;
+
   DISALLOW_COPY_AND_ASSIGN(ConstrainedWebDialogDelegateViewViews);
 };
 
 }  // namespace
 
-ConstrainedWebDialogDelegate* CreateConstrainedWebDialog(
+ConstrainedWebDialogDelegate* ShowConstrainedWebDialog(
     content::BrowserContext* browser_context,
     ui::WebDialogDelegate* delegate,
     content::WebContents* web_contents) {
   ConstrainedWebDialogDelegateViewViews* dialog =
       new ConstrainedWebDialogDelegateViewViews(
-          browser_context, delegate, web_contents);
+          browser_context, delegate, web_contents,
+          gfx::Size(), gfx::Size());
   constrained_window::ShowWebModalDialogViews(dialog, web_contents);
   return dialog;
 }
+
+ConstrainedWebDialogDelegate* CreateConstrainedWebDialogWithAutoResize(
+    content::BrowserContext* browser_context,
+    ui::WebDialogDelegate* delegate,
+    content::WebContents* web_contents,
+    const gfx::Size& min_size,
+    const gfx::Size& max_size) {
+  DCHECK(!min_size.IsEmpty());
+  DCHECK(!max_size.IsEmpty());
+  ConstrainedWebDialogDelegateViewViews* dialog =
+      new ConstrainedWebDialogDelegateViewViews(
+          browser_context, delegate, web_contents,
+          min_size, max_size);
+  constrained_window::CreateWebModalDialogViews(dialog, web_contents);
+  return dialog;
+}