Lifetime of Compose's BubbleContentsWrapperT matches its BubbleDialogView.

Previously the ChromeComposeClient would own (indirectly) the
BubbleContentsWrapperT via ChromeComposeDialogController. Normally this
would not be a problem as the dialog closes right after the web
contents, but it was possible for the webui to reference the destroyed
WebContents in certain scenarios.

Bug: 1499835
Change-Id: Id8c7ea3caad2dc9d4eb20cb97804b0cbadd0f230
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5021240
Reviewed-by: Jeffrey Cohen <jeffreycohen@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1224489}
diff --git a/chrome/browser/compose/compose_dialog_browsertest.cc b/chrome/browser/compose/compose_dialog_browsertest.cc
new file mode 100644
index 0000000..4487159
--- /dev/null
+++ b/chrome/browser/compose/compose_dialog_browsertest.cc
@@ -0,0 +1,66 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/compose/compose_session.h"
+
+#include "base/test/scoped_feature_list.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_enums.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/compose/core/browser/compose_features.h"
+#include "components/optimization_guide/core/optimization_guide_features.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "ui/gfx/geometry/point_conversions.h"
+
+namespace compose {
+
+//  IDC_CONTEXT_COMPOSE
+
+class ComposeSessionBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUp() override {
+    feature_list()->InitWithFeatures(
+        {compose::features::kEnableCompose,
+         optimization_guide::features::kOptimizationGuideModelExecution},
+        {});
+    InProcessBrowserTest::SetUp();
+  }
+
+  base::test::ScopedFeatureList* feature_list() { return &feature_list_; }
+
+ protected:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(ComposeSessionBrowserTest, LifetimeOfBubbleWrapper) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL("/compose/test2.html")));
+  ASSERT_NE(nullptr, ChromeComposeClient::FromWebContents(web_contents));
+  auto* client = ChromeComposeClient::FromWebContents(web_contents);
+  client->GetComposeEnabling().SetEnabledForTesting();
+
+  // get point of element
+  gfx::PointF textarea_center =
+      content::GetCenterCoordinatesOfElementWithId(web_contents, "elem1");
+  autofill::FormFieldData field_data;
+  field_data.bounds = gfx::RectF((textarea_center), gfx::SizeF(1, 1));
+
+  client->ShowComposeDialog(
+      autofill::AutofillComposeDelegate::UiEntryPoint::kAutofillPopup,
+      field_data, std::nullopt, base::NullCallback());
+
+  // close window right away
+  browser()->tab_strip_model()->CloseWebContentsAt(0,
+                                                   TabCloseTypes::CLOSE_NONE);
+}
+
+}  // namespace compose
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 4d3d438..39d75152 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -5017,8 +5017,6 @@
       "views/commerce/price_tracking_view.h",
       "views/commerce/shopping_collection_iph_view.cc",
       "views/commerce/shopping_collection_iph_view.h",
-      "views/compose/compose_dialog_view.cc",
-      "views/compose/compose_dialog_view.h",
       "views/confirm_bubble_views.cc",
       "views/confirm_bubble_views.h",
       "views/constrained_web_dialog_delegate_views.cc",
@@ -6282,6 +6280,8 @@
       sources += [
         "views/compose/chrome_compose_dialog_controller.cc",
         "views/compose/chrome_compose_dialog_controller.h",
+        "views/compose/compose_dialog_view.cc",
+        "views/compose/compose_dialog_view.h",
       ]
 
       deps += [ "//components/compose/core/browser" ]
diff --git a/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.cc b/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.cc
index 5c1a64c..3066cc6 100644
--- a/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.cc
+++ b/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.cc
@@ -52,31 +52,29 @@
 
   Profile* profile =
       Profile::FromBrowserContext(web_contents_->GetBrowserContext());
