cbui: stop using BubbleUi for bluetooth chooser

This change:
1) Adds a new function, chrome::ShowDeviceChooserDialog, that displays a
   device chooser bypassing the BubbleUi framework
2) Has Browser use this new function rather than BubbleUi to show the
   bluetooth device choosers

Note that ShowDeviceChooserDialog returns a OnceClosure, which the client
can call to close the dialog. It does not return a pointer to the dialog
since the dialog is self-owning. Previously, this code dealt with that
by passing around BubbleReferences, which have weak pointer semantics;
returning a OnceClosure (which internally has a WeakPtr to the UI)
allows the client code to close the bubble when it wants to without adding
new lifetime issues.

Remaining uses of ChooserBubbleDelegate directly:
* Web USB chooser
* HID chooser
* Serial chooser

These use similar design patterns, and will be migrated in follow-up
changes.

Bug: 496955
Change-Id: I1b45c1cfdba6e0eaee57ef61d91da294832a0888
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102947
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750540}

chrome/browser/ui/bluetooth/bluetooth_chooser_desktop.cc

@@ -6,11 +6,12 @@
 
 #include "base/logging.h"
 #include "chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h"
-#include "components/bubble/bubble_controller.h"
 
 BluetoothChooserDesktop::BluetoothChooserDesktop(
-    BluetoothChooserController* bluetooth_chooser_controller)
-    : bluetooth_chooser_controller_(bluetooth_chooser_controller) {
+    BluetoothChooserController* bluetooth_chooser_controller,
+    base::OnceClosure&& close_closure)
+    : bluetooth_chooser_controller_(bluetooth_chooser_controller),
+      close_closure_(std::move(close_closure)) {
   DCHECK(bluetooth_chooser_controller_);
 }
 
@@ -19,8 +20,8 @@ BluetoothChooserDesktop::~BluetoothChooserDesktop() {
   // that the EventHandler can be destroyed any time after the BluetoothChooser
   // instance.
   bluetooth_chooser_controller_->ResetEventHandler();
-  if (bubble_)
-    bubble_->CloseBubble(BUBBLE_CLOSE_FORCED);
+  if (close_closure_)
+    std::move(close_closure_).Run();
 }
 
 void BluetoothChooserDesktop::SetAdapterPresence(AdapterPresence presence) {

chrome/browser/ui/bluetooth/bluetooth_chooser_desktop.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_BLUETOOTH_BLUETOOTH_CHOOSER_DESKTOP_H_
 #define CHROME_BROWSER_UI_BLUETOOTH_BLUETOOTH_CHOOSER_DESKTOP_H_
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "components/bubble/bubble_reference.h"
 #include "content/public/browser/bluetooth_chooser.h"
@@ -16,8 +17,9 @@ class BluetoothChooserController;
 // BluetoothChooserAndroid implements the mobile part.
 class BluetoothChooserDesktop : public content::BluetoothChooser {
  public:
-  explicit BluetoothChooserDesktop(
-      BluetoothChooserController* bluetooth_chooser_controller);
+  BluetoothChooserDesktop(
+      BluetoothChooserController* bluetooth_chooser_controller,
+      base::OnceClosure&& close_closure);
   ~BluetoothChooserDesktop() override;
 
   // BluetoothChooser:
@@ -30,17 +32,13 @@ class BluetoothChooserDesktop : public content::BluetoothChooser {
                          bool is_paired,
                          int signal_strength_level) override;
 
-  // Sets a reference to the bubble being displayed so that it can be closed if
-  // this object is destroyed.
-  void set_bubble(base::WeakPtr<BubbleController> bubble) {
-    bubble_ = std::move(bubble);
-  }
-
  private:
   // Weak. DeviceChooserContentView[Cocoa] owns it.
   BluetoothChooserController* bluetooth_chooser_controller_;
 
-  base::WeakPtr<BubbleController> bubble_;
+  // Closes the displayed UI if it is still open. Used to ensure the bubble
+  // closes if this controller is torn down.
+  base::OnceClosure close_closure_;
 
   DISALLOW_COPY_AND_ASSIGN(BluetoothChooserDesktop);
 };

chrome/browser/ui/bluetooth/bluetooth_scanning_prompt_desktop.cc

@@ -6,12 +6,13 @@
 
 #include "base/logging.h"
 #include "chrome/browser/ui/bluetooth/bluetooth_scanning_prompt_controller.h"
-#include "components/bubble/bubble_controller.h"
 
 BluetoothScanningPromptDesktop::BluetoothScanningPromptDesktop(
-    BluetoothScanningPromptController* bluetooth_scanning_prompt_controller)
+    BluetoothScanningPromptController* bluetooth_scanning_prompt_controller,
+    base::OnceClosure&& close_closure)
     : bluetooth_scanning_prompt_controller_(
-          bluetooth_scanning_prompt_controller) {
+          bluetooth_scanning_prompt_controller),
+      close_closure_(std::move(close_closure)) {
   DCHECK(bluetooth_scanning_prompt_controller_);
 }
 
