[lensoverlay] Fix menu button on Windows.

The menu button was previously only working on Mac and Linux due to
crbug.com/378566071. The preselection bubble should be active for the
menu button to work.

Also fixes a pre-existing issue where closing the bubble using the
escape key leads to a crash when reshowing the bubble.

Bug: b:374051721, 378565178
Change-Id: I1ff68c8a7e3539d612653c6d3afa3dc8a28efb3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6013706
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Duncan Mercer <mercerd@google.com>
Commit-Queue: Bryan Nguyen <nguyenbryan@google.com>
Cr-Commit-Position: refs/heads/main@{#1383339}

chrome/browser/ui/lens/lens_overlay_controller.cc

@@ -2015,6 +2015,7 @@ void LensOverlayController::OnViewBoundsChanged(views::View* observed_view) {
 
 void LensOverlayController::OnWidgetDestroying(views::Widget* widget) {
   preselection_widget_ = nullptr;
+  preselection_widget_observer_.Reset();
 }
 
 void LensOverlayController::OnOmniboxFocusChanged(
@@ -2488,10 +2489,16 @@ void LensOverlayController::ShowPreselectionBubble() {
             weak_factory_.GetWeakPtr(),
             tab_->GetBrowserWindowInterface()->TopContainer(),
             net::NetworkChangeNotifier::IsOffline(),
-            base::BindRepeating(&LensOverlayController::CloseUIAsync,
-                                weak_factory_.GetWeakPtr(),
-                                lens::LensOverlayDismissalSource::
-                                    kPreselectionToastExitButton)));
+            /*exit_clicked_callback=*/
+            base::BindRepeating(
+                &LensOverlayController::CloseUIAsync,
+                weak_factory_.GetWeakPtr(),
+                lens::LensOverlayDismissalSource::kPreselectionToastExitButton),
+            /*on_cancel_callback=*/
+            base::BindOnce(&LensOverlayController::CloseUIAsync,
+                           weak_factory_.GetWeakPtr(),
+                           lens::LensOverlayDismissalSource::
+                               kPreselectionToastEscapeKeyPress)));
     preselection_widget_->SetNativeWindowProperty(
         views::kWidgetIdentifierKey,
         const_cast<void*>(kLensOverlayPreselectionWidgetIdentifier));

chrome/browser/ui/lens/lens_preselection_bubble.cc

@@ -53,22 +53,24 @@ LensPreselectionBubble::LensPreselectionBubble(
     base::WeakPtr<LensOverlayController> lens_overlay_controller,
     views::View* anchor_view,
     bool offline,
-    ExitClickedCallback callback)
+    ExitClickedCallback exit_clicked_callback,
+    base::OnceClosure on_cancel_callback)
     : BubbleDialogDelegateView(anchor_view,
                                views::BubbleBorder::NONE,
                                views::BubbleBorder::NO_SHADOW),
       lens_overlay_controller_(lens_overlay_controller),
       offline_(offline),
-      callback_(std::move(callback)) {
-  // Toast bubble doesn't have any buttons, cannot be active, and should not be
-  // focus traversable.
+      exit_clicked_callback_(std::move(exit_clicked_callback)) {
   SetShowCloseButton(false);
-  SetCanActivate(false);
-  set_focus_traversable_from_anchor_view(false);
+  // Should be true for menu button to work, although this constraint is not
+  // being upheld on Mac and Linux. See crbug.com/378566071.
+  SetCanActivate(true);
+  set_close_on_deactivate(false);
   DialogDelegate::SetButtons(static_cast<int>(ui::mojom::DialogButton::kNone));
   set_corner_radius(48);
   SetProperty(views::kElementIdentifierKey, kLensPreselectionBubbleElementId);
   SetAccessibleWindowRole(ax::mojom::Role::kAlertDialog);
+  SetCancelCallback(std::move(on_cancel_callback));
 }
 
 LensPreselectionBubble::~LensPreselectionBubble() = default;
@@ -117,7 +119,7 @@ void LensPreselectionBubble::Init() {
   label_->SetAutoColorReadabilityEnabled(false);
   if (offline_) {
     exit_button_ = AddChildView(std::make_unique<views::MdTextButton>(
-        std::move(callback_),
+        std::move(exit_clicked_callback_),
         l10n_util::GetStringUTF16(
             IDS_LENS_OVERLAY_INITIAL_TOAST_ERROR_EXIT_BUTTON_TEXT)));
     exit_button_->SetProperty(views::kMarginsKey,

chrome/browser/ui/lens/lens_preselection_bubble.h

@@ -32,7 +32,8 @@ class LensPreselectionBubble : public views::BubbleDialogDelegateView,
       base::WeakPtr<LensOverlayController> lens_overlay_controller,
       views::View* anchor_view,
       bool offline,
-      ExitClickedCallback callback);
+      ExitClickedCallback exit_clicked_callback,
+      base::OnceClosure on_cancel_callback);
   ~LensPreselectionBubble() override;
 
   // views::BubbleDialogDelegateView:
@@ -75,7 +76,7 @@ class LensPreselectionBubble : public views::BubbleDialogDelegateView,
   // Whether user is offline.
   bool offline_ = false;
   // Callback for exit button which closes the lens overlay.
-  ExitClickedCallback callback_;
+  ExitClickedCallback exit_clicked_callback_;
   // Model for the more info menu.
   std::unique_ptr<ui::MenuModel> more_info_menu_model_;
   // Runner for the more info menu.

chrome/browser/ui/lens/lens_preselection_bubble_interactive_uitest.cc

@@ -49,7 +49,8 @@ class LensPreselectionBubbleInteractiveUiTest : public InteractiveBrowserTest {
               net::NetworkChangeNotifier::IsOffline(),
               base::BindRepeating(
                   &LensPreselectionBubbleInteractiveUiTest::ExitClickedCallback,
-                  base::Unretained(this))));
+                  base::Unretained(this)),
+              base::NullCallback()));
       preselection_widget_->Show();
     }));
   }

components/lens/lens_overlay_dismissal_source.h

@@ -118,7 +118,11 @@ enum class LensOverlayDismissalSource {
   // becomes unresponsive with slow connection).
   kNetworkIssue = 29,
 
-  kMaxValue = kNetworkIssue
+  // The user pressed the escape key while focused on the preselection toast.
+  // Only used on Desktop.
+  kPreselectionToastEscapeKeyPress = 30,
+
+  kMaxValue = kPreselectionToastEscapeKeyPress
 };
 // LINT.ThenChange(//tools/metrics/histograms/metadata/lens/enums.xml:LensOverlayDismissalSource)
 

tools/metrics/histograms/metadata/lens/enums.xml

@@ -60,6 +60,7 @@ chromium-metrics-reviews@google.com.
   <int value="27" label="Lens permission declined"/>
   <int value="28" label="Low memory"/>
   <int value="29" label="Network issue"/>
+  <int value="30" label="Escape key press on preselection toast"/>
 </enum>
 
 <!-- LINT.ThenChange(//components/lens/lens_overlay_dismissal_source.h:LensOverlayDismissalSource) -->