-  bubble_wrapper_ = std::make_unique<BubbleContentsWrapperT<ComposeUI>>(
+  auto bubble_wrapper = std::make_unique<BubbleContentsWrapperT<ComposeUI>>(
       GURL(kComposeURL), profile, IDS_COMPOSE_TITLE);
-  bubble_wrapper_->ReloadWebContents();
+  bubble_wrapper->ReloadWebContents();
 
   // This WebUI needs to know the calling BrowserContents so that the compose
   // request/result can be properly associated with the triggering form.
-  bubble_wrapper_->GetWebUIController()->set_triggering_web_contents(
+  bubble_wrapper->GetWebUIController()->set_triggering_web_contents(
       web_contents_.get());
 
-  auto bubble_view = std::make_unique<WebUIBubbleDialogView>(
-      anchor_view, bubble_wrapper_.get(),
-      gfx::ToRoundedRect(element_bounds_in_screen));
-
-  // Allows the bubble bounds to escape the browser window.
-  bubble_view->set_has_parent(false);
-
-  bubble_ = bubble_view->GetWeakPtr();
-  views::BubbleDialogDelegateView::CreateBubble(std::move(bubble_view));
-
-  bubble_->set_adjust_if_offscreen(true);
+  auto compose_dialog_view = std::make_unique<ComposeDialogView>(
+      anchor_view, std::move(bubble_wrapper),
+      gfx::ToRoundedRect(element_bounds_in_screen),
+      views::BubbleBorder::Arrow::NONE);
+  bubble_ = compose_dialog_view->GetWeakPtr();
+  views::BubbleDialogDelegateView::CreateBubble(std::move(compose_dialog_view));
 }
 
 BubbleContentsWrapperT<ComposeUI>*
 ChromeComposeDialogController::GetBubbleWrapper() const {
-  return bubble_wrapper_.get();
+  if (bubble_) {
+    return bubble_->bubble_wrapper();
+  }
+  return nullptr;
 }
 
 void ChromeComposeDialogController::ShowUI() {
@@ -87,8 +85,10 @@
 
 // TODO(b/300939629): Flesh out implementation and cover other closing paths.
 void ChromeComposeDialogController::Close() {
-  bubble_wrapper_->CloseUI();
-  bubble_wrapper_.reset();
+  auto* wrapper = GetBubbleWrapper();
+  if (wrapper) {
+    wrapper->CloseUI();
+  }
 }
 
 ChromeComposeDialogController::ChromeComposeDialogController(
diff --git a/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.h b/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.h
index 2e149a1..4839c2d7 100644
--- a/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.h
+++ b/chrome/browser/ui/views/compose/chrome_compose_dialog_controller.h
@@ -42,8 +42,7 @@
  private:
   friend class ChromeComposeDialogControllerTest;
 
-  std::unique_ptr<BubbleContentsWrapperT<ComposeUI>> bubble_wrapper_;
-  base::WeakPtr<WebUIBubbleDialogView> bubble_;
+  base::WeakPtr<ComposeDialogView> bubble_;
   base::WeakPtr<content::WebContents> web_contents_;
 
   base::WeakPtrFactory<ChromeComposeDialogController> weak_ptr_factory_{this};
diff --git a/chrome/browser/ui/views/compose/compose_dialog_view.cc b/chrome/browser/ui/views/compose/compose_dialog_view.cc
index ad5a7c6..bf70beac 100644
--- a/chrome/browser/ui/views/compose/compose_dialog_view.cc
+++ b/chrome/browser/ui/views/compose/compose_dialog_view.cc
@@ -8,33 +8,25 @@
 #include "ui/gfx/geometry/skia_conversions.h"
 #include "ui/views/bubble/bubble_border.h"
 
-namespace compose {
-
-// Default size from Figma spec. The size of the view will follow the requested
-// size of the WebUI, once these are connected.
-constexpr gfx::Size kInputDialogSize(448, 216);
-
 ComposeDialogView::~ComposeDialogView() = default;
 
-ComposeDialogView::ComposeDialogView(View* anchor_view,
-                                     views::BubbleBorder::Arrow anchor_position,
-                                     const gfx::Rect bounds,
-                                     content::WebContents* web_contents)
-    : BubbleDialogDelegateView(anchor_view, anchor_position) {
-  // For testing, a test Window widget is used. Otherwise, no |anchor_view| is
-  // given and the parent Window must be manually set.
-  if (!anchor_view) {
-    set_parent_window(web_contents->GetNativeView());
-    SetAnchorRect(bounds);
-  }
+ComposeDialogView::ComposeDialogView(
+    View* anchor_view,
+    std::unique_ptr<BubbleContentsWrapperT<ComposeUI>> bubble_wrapper,
+    const gfx::Rect& anchor_bounds,
+    views::BubbleBorder::Arrow anchor_position)
+    : WebUIBubbleDialogView(anchor_view,
+                            bubble_wrapper.get(),
+                            anchor_bounds,
+                            anchor_position),
+      bubble_wrapper_(std::move(bubble_wrapper)) {
+  set_has_parent(false);
+  set_adjust_if_offscreen(true);
+}
 
-  SetButtons(ui::DIALOG_BUTTON_NONE);
-  SetPreferredSize(kInputDialogSize);
-
-  // Empty contents, to be populated by WebUI.
+base::WeakPtr<ComposeDialogView> ComposeDialogView::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
 }
 
 BEGIN_METADATA(ComposeDialogView, views::View)
 END_METADATA
-
-}  // namespace compose
diff --git a/chrome/browser/ui/views/compose/compose_dialog_view.h b/chrome/browser/ui/views/compose/compose_dialog_view.h
index 196e9d8d..2db755e 100644
--- a/chrome/browser/ui/views/compose/compose_dialog_view.h
+++ b/chrome/browser/ui/views/compose/compose_dialog_view.h
@@ -5,26 +5,34 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_COMPOSE_COMPOSE_DIALOG_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_COMPOSE_COMPOSE_DIALOG_VIEW_H_
 
+#include "chrome/browser/ui/views/bubble/webui_bubble_dialog_view.h"
+#include "chrome/browser/ui/webui/compose/compose_ui.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/metadata/metadata_header_macros.h"
-#include "ui/views/bubble/bubble_dialog_delegate_view.h"
 #include "ui/views/view.h"
 
-namespace compose {
-
 // A view for the contents area of the Compose dialog.
-class ComposeDialogView : public views::BubbleDialogDelegateView {
+class ComposeDialogView : public WebUIBubbleDialogView {
  public:
   METADATA_HEADER(ComposeDialogView);
 
-  explicit ComposeDialogView(View* anchor_view,
-                             views::BubbleBorder::Arrow anchor_position,
-                             const gfx::Rect bounds,
-                             content::WebContents* web_contents);
+  explicit ComposeDialogView(
+      View* anchor_view,
+      std::unique_ptr<BubbleContentsWrapperT<ComposeUI>> bubble_wrapper,
+      const gfx::Rect& anchor_bounds,
+      views::BubbleBorder::Arrow anchor_position);
 
   ~ComposeDialogView() override;
-};
 
-}  // namespace compose
+  BubbleContentsWrapperT<ComposeUI>* bubble_wrapper() {
+    return bubble_wrapper_.get();
+  }
+
+  base::WeakPtr<ComposeDialogView> GetWeakPtr();
+
+ private:
+  std::unique_ptr<BubbleContentsWrapperT<ComposeUI>> bubble_wrapper_;
+  base::WeakPtrFactory<ComposeDialogView> weak_ptr_factory_{this};
+};
 
 #endif  // CHROME_BROWSER_UI_VIEWS_COMPOSE_COMPOSE_DIALOG_VIEW_H_
diff --git a/chrome/browser/ui/webui/compose/compose_ui.cc b/chrome/browser/ui/webui/compose/compose_ui.cc
index 62d5615..bf4f226 100644
--- a/chrome/browser/ui/webui/compose/compose_ui.cc
+++ b/chrome/browser/ui/webui/compose/compose_ui.cc
@@ -90,9 +90,12 @@
   content::WebContents* web_contents = triggering_web_contents_
                                            ? triggering_web_contents_.get()
                                            : web_ui()->GetWebContents();
-  ChromeComposeClient::FromWebContents(web_contents)
-      ->BindComposeDialog(std::move(close_handler), std::move(handler),
-                          std::move(dialog));
+  ChromeComposeClient* client =
+      ChromeComposeClient::FromWebContents(web_contents);
+  if (client) {
+    client->BindComposeDialog(std::move(close_handler), std::move(handler),
+                              std::move(dialog));
+  }
 }
 
 WEB_UI_CONTROLLER_TYPE_IMPL(ComposeUI)
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index b0325462..6310e2d 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -2844,7 +2844,12 @@
     ]
 
     if (enable_compose) {
-      sources += [ "../browser/compose/compose_inner_text_browsertest.cc" ]
+      sources += [
+        "../browser/compose/compose_dialog_browsertest.cc",
+        "../browser/compose/compose_inner_text_browsertest.cc",
+      ]
+
+      deps += [ "//components/compose/core/browser:features" ]
     }
 
     if (enable_supervised_users) {
diff --git a/chrome/test/data/compose/test2.html b/chrome/test/data/compose/test2.html
new file mode 100644
index 0000000..699e41d2
--- /dev/null
+++ b/chrome/test/data/compose/test2.html
@@ -0,0 +1,11 @@
+<html>
+  <head>
+    <meta name="viewport" content="width=device-width,minimum-scale=1"></meta>
+  </head>
+  <body>
+    <textarea id="elem1" rows="4" style="font-family: monospace; width: 50ch">
+The
+textarea tag  defines a multi-line text input control.
+</textarea>
+  </body>
+</html>