@@ -20,8 +21,8 @@ BluetoothScanningPromptDesktop::~BluetoothScanningPromptDesktop() {
   // requirement that the EventHandler can be destroyed any time after the
   // BluetoothScanningPrompt instance.
   bluetooth_scanning_prompt_controller_->ResetEventHandler();
-  if (bubble_)
-    bubble_->CloseBubble(BUBBLE_CLOSE_FORCED);
+  if (close_closure_)
+    std::move(close_closure_).Run();
 }
 
 void BluetoothScanningPromptDesktop::AddOrUpdateDevice(

chrome/browser/ui/bluetooth/bluetooth_scanning_prompt_desktop.h

@@ -5,8 +5,8 @@
 #ifndef CHROME_BROWSER_UI_BLUETOOTH_BLUETOOTH_SCANNING_PROMPT_DESKTOP_H_
 #define CHROME_BROWSER_UI_BLUETOOTH_BLUETOOTH_SCANNING_PROMPT_DESKTOP_H_
 
+#include "base/callback.h"
 #include "base/macros.h"
-#include "components/bubble/bubble_reference.h"
 #include "content/public/browser/bluetooth_scanning_prompt.h"
 
 class BluetoothScanningPromptController;
@@ -17,7 +17,8 @@ class BluetoothScanningPromptController;
 class BluetoothScanningPromptDesktop : public content::BluetoothScanningPrompt {
  public:
   explicit BluetoothScanningPromptDesktop(
-      BluetoothScanningPromptController* bluetooth_scanning_prompt_controller);
+      BluetoothScanningPromptController* bluetooth_scanning_prompt_controller,
+      base::OnceClosure&& close_closure);
   ~BluetoothScanningPromptDesktop() override;
 
   // content::BluetoothScanningPrompt:
@@ -25,17 +26,13 @@ class BluetoothScanningPromptDesktop : public content::BluetoothScanningPrompt {
                          bool should_update_name,
                          const base::string16& device_name) override;
 
-  // Sets a reference to the bubble being displayed so that it can be closed if
-  // this object is destroyed.
-  void set_bubble(base::WeakPtr<BubbleController> bubble) {
-    bubble_ = std::move(bubble);
-  }
-
  private:
   // Weak. DeviceChooserContentView owns it.
   BluetoothScanningPromptController* bluetooth_scanning_prompt_controller_;
 
-  base::WeakPtr<BubbleController> bubble_;
+  // Closes the displayed UI, if there is one. This is used to ensure the UI
+  // closes if this controller is destroyed.
+  base::OnceClosure close_closure_;
 
   DISALLOW_COPY_AND_ASSIGN(BluetoothScanningPromptDesktop);
 };

chrome/browser/ui/browser.cc

@@ -162,7 +162,6 @@
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "components/bookmarks/common/bookmark_pref_names.h"
-#include "components/bubble/bubble_controller.h"
 #include "components/captive_portal/core/buildflags.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/favicon/content/content_favicon_driver.h"
