Revert "Rotating focus with F6 includes the caption bubble."

This reverts commit bc22eed4.

Reason for revert: Test added in this CL seems to fail on Mac: https://ci.chromium.org/p/chromium/builders/ci/Mac10.12%20Tests/34405

Original change's description:
> Rotating focus with F6 includes the caption bubble.
> 
> Allows users to focus into and out of the caption bubble using F6,
> which rotates focus between focusable panes.
> 
> AX-Relnotes: N/A
> Bug: 1055150
> Change-Id: I82efd54644fbc1cada2cb3e2f5cf4c52da21336a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2197255
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Reviewed-by: Abigail Klein <abigailbklein@google.com>
> Commit-Queue: Katie Dektar <katie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#770294}

TBR=ellyjones@chromium.org,pkasting@chromium.org,katie@chromium.org,abigailbklein@google.com

Change-Id: Ia50b02b11a4c304cf9013128756acb60b79e682e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1055150
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208776Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770436}

chrome/browser/accessibility/caption_controller.cc

@@ -167,19 +167,14 @@ void CaptionController::DispatchTranscription(
     content::WebContents* web_contents,
     const chrome::mojom::TranscriptionResultPtr& transcription_result) {
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
-  if (!browser || !caption_bubble_controllers_.count(browser))
+  if (!browser)
+    return;
+  if (!caption_bubble_controllers_.count(browser))
     return;
   caption_bubble_controllers_[browser]->OnTranscription(transcription_result,
                                                         web_contents);
 }
 
