Use a constrained window to display HaTS dialog

Old: https://imgur.com/a/NhcBv4r
New: https://imgur.com/a/ERQPkol

Bug: 1051416
Change-Id: I044442e19af845724aac756f6c5579eb13eb5e53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2093592
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Wei Li <weili@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752305}

chrome/browser/ui/views/chrome_web_dialog_view.cc

@@ -90,33 +90,4 @@ gfx::NativeWindow ShowWebDialogWithParams(
   return window;
 }
 
-gfx::NativeWindow CreateWebDialogWithBounds(gfx::NativeView parent,
-                                            content::BrowserContext* context,
-                                            ui::WebDialogDelegate* delegate,
-                                            const gfx::Rect& bounds,
-                                            bool show) {
-  // Use custom dialog frame instead of platform frame when possible.
-  bool use_dialog_frame = views::DialogDelegate::CanSupportCustomFrame(parent);
-  views::WebDialogView* view = new views::WebDialogView(
-      context, delegate, std::make_unique<ChromeWebContentsHandler>(),
-      use_dialog_frame);
-
-  views::Widget::InitParams params;
-  params.delegate = view;
-  params.bounds = bounds;
-  params.parent = parent;
-  params.child = true;
-  if (use_dialog_frame) {
-    params.opacity = views::Widget::InitParams::WindowOpacity::kTranslucent;
-    params.remove_standard_frame = true;
-#if !defined(OS_MACOSX)
-    // Except on Mac, the bubble frame includes its own shadow; remove any
-    // native shadowing. On Mac, the window server provides the shadow.
-    params.shadow_type = views::Widget::InitParams::ShadowType::kNone;
-#endif
-  }
-
-  return CreateWebDialogWidget(std::move(params), view, show);
-}
-
 }  // namespace chrome

chrome/browser/ui/views/chrome_web_dialog_view.h

@@ -26,17 +26,6 @@ gfx::NativeWindow ShowWebDialogWithParams(
     ui::WebDialogDelegate* delegate,
     base::Optional<views::Widget::InitParams> extra_params);
 
-// The implementation is more aligned with the appearance of constrained
-// web dialog.
-// |show| indicates whether to show the web dialog after it is created.
-// TODO(weili): Solely use this function on non-ChromeOS platform, and
-// above ShowWebDialogWithParams() on ChromeOS. Or merge these two if possible.
-gfx::NativeWindow CreateWebDialogWithBounds(gfx::NativeView parent,
-                                            content::BrowserContext* context,
-                                            ui::WebDialogDelegate* delegate,
-                                            const gfx::Rect& bounds,
-                                            bool show = true);
-
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_UI_VIEWS_CHROME_WEB_DIALOG_VIEW_H_

chrome/browser/ui/views/hats/hats_web_dialog.cc

@@ -9,15 +9,16 @@
 #include "base/bind.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/views/chrome_web_dialog_view.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/hats/hats_bubble_view.h"
-#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
-#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "chrome/browser/ui/webui/chrome_web_contents_handler.h"
 #include "chrome/grit/browser_resources.h"
+#include "components/constrained_window/constrained_window_views.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/performance_manager/embedder/performance_manager_registry.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
@@ -25,6 +26,7 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/template_expressions.h"
 #include "ui/gfx/geometry/size.h"
+#include "ui/views/controls/webview/web_dialog_view.h"
 #include "ui/views/widget/widget.h"
 #include "url/url_canon.h"
 #include "url/url_util.h"
@@ -137,7 +139,6 @@ std::string HatsWebDialog::GetDialogArgs() const {
 }
 
 void HatsWebDialog::OnDialogClosed(const std::string& json_retval) {
-  delete this;
 }
 
 void HatsWebDialog::OnCloseContents(content::WebContents* source,
@@ -195,6 +196,22 @@ void HatsWebDialog::OnMainFrameResourceLoadComplete(
   }
 }
 
