ash: make AccessibilityFocusRingController owned by Shell

Bug: None
Test: compiles & trybots
Change-Id: I29d5c39a018e6a2ddc0df3e123780f0bffd7aa99
Reviewed-on: https://chromium-review.googlesource.com/963650
Commit-Queue: Qiang Xu <warx@google.com>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543519}

ash/accessibility/accessibility_focus_ring_controller.cc

@@ -12,6 +12,8 @@
 #include <utility>
 #include <vector>
 
+#include "ash/accessibility/accessibility_cursor_ring_layer.h"
+#include "ash/accessibility/accessibility_focus_ring_layer.h"
 #include "ash/accessibility/accessibility_highlight_layer.h"
 #include "ash/accessibility/focus_ring_layer.h"
 #include "base/logging.h"
@@ -23,31 +25,31 @@ namespace {
 // The number of pixels the focus ring is outset from the object it outlines,
 // which also determines the border radius of the rounded corners.
 // TODO(dmazzoni): take display resolution into account.
-const int kAccessibilityFocusRingMargin = 7;
+constexpr int kAccessibilityFocusRingMargin = 7;
 
 // Time to transition between one location and the next.
-const int kTransitionTimeMilliseconds = 300;
+constexpr int kTransitionTimeMilliseconds = 300;
 
 // Focus constants.
-const int kFocusFadeInTimeMilliseconds = 100;
-const int kFocusFadeOutTimeMilliseconds = 1600;
+constexpr int kFocusFadeInTimeMilliseconds = 100;
+constexpr int kFocusFadeOutTimeMilliseconds = 1600;
 
 // Cursor constants.
-const int kCursorFadeInTimeMilliseconds = 400;
-const int kCursorFadeOutTimeMilliseconds = 1200;
-const int kCursorRingColorRed = 255;
-const int kCursorRingColorGreen = 51;
-const int kCursorRingColorBlue = 51;
+constexpr int kCursorFadeInTimeMilliseconds = 400;
+constexpr int kCursorFadeOutTimeMilliseconds = 1200;
+constexpr int kCursorRingColorRed = 255;
+constexpr int kCursorRingColorGreen = 51;
+constexpr int kCursorRingColorBlue = 51;
 
 // Caret constants.
-const int kCaretFadeInTimeMilliseconds = 100;
-const int kCaretFadeOutTimeMilliseconds = 1600;
-const int kCaretRingColorRed = 51;
-const int kCaretRingColorGreen = 51;
-const int kCaretRingColorBlue = 255;
+constexpr int kCaretFadeInTimeMilliseconds = 100;
+constexpr int kCaretFadeOutTimeMilliseconds = 1600;
+constexpr int kCaretRingColorRed = 51;
+constexpr int kCaretRingColorGreen = 51;
+constexpr int kCaretRingColorBlue = 255;
 
 // Highlight constants.
-const float kHighlightOpacity = 0.3f;
+constexpr float kHighlightOpacity = 0.3f;
 
 // A Region is an unordered collection of Rects that maintains its
 // bounding box. Used in the middle of an algorithm that groups
@@ -63,12 +65,6 @@ struct Region {
 
 }  // namespace
 
-// static
-AccessibilityFocusRingController*
-AccessibilityFocusRingController::GetInstance() {
-  return base::Singleton<AccessibilityFocusRingController>::get();
-}
-
 AccessibilityFocusRingController::AccessibilityFocusRingController()
     : binding_(this) {
   focus_animation_info_.fade_in_time =

ash/accessibility/accessibility_focus_ring_controller.h

@@ -8,14 +8,12 @@
 #include <memory>
 #include <vector>
 
-#include "ash/accessibility/accessibility_cursor_ring_layer.h"
-#include "ash/accessibility/accessibility_focus_ring_layer.h"
-#include "ash/accessibility/accessibility_highlight_layer.h"
+#include "ash/accessibility/accessibility_focus_ring.h"
+#include "ash/accessibility/accessibility_layer.h"
 #include "ash/ash_export.h"
 #include "ash/public/interfaces/accessibility_focus_ring_controller.mojom.h"
 #include "base/bind.h"
 #include "base/macros.h"
-#include "base/memory/singleton.h"
 #include "base/optional.h"
 #include "base/time/time.h"
 #include "mojo/public/cpp/bindings/binding.h"
@@ -24,14 +22,18 @@
 
 namespace ash {
 
+class AccessibilityCursorRingLayer;
+class AccessibilityFocusRingLayer;
+class AccessibilityHighlightLayer;
+
 // AccessibilityFocusRingController handles drawing custom rings around
 // the focused object, cursor, and/or caret for accessibility.
 class ASH_EXPORT AccessibilityFocusRingController
     : public AccessibilityLayerDelegate,
       public mojom::AccessibilityFocusRingController {
  public:
-  // Get the single instance of this class.
-  static AccessibilityFocusRingController* GetInstance();
+  AccessibilityFocusRingController();
+  ~AccessibilityFocusRingController() override;
 
   // Binds the mojom::AccessibilityFocusRingController interface to this object.
   void BindRequest(mojom::AccessibilityFocusRingControllerRequest request);
@@ -68,14 +70,8 @@ class ASH_EXPORT AccessibilityFocusRingController
   focus_ring_layers_for_testing() {
     return focus_layers_;
   }
-  AccessibilityHighlightLayer* highlight_layer_for_testing() {
-    return highlight_layer_.get();
-  }
 
  protected:
-  AccessibilityFocusRingController();
-  ~AccessibilityFocusRingController() override;
-
   // Given an unordered vector of bounding rectangles that cover everything
   // that currently has focus, populate a vector of one or more
   // AccessibilityFocusRings that surround the rectangles. Adjacent or
@@ -151,8 +147,6 @@ class ASH_EXPORT AccessibilityFocusRingController
   SkColor highlight_color_ = SK_ColorBLACK;
   float highlight_opacity_ = 0.f;
 
-  friend struct base::DefaultSingletonTraits<AccessibilityFocusRingController>;
-
   DISALLOW_COPY_AND_ASSIGN(AccessibilityFocusRingController);
 };
 

ash/accessibility/accessibility_focus_ring_controller_unittest.cc

@@ -148,7 +148,7 @@ TEST_F(AccessibilityFocusRingControllerTest, CursorWorksOnMultipleDisplays) {
 
   // Simulate a mouse event on the primary display.
   AccessibilityFocusRingController* controller =
-      AccessibilityFocusRingController::GetInstance();
+      Shell::Get()->accessibility_focus_ring_controller();
   gfx::Point location(90, 90);
   controller->SetCursorRing(location);
   AccessibilityCursorRingLayer* cursor_layer =

ash/accessibility/accessibility_highlight_controller.cc

@@ -42,10 +42,12 @@ AccessibilityHighlightController::AccessibilityHighlightController() {
 }
 
 AccessibilityHighlightController::~AccessibilityHighlightController() {
-  AccessibilityFocusRingController::GetInstance()->SetFocusRing(
-      std::vector<gfx::Rect>(), mojom::FocusRingBehavior::FADE_OUT_FOCUS_RING);
-  AccessibilityFocusRingController::GetInstance()->HideCaretRing();
-  AccessibilityFocusRingController::GetInstance()->HideCursorRing();
+  AccessibilityFocusRingController* controller =
+      Shell::Get()->accessibility_focus_ring_controller();
+  controller->SetFocusRing(std::vector<gfx::Rect>(),
+                           mojom::FocusRingBehavior::FADE_OUT_FOCUS_RING);
+  controller->HideCaretRing();
+  controller->HideCursorRing();
 
   aura::Window* root_window = Shell::GetPrimaryRootWindow();
   ui::InputMethod* input_method = GetInputMethod(root_window);
@@ -141,7 +143,8 @@ bool AccessibilityHighlightController::IsCaretVisible(
 }
 
 void AccessibilityHighlightController::UpdateFocusAndCaretHighlights() {
-  auto* controller = AccessibilityFocusRingController::GetInstance();
+  AccessibilityFocusRingController* controller =
+      Shell::Get()->accessibility_focus_ring_controller();
 
   // The caret highlight takes precedence over the focus highlight if
   // both are visible.
@@ -164,12 +167,12 @@ void AccessibilityHighlightController::UpdateFocusAndCaretHighlights() {
 }
 
 void AccessibilityHighlightController::UpdateCursorHighlight() {
-  if (cursor_ && IsCursorVisible()) {
-    AccessibilityFocusRingController::GetInstance()->SetCursorRing(
-        cursor_point_);
-  } else {
-    AccessibilityFocusRingController::GetInstance()->HideCursorRing();
-  }
+  AccessibilityFocusRingController* controller =
+      Shell::Get()->accessibility_focus_ring_controller();
+  if (cursor_ && IsCursorVisible())
+    controller->SetCursorRing(cursor_point_);
+  else
+    controller->HideCursorRing();
 }
 
 }  // namespace ash

ash/accessibility/accessibility_highlight_controller_unittest.cc

@@ -5,6 +5,7 @@
 #include "ash/accessibility/accessibility_highlight_controller.h"
 
 #include <cmath>
+#include <memory>
 
 #include "ash/accessibility/accessibility_cursor_ring_layer.h"
 #include "ash/accessibility/accessibility_focus_ring_controller.h"
@@ -72,7 +73,7 @@ class AccessibilityHighlightControllerTest : public AshTestBase {
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         ::switches::kEnablePixelOutputInTests);
     AshTestBase::SetUp();
-    AccessibilityFocusRingController::GetInstance()->SetNoFadeForTesting();
+    Shell::Get()->accessibility_focus_ring_controller()->SetNoFadeForTesting();
   }
 
   void CaptureBeforeImage(const gfx::Rect& bounds) {
@@ -226,7 +227,8 @@ TEST_F(AccessibilityHighlightControllerTest, CursorWorksOnMultipleDisplays) {
   event_mod.set_target(root_windows[0]);
   highlight_controller.OnMouseEvent(&event0);
 
-  auto* focus_ring_controller = AccessibilityFocusRingController::GetInstance();
+  AccessibilityFocusRingController* focus_ring_controller =
+      Shell::Get()->accessibility_focus_ring_controller();
   auto* cursor_layer = focus_ring_controller->cursor_layer_for_testing();
   EXPECT_EQ(root_windows[0], cursor_layer->root_window());
   EXPECT_LT(
@@ -267,7 +269,8 @@ TEST_F(AccessibilityHighlightControllerTest, CaretRingDrawnOnlyWithinBounds) {
   text_input_client.SetCaretBounds(caret_bounds);
   highlight_controller.OnCaretBoundsChanged(&text_input_client);
 
-  auto* focus_ring_controller = AccessibilityFocusRingController::GetInstance();
+  AccessibilityFocusRingController* focus_ring_controller =
+      Shell::Get()->accessibility_focus_ring_controller();
   auto* caret_layer = focus_ring_controller->caret_layer_for_testing();
   EXPECT_EQ(
       std::abs(caret_layer->layer()->GetTargetBounds().x() - caret_bounds.x()),

ash/accessibility/touch_exploration_manager.cc

@@ -212,7 +212,7 @@ void TouchExplorationManager::UpdateTouchExplorationState() {
       touch_exploration_controller_->SetExcludeBounds(work_area);
       SilenceSpokenFeedback();
       // Clear the focus highlight.
-      AccessibilityFocusRingController::GetInstance()->SetFocusRing(
+      Shell::Get()->accessibility_focus_ring_controller()->SetFocusRing(
           std::vector<gfx::Rect>(),
           mojom::FocusRingBehavior::PERSIST_FOCUS_RING);
     } else {

ash/mojo_interface_factory.cc

@@ -60,7 +60,7 @@ void BindAccessibilityControllerRequestOnMainThread(
 
 void BindAccessibilityFocusRingControllerRequestOnMainThread(
     mojom::AccessibilityFocusRingControllerRequest request) {
-  AccessibilityFocusRingController::GetInstance()->BindRequest(
+  Shell::Get()->accessibility_focus_ring_controller()->BindRequest(
       std::move(request));
 }
 

ash/shell.cc

@@ -16,6 +16,7 @@
 #include "ash/accelerators/spoken_feedback_toggler.h"
 #include "ash/accessibility/accessibility_controller.h"
 #include "ash/accessibility/accessibility_delegate.h"
+#include "ash/accessibility/accessibility_focus_ring_controller.h"
 #include "ash/app_list/app_list_controller_impl.h"
 #include "ash/ash_constants.h"
 #include "ash/autoclick/autoclick_controller.h"
@@ -808,6 +809,7 @@ Shell::~Shell() {
   wallpaper_controller_.reset();
   accessibility_controller_.reset();
   accessibility_delegate_.reset();
+  accessibility_focus_ring_controller_.reset();
 
   // Balances the Install() in Initialize().
   views::FocusManagerFactory::Install(nullptr);
@@ -914,6 +916,8 @@ void Shell::Init(ui::ContextFactory* context_factory,
 
   // Some delegates access ShellPort during their construction. Create them here
   // instead of the ShellPort constructor.
+  accessibility_focus_ring_controller_ =
+      std::make_unique<AccessibilityFocusRingController>();
   accessibility_delegate_.reset(shell_delegate_->CreateAccessibilityDelegate());
   accessibility_controller_ = std::make_unique<AccessibilityController>(
       shell_delegate_->GetShellConnector());

ash/shell.h

@@ -77,6 +77,7 @@ namespace ash {
 class AcceleratorController;
 class AccessibilityController;
 class AccessibilityDelegate;
+class AccessibilityFocusRingController;
 class AshDisplayController;
 class AppListControllerImpl;
 class NativeCursorManagerAsh;
@@ -307,6 +308,9 @@ class ASH_EXPORT Shell : public SessionObserver,
   AccessibilityDelegate* accessibility_delegate() {
     return accessibility_delegate_.get();
   }
+  AccessibilityFocusRingController* accessibility_focus_ring_controller() {
+    return accessibility_focus_ring_controller_.get();
+  }
   ::wm::ActivationClient* activation_client();
   AppListControllerImpl* app_list_controller() {
     return app_list_controller_.get();
@@ -659,6 +663,8 @@ class ASH_EXPORT Shell : public SessionObserver,
   std::unique_ptr<AcceleratorController> accelerator_controller_;
   std::unique_ptr<AccessibilityController> accessibility_controller_;
   std::unique_ptr<AccessibilityDelegate> accessibility_delegate_;
+  std::unique_ptr<AccessibilityFocusRingController>
+      accessibility_focus_ring_controller_;
   std::unique_ptr<AppListControllerImpl> app_list_controller_;
   std::unique_ptr<AshDisplayController> ash_display_controller_;
   std::unique_ptr<BacklightsForcedOffSetter> backlights_forced_off_setter_;

chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc

@@ -196,7 +196,7 @@ IN_PROC_BROWSER_TEST_F(SelectToSpeakTest, FocusRingMovesWithMouse) {
   chromeos::AccessibilityManager::Get()->SetFocusRingObserverForTest(callback);
 
   ash::AccessibilityFocusRingController* controller =
-      ash::AccessibilityFocusRingController::GetInstance();
+      ash::Shell::Get()->accessibility_focus_ring_controller();
   std::vector<std::unique_ptr<ash::AccessibilityFocusRingLayer>> const&
       focus_rings = controller->focus_ring_layers_for_testing();