-CaptionBubbleController*
-CaptionController::GetCaptionBubbleControllerForBrowser(Browser* browser) {
-  if (!browser || !caption_bubble_controllers_.count(browser))
-    return nullptr;
-  return caption_bubble_controllers_[browser].get();
-}
-
 void CaptionController::UpdateCaptionStyle() {
   PrefService* profile_prefs = profile_->GetPrefs();
   // Metrics are recorded when passing the caption prefs to the browser, so do

chrome/browser/accessibility/caption_controller.h

@@ -73,9 +73,6 @@ class CaptionController : public BrowserListObserver, public KeyedService {
       content::WebContents* web_contents,
       const chrome::mojom::TranscriptionResultPtr& transcription_result);
 
-  CaptionBubbleController* GetCaptionBubbleControllerForBrowser(
-      Browser* browser);
-
  private:
   friend class CaptionControllerFactory;
 

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

@@ -14,7 +14,6 @@
 #include "base/strings/string16.h"
 #include "build/build_config.h"
 #include "chrome/browser/accessibility/caption_controller.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/vector_icons/vector_icons.h"
 #include "third_party/re2/src/re2/re2.h"
@@ -155,20 +154,16 @@ class CaptionBubbleFrameView : public views::BubbleFrameView {
 };
 
 CaptionBubble::CaptionBubble(views::View* anchor,
-                             BrowserView* browser_view,
                              base::OnceClosure destroyed_callback)
     : BubbleDialogDelegateView(anchor,
                                views::BubbleBorder::FLOAT,
                                views::BubbleBorder::Shadow::NO_SHADOW),
       destroyed_callback_(std::move(destroyed_callback)),
       ratio_in_parent_x_(kDefaultRatioInParentX),
-      ratio_in_parent_y_(kDefaultRatioInParentY),
-      browser_view_(browser_view) {
+      ratio_in_parent_y_(kDefaultRatioInParentY) {
   SetButtons(ui::DIALOG_BUTTON_NONE);
   set_draggable(true);
   AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
-  AddAccelerator(ui::Accelerator(ui::VKEY_F6, ui::EF_NONE));
-  AddAccelerator(ui::Accelerator(ui::VKEY_F6, ui::EF_SHIFT_DOWN));
   // The CaptionBubble is focusable. It will alert the CaptionBubbleFrameView
   // when its focus changes so that the focus ring can be updated.
   // TODO(crbug.com/1055150): Consider using
@@ -383,25 +378,14 @@ void CaptionBubble::OnKeyEvent(ui::KeyEvent* event) {
 }
 
 bool CaptionBubble::AcceleratorPressed(const ui::Accelerator& accelerator) {
-  if (accelerator.key_code() == ui::VKEY_ESCAPE) {
-    // We don't want to close when the user hits "escape", because this isn't a
-    // normal dialog bubble -- it's meant to be up all the time. We just want to
-    // release focus back to the page in that case.
-    // Users should use the "close" button to close the bubble.
-    GetAnchorView()->RequestFocus();
-    GetAnchorView()->GetWidget()->Activate();
-    return true;
-  }
-  if (accelerator.key_code() == ui::VKEY_F6) {
-    // F6 rotates focus through the panes in the browser. Use
-    // BrowserView::AcceleratorPressed so that metrics are logged appropriately.
-    browser_view_->AcceleratorPressed(accelerator);
-    // Remove focus from this widget.
-    browser_view_->GetWidget()->Activate();
-    return true;
-  }
-  NOTREACHED();
-  return false;
+  DCHECK_EQ(accelerator.key_code(), ui::VKEY_ESCAPE);
+  // We don't want to close when the user hits "escape", because this isn't a
+  // normal dialog bubble -- it's meant to be up all the time. We just want to
+  // release focus back to the page in that case.
+  // Users should use the "close" button to close the bubble.
+  // TODO(crbug.com/1055150): This doesn't work in Mac.
+  GetAnchorView()->RequestFocus();
+  return true;
 }
 
 void CaptionBubble::OnFocus() {
@@ -514,10 +498,6 @@ void CaptionBubble::Hide() {
   UpdateBubbleVisibility();
 }
 
-const char* CaptionBubble::GetClassName() const {
-  return "CaptionBubble";
-}
-
 double CaptionBubble::GetTextScaleFactor() {
   double textScaleFactor = 1;
   if (caption_style_) {

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

@@ -21,8 +21,6 @@ namespace ui {
 struct AXNodeData;
 }
 
-class BrowserView;
-
 namespace captions {
 class CaptionBubbleFrameView;
 
@@ -35,9 +33,7 @@ class CaptionBubbleFrameView;
 class CaptionBubble : public views::BubbleDialogDelegateView,
                       public views::ButtonListener {
  public:
-  CaptionBubble(views::View* anchor,
-                BrowserView* browser_view,
-                base::OnceClosure destroyed_callback);
+  CaptionBubble(views::View* anchor, base::OnceClosure destroyed_callback);
   ~CaptionBubble() override;
   CaptionBubble(const CaptionBubble&) = delete;
   CaptionBubble& operator=(const CaptionBubble&) = delete;
@@ -60,8 +56,6 @@ class CaptionBubble : public views::BubbleDialogDelegateView,
   // directly.
   void Hide();
 
-  const char* GetClassName() const override;
-
  protected:
   // views::BubbleDialogDelegateView:
   void Init() override;
@@ -119,9 +113,6 @@ class CaptionBubble : public views::BubbleDialogDelegateView,
 
   // Whether we should show the widget. False if explicitly asked to hide.
   bool should_show_ = true;
-
-  // A reference to the BrowserView holding this bubble. Unowned.
-  BrowserView* browser_view_;
 };
 
 }  // namespace captions

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

@@ -6,8 +6,6 @@
 
 #include <memory>
 
-#include "chrome/browser/accessibility/caption_controller.h"
-#include "chrome/browser/accessibility/caption_controller_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/views/accessibility/caption_bubble.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -21,26 +19,11 @@ std::unique_ptr<CaptionBubbleController> CaptionBubbleController::Create(
   return std::make_unique<CaptionBubbleControllerViews>(browser);
 }
 
-// Static
-views::View* CaptionBubbleControllerViews::GetCaptionBubbleAccessiblePane(
-    Browser* browser) {
-  CaptionController* caption_controller =
-      CaptionControllerFactory::GetForProfileIfExists(browser->profile());
-  if (caption_controller) {
-    CaptionBubbleControllerViews* bubble_controller =
-        static_cast<CaptionBubbleControllerViews*>(
-            caption_controller->GetCaptionBubbleControllerForBrowser(browser));
-    if (bubble_controller)
-      return bubble_controller->GetFocusableCaptionBubble();
-  }
-  return nullptr;
-}
-
 CaptionBubbleControllerViews::CaptionBubbleControllerViews(Browser* browser)
     : CaptionBubbleController(browser), browser_(browser) {
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser_);
   caption_bubble_ = new CaptionBubble(
-      browser_view->GetContentsView(), browser_view,
+      browser_view->GetContentsView(),
       base::BindOnce(&CaptionBubbleControllerViews::OnCaptionBubbleDestroyed,
                      base::Unretained(this)));
   caption_widget_ =
@@ -118,10 +101,4 @@ void CaptionBubbleControllerViews::UpdateCaptionStyle(
   caption_bubble_->UpdateCaptionStyle(caption_style);
 }
 
-views::View* CaptionBubbleControllerViews::GetFocusableCaptionBubble() {
-  if (caption_widget_ && caption_widget_->IsVisible())
-    return caption_bubble_;
-  return nullptr;
-}
-
 }  // namespace captions

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

@@ -12,7 +12,6 @@
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 
 namespace views {
-class View;
 class Widget;
 }
 
@@ -37,8 +36,6 @@ struct CaptionText {
 class CaptionBubbleControllerViews : public CaptionBubbleController,
                                      public TabStripModelObserver {
  public:
-  static views::View* GetCaptionBubbleAccessiblePane(Browser* browser);
-
   explicit CaptionBubbleControllerViews(Browser* browser);
   ~CaptionBubbleControllerViews() override;
   CaptionBubbleControllerViews(const CaptionBubbleControllerViews&) = delete;
@@ -54,10 +51,6 @@ class CaptionBubbleControllerViews : public CaptionBubbleController,
   void UpdateCaptionStyle(
       base::Optional<ui::CaptionStyle> caption_style) override;
 
-  // Returns the view of the caption bubble which should receive focus, if one
-  // exists.
-  views::View* GetFocusableCaptionBubble();
-
  private:
   friend class CaptionBubbleControllerViewsTest;
 
@@ -88,6 +81,7 @@ class CaptionBubbleControllerViews : public CaptionBubbleController,
   // transcription in a while.
   std::unordered_map<content::WebContents*, CaptionText> caption_texts_;
 };
+
 }  // namespace captions
 
 #endif  // CHROME_BROWSER_UI_VIEWS_ACCESSIBILITY_CAPTION_BUBBLE_CONTROLLER_VIEWS_H_

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

@@ -392,6 +392,8 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
                                               false, false, false));
   EXPECT_EQ(bounds, GetCaptionWidget()->GetClientAreaBoundsInScreen());
 
+#if !defined(OS_MACOSX)
+  // TODO(crbug.com/1055150): Get this working for Mac.
   // Hitting the escape key should remove focus from the view, so arrows no
   // longer work.
   EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_ESCAPE, false,
@@ -400,6 +402,7 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
   EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_UP, false,
                                               false, false, false));
   EXPECT_EQ(bounds, GetCaptionWidget()->GetClientAreaBoundsInScreen());