@@ -1421,44 +1420,26 @@ blink::SecurityStyle Browser::GetSecurityStyle(
 std::unique_ptr<content::BluetoothChooser> Browser::RunBluetoothChooser(
     content::RenderFrameHost* frame,
     const content::BluetoothChooser::EventHandler& event_handler) {
-  std::unique_ptr<BluetoothChooserController> bluetooth_chooser_controller(
-      new BluetoothChooserController(frame, event_handler));
+  auto controller =
+      std::make_unique<BluetoothChooserController>(frame, event_handler);
+  auto* controller_ptr = controller.get();
 
-  std::unique_ptr<BluetoothChooserDesktop> bluetooth_chooser_desktop(
-      new BluetoothChooserDesktop(bluetooth_chooser_controller.get()));
-
-  std::unique_ptr<ChooserBubbleDelegate> chooser_bubble_delegate(
-      new ChooserBubbleDelegate(frame,
-                                std::move(bluetooth_chooser_controller)));
-
-  Browser* browser = chrome::FindBrowserWithWebContents(
-      WebContents::FromRenderFrameHost(frame));
-  BubbleReference bubble_reference = browser->GetBubbleManager()->ShowBubble(
-      std::move(chooser_bubble_delegate));
-  bluetooth_chooser_desktop->set_bubble(std::move(bubble_reference));
-
-  return std::move(bluetooth_chooser_desktop);
+  return std::make_unique<BluetoothChooserDesktop>(
+      controller_ptr,
+      chrome::ShowDeviceChooserDialog(frame, std::move(controller)));
 }
 
 std::unique_ptr<content::BluetoothScanningPrompt>
 Browser::ShowBluetoothScanningPrompt(
     content::RenderFrameHost* frame,
     const content::BluetoothScanningPrompt::EventHandler& event_handler) {
-  auto bluetooth_scanning_prompt_controller =
+  auto controller =
       std::make_unique<BluetoothScanningPromptController>(frame, event_handler);
+  auto* controller_ptr = controller.get();
 
-  auto bluetooth_scanning_prompt_desktop =
-      std::make_unique<BluetoothScanningPromptDesktop>(
-          bluetooth_scanning_prompt_controller.get());
-
-  auto chooser_bubble_delegate = std::make_unique<ChooserBubbleDelegate>(
-      frame, std::move(bluetooth_scanning_prompt_controller));
-
-  BubbleReference bubble_reference =
-      GetBubbleManager()->ShowBubble(std::move(chooser_bubble_delegate));
-  bluetooth_scanning_prompt_desktop->set_bubble(std::move(bubble_reference));
-
-  return std::move(bluetooth_scanning_prompt_desktop);
+  return std::make_unique<BluetoothScanningPromptDesktop>(
+      controller_ptr,
+      chrome::ShowDeviceChooserDialog(frame, std::move(controller)));
 }
 
 void Browser::CreateSmsPrompt(content::RenderFrameHost*,

chrome/browser/ui/browser_dialogs.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/browser_dialogs.h"
 
+#include "base/bind_helpers.h"
 #include "base/metrics/histogram_macros.h"
 
 namespace chrome {
@@ -13,6 +14,15 @@ void RecordDialogCreation(DialogIdentifier identifier) {
                             DialogIdentifier::MAX_VALUE);
 }
 
+#if !defined(TOOLKIT_VIEWS)
+base::OnceClosure ShowDeviceChooserDialog(
+    content::RenderFrameHost* owner,
+    std::unique_ptr<ChooserController> controller) {
+  NOTIMPLEMENTED();
+  return base::DoNothing();
+}
+#endif
+
 }  // namespace chrome
 
 #if !defined(TOOLKIT_VIEWS)

chrome/browser/ui/browser_dialogs.h

@@ -20,6 +20,7 @@
 #include "ui/gfx/native_widget_types.h"
 
 class Browser;
+class ChooserController;
 class LoginHandler;
 class Profile;
 struct WebApplicationInfo;
@@ -309,6 +310,13 @@ void ShowExtensionInstallBlockedDialog(
     content::WebContents* web_contents,
     base::OnceClosure done_callback);
 
+// Returns a OnceClosure that client code can call to close the device chooser.
+// This OnceClosure references the actual dialog as a WeakPtr, so it's safe to
+// call at any point.
+base::OnceClosure ShowDeviceChooserDialog(
+    content::RenderFrameHost* owner,
+    std::unique_ptr<ChooserController> controller);
+
 }  // namespace chrome
 
 void ShowFolderUploadConfirmationDialog(

chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc

@@ -4,9 +4,12 @@
 
 #include "chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.h"
 
+#include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/chooser_controller/chooser_controller.h"
 #include "chrome/browser/ui/browser_dialogs.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/permission_bubble/chooser_bubble_delegate.h"
 #include "chrome/browser/ui/views/bubble_anchor_util_views.h"
 #include "chrome/browser/ui/views/device_chooser_content_view.h"
@@ -63,17 +66,21 @@ class ChooserBubbleUiViewDelegate : public views::BubbleDialogDelegateView,
   void set_bubble_reference(BubbleReference bubble_reference);
   void UpdateTableView() const;
 
+  base::OnceClosure MakeCloseClosure();
+  void Close();
+
  private:
-  DeviceChooserContentView* device_chooser_content_view_;
+  DeviceChooserContentView* device_chooser_content_view_ = nullptr;
   BubbleReference bubble_reference_;
 
+  base::WeakPtrFactory<ChooserBubbleUiViewDelegate> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(ChooserBubbleUiViewDelegate);
 };
 
 ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
     Browser* browser,
