[Live Caption] Ensure widget can only be activated when it is visible.

This fixes a bug on Windows in which switching to a new tab via a
keyboard shortcut did not focus the tab. Instead, the CaptionBubble was
focused by the FocusManager. This caused keyboard shortcuts, such as
Ctrl+W, to not work. This occurred even if the CaptionBubble was not
not visible.

Bug: 1055150, 1123635
Change-Id: I614715f75f1ff36fb336ae13ca3a53178413ca8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2482005
Commit-Queue: Abigail Klein <abigailbklein@google.com>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823622}

chrome/browser/ui/views/accessibility/caption_bubble.cc

@@ -244,7 +244,7 @@ gfx::Rect CaptionBubble::GetBubbleBounds() {
 
 void CaptionBubble::OnWidgetBoundsChanged(views::Widget* widget,
                                           const gfx::Rect& new_bounds) {
-  DCHECK(GetWidget());
+  DCHECK_EQ(widget, GetWidget());
   gfx::Rect widget_bounds = GetWidget()->GetWindowBoundsInScreen();
   gfx::Rect anchor_rect = GetAnchorView()->GetBoundsInScreen();
   if (latest_bounds_ == widget_bounds && latest_anchor_bounds_ == anchor_rect) {
@@ -257,12 +257,10 @@ void CaptionBubble::OnWidgetBoundsChanged(views::Widget* widget,
     return;
   }
 
-  // Check that the widget which changed size is our widget. It's possible for
-  // this to be called when another widget resizes.
-  // Also check that our widget is visible. If it is not visible then
+  // Check that our widget is visible. If it is not visible then
   // the user has not explicitly moved it (because the user can't see it),
   // so we should take no action.
-  if (widget != GetWidget() || !GetWidget()->IsVisible())
+  if (!GetWidget()->IsVisible())
     return;
 
   // The widget has moved within the window. Recalculate the desired ratio
@@ -633,11 +631,9 @@ void CaptionBubble::UpdateBubbleVisibility() {
     if (GetWidget()->IsVisible())
       GetWidget()->Hide();
   } else if (!model_->GetFullText().empty() || model_->HasError()) {
-    // Show the widget if it has text or an error to display. Only show the
-    // widget if it isn't already visible. Always calling Widget::Show() will
-    // mean the widget gets focus each time.
+    // Show the widget if it has text or an error to display.
     if (!GetWidget()->IsVisible()) {
-      GetWidget()->Show();
+      GetWidget()->ShowInactive();
       GetViewAccessibility().AnnounceText(l10n_util::GetStringUTF16(
           IDS_LIVE_CAPTION_BUBBLE_APPEAR_SCREENREADER_ANNOUNCEMENT));
     }
@@ -647,6 +643,24 @@ void CaptionBubble::UpdateBubbleVisibility() {
   }
 }
 
+void CaptionBubble::OnWidgetVisibilityChanged(views::Widget* widget,
+                                              bool visible) {
+  DCHECK_EQ(widget, GetWidget());
+  // Ensure that the widget is only activated when it is visible.
+  // TODO(crbug.com/1144201): Investigate whether Hide() should always
+  // deactivate widgets, and if so, remove this.
+  if (visible) {
+#if !defined(OS_MAC)
+    // On MacOS browsertests, which do not have an activation policy, the widget
+    // might already be activated.
+    DCHECK(!widget->IsActive());
+#endif
+    widget->Activate();
+  } else {
+    widget->Deactivate();
+  }
+}
+
 void CaptionBubble::UpdateCaptionStyle(
     base::Optional<ui::CaptionStyle> caption_style) {
   caption_style_ = caption_style;

chrome/browser/ui/views/accessibility/caption_bubble.h

@@ -82,6 +82,7 @@ class CaptionBubble : public views::BubbleDialogDelegateView {
   gfx::Rect GetBubbleBounds() override;
   void OnWidgetBoundsChanged(views::Widget* widget,
                              const gfx::Rect& new_bounds) override;
+  void OnWidgetVisibilityChanged(views::Widget* widget, bool visible) override;
   void OnKeyEvent(ui::KeyEvent* event) override;
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
   void OnFocus() override;

chrome/browser/ui/views/accessibility/caption_bubble_controller_views_browsertest.cc

@@ -103,6 +103,10 @@ class CaptionBubbleControllerViewsTest : public InProcessBrowserTest {
     return controller_ && controller_->IsWidgetVisibleForTesting();
   }
 
+  bool IsWidgetActive() {
+    return GetCaptionWidget() && GetCaptionWidget()->IsActive();
+  }
+
   void DestroyController() { controller_.reset(nullptr); }
 
   void ClickButton(views::Button* button) {
@@ -1014,4 +1018,20 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
   EXPECT_EQ("a ", GetVirtualChildrenText()[8]);
 }
 
+#if !defined(OS_MAC)
+// Tests are flaky on Mac: Mac browsertests do not have an activation policy so
+// the widget activation may not work as expected.
+IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
+                       BubbleDeactivatedWhenHidden) {
+  EXPECT_FALSE(IsWidgetVisible());
+  EXPECT_FALSE(IsWidgetActive());
+  OnPartialTranscription("Cows can detect odors up to 6 miles away.");
+  EXPECT_TRUE(IsWidgetVisible());
+  EXPECT_TRUE(IsWidgetActive());
+  ClickButton(GetCloseButton());
+  EXPECT_FALSE(IsWidgetVisible());
+  EXPECT_FALSE(IsWidgetActive());
+}
+#endif
+
 }  // namespace captions

ui/views/widget/widget.h

@@ -625,7 +625,9 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
   // Hides the widget.
   void Hide();
 
-  // Like Show(), but does not activate the window.
+  // Like Show(), but does not activate the window. Tests may be flaky on Mac:
+  // Mac browsertests do not have an activation policy so the widget may be
+  // activated.
   void ShowInactive();
 
   // Activates the widget, assuming it already exists and is visible.