+#endif
 }
 
 IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, FocusableInTabOrder) {
@@ -412,10 +415,9 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, FocusableInTabOrder) {
   EXPECT_FALSE(GetBubble()->GetFocusManager()->GetFocusedView());
 
   // Press tab until we enter the bubble.
-  while (!GetBubble()->HasFocus()) {
+  while (!GetBubble()->HasFocus())
     EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_TAB, false,
                                                 false, false, false));
-  }
 #if defined(USE_AURA) && !defined(OS_CHROMEOS)
   // Check the native widget has focus.
   aura::client::FocusClient* focus_client =

chrome/browser/ui/views/frame/browser_view.cc

@@ -75,7 +75,6 @@
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/accelerator_table.h"
 #include "chrome/browser/ui/views/accessibility/accessibility_focus_highlight.h"
-#include "chrome/browser/ui/views/accessibility/caption_bubble_controller_views.h"
 #include "chrome/browser/ui/views/accessibility/invert_bubble_view.h"
 #include "chrome/browser/ui/views/autofill/autofill_bubble_handler_impl.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
@@ -2559,12 +2558,6 @@ void BrowserView::GetAccessiblePanes(std::vector<views::View*>* panes) {
     panes->push_back(infobar_container_);
   if (download_shelf_.get())
     panes->push_back(download_shelf_.get());
-  // See if there is a caption bubble present.
-  views::View* caption_bubble =
-      captions::CaptionBubbleControllerViews::GetCaptionBubbleAccessiblePane(
-          browser());
-  if (caption_bubble)
-    panes->push_back(caption_bubble);
   panes->push_back(contents_web_view_);
   if (devtools_web_view_->GetVisible())
     panes->push_back(devtools_web_view_);

chrome/browser/ui/views/frame/browser_view_browsertest.cc

@@ -7,19 +7,15 @@
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
-#include "chrome/browser/accessibility/caption_controller.h"
-#include "chrome/browser/accessibility/caption_controller_factory.h"
 #include "chrome/browser/devtools/devtools_window_testing.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tab_modal_confirm_dialog.h"
 #include "chrome/browser/ui/tab_ui_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/ui/views/accessibility/caption_bubble_controller_views.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view_observer.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -32,21 +28,13 @@
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
-#include "media/base/media_switches.h"
 #include "ui/accessibility/platform/ax_platform_node.h"
 #include "ui/accessibility/platform/ax_platform_node_test_helper.h"
 #include "ui/base/l10n/l10n_util.h"
 
-#if defined(USE_AURA)
-#include "ui/aura/client/focus_client.h"
-#include "ui/views/widget/native_widget_aura.h"
-#endif  // USE_AURA
-
 class BrowserViewTest : public InProcessBrowserTest {
  public:
-  BrowserViewTest() : devtools_(nullptr) {
-    scoped_feature_list_.InitAndEnableFeature(media::kLiveCaption);
-  }
+  BrowserViewTest() : InProcessBrowserTest(), devtools_(nullptr) {}
 
  protected:
   BrowserView* browser_view() {
@@ -77,8 +65,6 @@ class BrowserViewTest : public InProcessBrowserTest {
   DevToolsWindow* devtools_;
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-
   DISALLOW_COPY_AND_ASSIGN(BrowserViewTest);
 };
 
@@ -378,55 +364,3 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, GetAccessibleTabModalDialogTree) {
 }
 
 #endif  // !defined(OS_MACOSX)
-
-IN_PROC_BROWSER_TEST_F(BrowserViewTest, F6CyclesThroughCaptionBubbleToo) {
-  captions::CaptionController* caption_controller =
-      captions::CaptionControllerFactory::GetForProfileIfExists(
-          browser()->profile());
-  caption_controller->Init();
-  browser()->profile()->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled,
-                                               true);
-  // No bubble is shown until a transcription happens.
-  captions::CaptionBubbleControllerViews* bubble_controller =
-      static_cast<captions::CaptionBubbleControllerViews*>(
-          caption_controller->GetCaptionBubbleControllerForBrowser(browser()));
-  EXPECT_FALSE(bubble_controller->GetFocusableCaptionBubble());
-
-  caption_controller->DispatchTranscription(
-      browser()->tab_strip_model()->GetActiveWebContents(),
-      chrome::mojom::TranscriptionResult::New("Hello, world", false));
-  // Now the caption bubble exists but is not focused.
-  views::View* bubble = bubble_controller->GetFocusableCaptionBubble();
-  EXPECT_TRUE(bubble);
-  EXPECT_TRUE(bubble->GetWidget()->IsVisible());
-  EXPECT_FALSE(bubble->HasFocus());
-  EXPECT_FALSE(bubble->GetFocusManager()->GetFocusedView());
-
-  // Press F6 until we enter the bubble.
-  while (!bubble->HasFocus()) {
-    EXPECT_TRUE(
-        browser_view()->AcceleratorPressed(ui::Accelerator(ui::VKEY_F6, 0)));
-  }
-
-#if defined(USE_AURA) && !defined(OS_CHROMEOS)
-  // Check the native widget has focus.
-  aura::client::FocusClient* focus_client =
-      aura::client::GetFocusClient(bubble->GetWidget()->GetNativeView());
-  EXPECT_TRUE(bubble->GetWidget()->GetNativeView() ==
-              focus_client->GetFocusedWindow());
-#endif
-
-  // F6 again exits the bubble. Because the bubble is focused, it gets the
-  // accelerator event.
-  EXPECT_TRUE(bubble->AcceleratorPressed(ui::Accelerator(ui::VKEY_F6, 0)));
-
-  // Now something else within the browser_view's focus manager is focused.
-  EXPECT_FALSE(bubble->HasFocus());
-  EXPECT_FALSE(bubble->GetFocusManager()->GetFocusedView());
-  EXPECT_TRUE(browser_view()->GetWidget()->GetFocusManager()->GetFocusedView());
-#if defined(USE_AURA) && !defined(OS_CHROMEOS)
-  // The bubble's native widget should no longer have focus.
-  EXPECT_FALSE(bubble->GetWidget()->GetNativeView() ==
-               focus_client->GetFocusedWindow());
-#endif
-}
\ No newline at end of file

ui/views/focus/focus_manager.cc

@@ -206,9 +206,7 @@ bool FocusManager::RotatePaneFocus(Direction direction,
       continue;
 
     pane->RequestFocus();
-    // |pane| may be in a different widget, so don't assume its focus manager
-    // is |this|.
-    focused_view = pane->GetWidget()->GetFocusManager()->GetFocusedView();
+    focused_view = GetFocusedView();
     if (pane == focused_view || pane->Contains(focused_view))
       return true;
   }