Enable AutoResize for Constrained Web Dialogs

chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc

 // Copyright (c) 2012 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/constrained_web_dialog_delegate_base.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/browser_finder.h"
 #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/popup_manager.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"
 #include "ui/web_dialogs/web_dialog_ui.h"
 
 namespace {
 
+class InitiatorWebContentsObserver

[msw, 2014/12/09 19:51:50]

Sorry, I must be crazy, but what code actually employs observation of the web
contents? As far as I can tell, holding a pointer to the initiator_web_contents
would be sufficient and no actual WebContentsObserver is required.

[apacible, 2014/12/10 18:39:28]

I'm currently using the InitiatorWebContentsObserver in 3 checks to see if the
web contents still exists.

Originally, I just held a pointer to the initiator_web_contents_. From digging
into the lifetime of the initiator_web_contents_ (per sky's earlier comment), I
found that the web contents can potentially be destroyed before a call to use
the web contents is made (e.g. during teardown with pending resizing call).
That's why I changed the pointer to the use of an observer.

[msw, 2014/12/11 01:13:57]

Please add an explanatory comment, otherwise it seems like an oversight.

[apacible, 2014/12/11 03:49:24]

Added in patch 11.

+    : public content::WebContentsObserver {
+  public:
+   explicit InitiatorWebContentsObserver(content::WebContents* web_contents)
+      : content::WebContentsObserver(web_contents) {
+   }
+
+  private:
+    DISALLOW_COPY_AND_ASSIGN(InitiatorWebContentsObserver);
+};
+
 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),
+        initiator_observer_(observer),
         web_view_(web_view) {
   }
   ~WebDialogWebContentsDelegateViews() override {}
 
   // ui::WebDialogWebContentsDelegate:
   void WebContentsFocused(content::WebContents* contents) override {
     // Ensure the WebView is focused when its WebContents is focused.
     web_view_->RequestFocus();
   }
   void HandleKeyboardEvent(
       content::WebContents* source,
       const content::NativeWebKeyboardEvent& event) override {
     // Forward shortcut keys in dialog to our initiator's delegate.
     // http://crbug.com/104586
     // Disabled on Mac due to http://crbug.com/112173
 #if !defined(OS_MACOSX)
-    auto delegate = initiator_->GetDelegate();
+    if (!initiator_observer_->web_contents())
+      return;
+
+    auto delegate = initiator_observer_->web_contents()->GetDelegate();
     if (!delegate)
       return;
-    delegate->HandleKeyboardEvent(initiator_, event);
+    delegate->HandleKeyboardEvent(initiator_observer_->web_contents(), event);
 #endif
   }
 
+  void ResizeDueToAutoResize(content::WebContents* source,
+                             const gfx::Size& preferred_size) override {
+    if (!initiator_observer_->web_contents())

[msw, 2014/12/09 19:51:50]

Should this check that the source matches the initiator?

[apacible, 2014/12/10 18:39:28]

The source and initiator WebContents are not the same WebContents. The source
WebContents is the same thing as the ConstrainedWebDialogDelegateViewView's
GetWebContents() and set in ConstrainedWebDialogDelegateBase as a new web
contents.

The initiator web contents is the web contents passed into the
ConstrainedWebDialogDelegateViewViews constructor.

[msw, 2014/12/11 01:13:57]

Sorry, should this check that the |source| WebContents matches the dialog's
WebContents?

[apacible, 2014/12/11 03:49:24]

Ah, yes. Added.

+      return;
+
+    // Sets WebView's preferred size based on auto-resized contents.
+    web_view_->SetPreferredSize(preferred_size);
+
+    constrained_window::UpdateWebContentsModalDialogPosition(
+        web_view_->GetWidget(),
+        web_modal::WebContentsModalDialogManager::FromWebContents(
+            initiator_observer_->web_contents())->delegate()->
+                GetWebContentsModalDialogHost());
+  }
+
  private:
-  content::WebContents* initiator_;
+  InitiatorWebContentsObserver* const initiator_observer_;
   views::WebView* web_view_;
 
   DISALLOW_COPY_AND_ASSIGN(WebDialogWebContentsDelegateViews);
 };
 
 class ConstrainedWebDialogDelegateViews
     : public ConstrainedWebDialogDelegateBase {
  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 {}
 
   // ui::WebDialogWebContentsDelegate:
   void CloseContents(content::WebContents* source) override {
     view_->GetWidget()->Close();
   }
 
   // contents::WebContentsDelegate:
@@ skipping 19 matching lines @@
 };
 
 class ConstrainedWebDialogDelegateViewViews
     : public views::WebView,
       public ConstrainedWebDialogDelegate,
       public views::WidgetDelegate {
  public:
   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),
+        initiator_observer_(web_contents),
         impl_(new ConstrainedWebDialogDelegateViews(browser_context, delegate,
-                                                    web_contents, this)) {
+                                                    &initiator_observer_,
+                                                    this)),
+        min_size_(min_size),
+        max_size_(max_size) {
     SetWebContents(GetWebContents());
     AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
   }
   ~ConstrainedWebDialogDelegateViewViews() override {}
 
   // ConstrainedWebDialogDelegate:
   const ui::WebDialogDelegate* GetWebDialogDelegate() const override {
     return impl_->GetWebDialogDelegate();
   }
   ui::WebDialogDelegate* GetWebDialogDelegate() override {
@@ skipping 37 matching lines @@
 
   // views::WebView:
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override {
     // Pressing ESC closes the dialog.
     DCHECK_EQ(ui::VKEY_ESCAPE, accelerator.key_code());
     GetWidget()->Close();
     return true;
   }
   gfx::Size GetPreferredSize() const override {
     gfx::Size size;
-    if (!impl_->closed_via_webui())
-      GetWebDialogDelegate()->GetDialogSize(&size);
+    if (!impl_->closed_via_webui()) {
+      // Size is set here if the dialog has been auto-resized in

[msw, 2014/12/09 19:51:50]

nit: "The size"

[apacible, 2014/12/10 18:39:28]

Done.

+      // WebDialogWebContentsDelegateViews's ResizeDueToAutoResize.
+      size = WebView::GetPreferredSize();
+      if (size.IsEmpty()) {
+        // Size set here if dialog has not been auto-resized or

[msw, 2014/12/09 19:51:50]

nit: "The size is set" and "if the dialog"

[apacible, 2014/12/10 18:39:28]

Done.

+        // auto-resizable is not enabled.
+        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 (initiator_observer_.web_contents()) {
+        web_modal::PopupManager* popup_manager =
+            web_modal::PopupManager::FromWebContents(
+                initiator_observer_.web_contents());
+        popup_manager->ShowModalDialog(GetWidget()->GetNativeView(),
+                                       initiator_observer_.web_contents());
+      }
+    }
   }
 
  private:
+  void EnableAutoResize() {
+    content::RenderViewHost* render_view_host =
+        GetWebContents()->GetRenderViewHost();
+    render_view_host->EnableAutoResize(min_size_, max_size_);
+  }
+
+  InitiatorWebContentsObserver initiator_observer_;
+
   scoped_ptr<ConstrainedWebDialogDelegateViews> impl_;
 
+  // Minimum and maximum sizes to determine dialog bounds if

[msw, 2014/12/09 19:51:50]

nit: "bounds for auto-resizing." for a one-liner.

[apacible, 2014/12/10 18:39:28]

Done.

+  // auto-resizing is enabled.
+  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;
+}