-    std::unique_ptr<ChooserController> chooser_controller)
-    : device_chooser_content_view_(nullptr) {
+    std::unique_ptr<ChooserController> chooser_controller) {
   // ------------------------------------
   // | Chooser bubble title             |
   // | -------------------------------- |
@@ -166,6 +173,16 @@ void ChooserBubbleUiViewDelegate::UpdateTableView() const {
   device_chooser_content_view_->UpdateTableView();
 }
 
+base::OnceClosure ChooserBubbleUiViewDelegate::MakeCloseClosure() {
+  return base::BindOnce(&ChooserBubbleUiViewDelegate::Close,
+                        weak_ptr_factory_.GetWeakPtr());
+}
+
+void ChooserBubbleUiViewDelegate::Close() {
+  if (GetWidget())
+    GetWidget()->CloseWithReason(views::Widget::ClosedReason::kUnspecified);
+}
+
 //////////////////////////////////////////////////////////////////////////////
 // ChooserBubbleUi
 ChooserBubbleUi::ChooserBubbleUi(
@@ -209,3 +226,33 @@ void ChooserBubbleUi::OnWidgetClosing(views::Widget* widget) {
   widget->RemoveObserver(this);
   chooser_bubble_ui_view_delegate_ = nullptr;
 }
+
+namespace chrome {
+
+base::OnceClosure ShowDeviceChooserDialog(
+    content::RenderFrameHost* owner,
+    std::unique_ptr<ChooserController> controller) {
+  Browser* browser = chrome::FindBrowserWithWebContents(
+      content::WebContents::FromRenderFrameHost(owner));
+  auto bubble = std::make_unique<ChooserBubbleUiViewDelegate>(
+      browser, std::move(controller));
+
+  // Set |parent_window_| because some valid anchors can become hidden.
+  views::Widget* parent_widget = views::Widget::GetWidgetForNativeWindow(
+      browser->window()->GetNativeWindow());
+  gfx::NativeView parent = parent_widget->GetNativeView();
+  DCHECK(parent);
+  bubble->set_parent_window(parent);
+
+  base::OnceClosure close_closure = bubble->MakeCloseClosure();
+  views::Widget* widget =
+      views::BubbleDialogDelegateView::CreateBubble(bubble.release());
+  if (browser->window()->IsActive())
+    widget->Show();
+  else
+    widget->ShowInactive();
+
+  return close_closure;
+}
+
+}  // namespace chrome