+views::View* HatsWebDialog::GetContentsView() {
+  return webview_;
+}
+
+views::Widget* HatsWebDialog::GetWidget() {
+  return webview_->GetWidget();
+}
+
+const views::Widget* HatsWebDialog::GetWidget() const {
+  return webview_->GetWidget();
+}
+
+ui::ModalType HatsWebDialog::GetModalType() const {
+  return ui::MODAL_TYPE_WINDOW;
+}
+
 void HatsWebDialog::OnLoadTimedOut() {
   // Once loading is timed out, it means there is some problem such as network
   // error, unresponsive server etc. No need to wait any longer. Delete the
@@ -212,23 +229,25 @@ const base::TimeDelta HatsWebDialog::ContentLoadingTimeout() const {
 
 void HatsWebDialog::CreateWebDialog(Browser* browser) {
   // Create a web dialog aligned to the bottom center of the location bar.
-  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
-  LocationBarView* location_bar = browser_view->GetLocationBarView();
-  DCHECK(location_bar);
-  gfx::Point origin(location_bar->origin());
-  views::View::ConvertPointToTarget(location_bar->parent(), browser_view,
-                                    &origin);
-  gfx::Rect bounds(origin, location_bar->size());
-  bounds = gfx::Rect(
-      bounds.x() +
-          std::max(0, bounds.width() / 2 - kDefaultHatsDialogWidth / 2),
-      bounds.bottom() - views::BubbleBorder::GetBorderAndShadowInsets().top(),
-      kDefaultHatsDialogWidth, kDefaultHatsDialogHeight);
-  gfx::NativeWindow native_window = chrome::CreateWebDialogWithBounds(
-      browser_view->GetWidget()->GetNativeView(), off_the_record_profile(),
-      this, bounds,
-      /*show=*/false);
-  preloading_widget_ = views::Widget::GetWidgetForNativeWindow(native_window);
+  DialogDelegate::SetButtons(ui::DIALOG_BUTTON_NONE);
+  webview_ =
+      new views::WebDialogView(off_the_record_profile(), this,
+                               std::make_unique<ChromeWebContentsHandler>(),
+                               /* use_dialog_frame= */ true);
+  webview_->SetPreferredSize(
+      gfx::Size(kDefaultHatsDialogWidth, kDefaultHatsDialogHeight));
+  preloading_widget_ = constrained_window::CreateBrowserModalDialogViews(
+      this, browser_->tab_strip_model()
+                ->GetActiveWebContents()
+                ->GetTopLevelNativeWindow());
+
+  // Observer is needed for ChromeVox extension to send messages between content
+  // and background scripts.
+  extensions::ChromeExtensionWebContentsObserver::CreateForWebContents(
+      webview_->web_contents());
+
+  performance_manager::PerformanceManagerRegistry::GetInstance()
+      ->CreatePageNodeForWebContents(webview_->web_contents());
 
   // Start the loading timer once it is created.
   loading_timer_.Start(FROM_HERE, ContentLoadingTimeout(),
@@ -244,8 +263,10 @@ void HatsWebDialog::OnOriginalProfileDestroyed(Profile* profile) {
 
 void HatsWebDialog::Show(views::Widget* widget, bool accept) {
   if (accept) {
-    if (widget)
+    if (widget) {
       widget->Show();
+      webview_->RequestFocus();
+    }
     return;
   }
 

chrome/browser/ui/views/hats/hats_web_dialog.h

@@ -12,12 +12,14 @@
 #include "base/macros.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/profiles/independent_otr_profile_manager.h"
+#include "ui/views/window/dialog_delegate.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
 
 class Browser;
 class Profile;
 
 namespace views {
+class WebDialogView;
 class Widget;
 }
 
@@ -25,7 +27,8 @@ class Widget;
 // This class lives on the UI thread and is self deleting.
 // TODO(weili): This dialog shares a lot of common code with the one in
 // chrome/browser/chromeos/hats/, should be merged into one.
-class HatsWebDialog : public ui::WebDialogDelegate {
+class HatsWebDialog : public ui::WebDialogDelegate,
+                      public views::DialogDelegateView {
  public:
   // Create an instance of HatsWebDialog and load its content without showing.
   static void Create(Browser* browser, const std::string& site_id);
@@ -50,7 +53,6 @@ class HatsWebDialog : public ui::WebDialogDelegate {
   void GetDialogSize(gfx::Size* size) const override;
   bool CanResizeDialog() const override;
   std::string GetDialogArgs() const override;
-  // NOTE: This function deletes this object at the end.
   void OnDialogClosed(const std::string& json_retval) override;
   void OnCloseContents(content::WebContents* source,
                        bool* out_close_dialog) override;
@@ -61,6 +63,12 @@ class HatsWebDialog : public ui::WebDialogDelegate {
   void OnMainFrameResourceLoadComplete(
       const blink::mojom::ResourceLoadInfo& resource_load_info) override;
 
+  // views::DialogDelegateView implementation.
+  views::View* GetContentsView() override;
+  views::Widget* GetWidget() override;
+  const views::Widget* GetWidget() const override;
+  ui::ModalType GetModalType() const override;
+
   // These are virtual for tests.
   virtual void OnLoadTimedOut();
   virtual const base::TimeDelta ContentLoadingTimeout() const;
@@ -84,6 +92,7 @@ class HatsWebDialog : public ui::WebDialogDelegate {
   // The widget created for preloading. It is owned by us until it is shown to
   // user.
   views::Widget* preloading_widget_{nullptr};
+  views::WebDialogView* webview_{nullptr};
 
   // Indicate whether HaTS resources were loaded successfully.
   bool resource_loaded_{false};