mash: move notification of braille display to ash

changes:
- OnBrailleDisplayStateChanged may set spoken feedback enabled, and also
  trigger showing notification. Move this part of code to ash.
- Move TrayAccessibilityTest.ShowNotification test from browser test
  to ash test.

Bug: 594887
Test: Tested on device connects/disconnects braille display.
Change-Id: I2c34c5fe33614b9e1617664661f38fc29820e750
Reviewed-on: https://chromium-review.googlesource.com/875206
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530779}

ash/accessibility/accessibility_controller.cc

@@ -197,6 +197,13 @@ void AccessibilityController::SetDarkenScreen(bool darken) {
   }
 }
 
+void AccessibilityController::BrailleDisplayStateChanged(bool connected) {
+  braille_display_connected_ = connected;
+  if (braille_display_connected_)
+    SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
+  NotifyAccessibilityStatusChanged(A11Y_NOTIFICATION_SHOW);
+}
+
 void AccessibilityController::OnSigninScreenPrefServiceInitialized(
     PrefService* prefs) {
   ObservePrefs(prefs);

ash/accessibility/accessibility_controller.h

@@ -61,6 +61,8 @@ class ASH_EXPORT AccessibilityController
                                 AccessibilityNotificationVisibility notify);
   bool IsSpokenFeedbackEnabled() const;
 
+  bool braille_display_connected() const { return braille_display_connected_; }
+
   // Triggers an accessibility alert to give the user feedback.
   void TriggerAccessibilityAlert(mojom::AccessibilityAlert alert);
 
@@ -83,6 +85,7 @@ class ASH_EXPORT AccessibilityController
   // mojom::AccessibilityController:
   void SetClient(mojom::AccessibilityControllerClientPtr client) override;
   void SetDarkenScreen(bool darken) override;
+  void BrailleDisplayStateChanged(bool connected) override;
 
   // SessionObserver:
   void OnSigninScreenPrefServiceInitialized(PrefService* prefs) override;
@@ -125,6 +128,7 @@ class ASH_EXPORT AccessibilityController
   int large_cursor_size_in_dip_ = kDefaultLargeCursorSize;
   bool mono_audio_enabled_ = false;
   bool spoken_feedback_enabled_ = false;
+  bool braille_display_connected_ = false;
 
   // TODO(warx): consider removing this and replacing it with a more reliable
   // way (https://crbug.com/800270).

ash/accessibility/accessibility_delegate.h

@@ -78,9 +78,6 @@ class ASH_EXPORT AccessibilityDelegate {
   // Returns true when the accessibility menu should be shown.
   virtual bool ShouldShowAccessibilityMenu() const = 0;
 
-  // Returns true if a braille display is connected to the system.
-  virtual bool IsBrailleDisplayConnected() const = 0;
-
   // Cancel all current and queued speech immediately.
   virtual void SilenceSpokenFeedback() const = 0;
 

ash/accessibility/default_accessibility_delegate.cc

@@ -97,10 +97,6 @@ bool DefaultAccessibilityDelegate::ShouldShowAccessibilityMenu() const {
          controller->IsHighContrastEnabled();
 }
 
-bool DefaultAccessibilityDelegate::IsBrailleDisplayConnected() const {
-  return false;
-}
-
 void DefaultAccessibilityDelegate::SilenceSpokenFeedback() const {}
 
 void DefaultAccessibilityDelegate::SaveScreenMagnifierScale(double scale) {}

ash/accessibility/default_accessibility_delegate.h

@@ -36,7 +36,6 @@ class ASH_EXPORT DefaultAccessibilityDelegate : public AccessibilityDelegate {
   void SetSwitchAccessEnabled(bool enabled) override;
   bool IsSwitchAccessEnabled() const override;
   bool ShouldShowAccessibilityMenu() const override;
-  bool IsBrailleDisplayConnected() const override;
   void SilenceSpokenFeedback() const override;
   void SaveScreenMagnifierScale(double scale) override;
   double GetSavedScreenMagnifierScale() override;

ash/public/interfaces/accessibility_controller.mojom

@@ -46,6 +46,9 @@ interface AccessibilityController {
   // Starts or stops darkening the screen (e.g. to allow chrome a11y extensions
   // to darken the screen).
   SetDarkenScreen(bool darken);
+
+  // Called when braille display state is changed.
+  BrailleDisplayStateChanged(bool connected);
 };
 
 // Interface for ash to request accessibility service from its client (e.g.

ash/system/tray_accessibility.cc

@@ -75,7 +75,7 @@ uint32_t GetAccessibilityState() {
     state |= A11Y_AUTOCLICK;
   if (delegate->IsVirtualKeyboardEnabled())
     state |= A11Y_VIRTUAL_KEYBOARD;
-  if (delegate->IsBrailleDisplayConnected())
+  if (controller->braille_display_connected())
     state |= A11Y_BRAILLE_DISPLAY_CONNECTED;
   if (controller->IsMonoAudioEnabled())
     state |= A11Y_MONO_AUDIO;

ash/system/tray_accessibility_unittest.cc

@@ -82,7 +82,7 @@ TEST_F(TrayAccessibilityTest, ShowNotificationOnSpokenFeedback) {
   controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
   message_center::NotificationList::Notifications notifications =
       MessageCenter::Get()->GetVisibleNotifications();
-  EXPECT_EQ(1u, notifications.size());
+  ASSERT_EQ(1u, notifications.size());
   EXPECT_EQ(kChromeVoxEnabledTitle, (*notifications.begin())->title());
   EXPECT_EQ(kChromeVoxEnabled, (*notifications.begin())->message());
 
@@ -92,5 +92,47 @@ TEST_F(TrayAccessibilityTest, ShowNotificationOnSpokenFeedback) {
   EXPECT_EQ(0u, notifications.size());
 }
 
+TEST_F(TrayAccessibilityTest, ShowNotificationOnBrailleDisplayStateChanged) {
+  const base::string16 kBrailleConnected =
+      base::ASCIIToUTF16("Braille display connected.");
+  const base::string16 kChromeVoxEnabled =
+      base::ASCIIToUTF16("Press Ctrl + Alt + Z to disable spoken feedback.");
+  const base::string16 kBrailleConnectedAndChromeVoxEnabledTitle =
+      base::ASCIIToUTF16("Braille and ChromeVox are enabled");
+  AccessibilityController* controller =
+      Shell::Get()->accessibility_controller();
+
+  controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
+  EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
+  // Connecting a braille display when spoken feedback is already enabled
+  // should only show the message about the braille display.
+  controller->BrailleDisplayStateChanged(true);
+  message_center::NotificationList::Notifications notifications =
+      MessageCenter::Get()->GetVisibleNotifications();
+  ASSERT_EQ(1u, notifications.size());
+  EXPECT_EQ(base::string16(), (*notifications.begin())->title());
+  EXPECT_EQ(kBrailleConnected, (*notifications.begin())->message());
+
+  // Neither disconnecting a braille display, nor disabling spoken feedback
+  // should show any notification.
+  controller->BrailleDisplayStateChanged(false);
+  EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
+  notifications = MessageCenter::Get()->GetVisibleNotifications();
+  EXPECT_EQ(0u, notifications.size());
+  controller->SetSpokenFeedbackEnabled(false, A11Y_NOTIFICATION_SHOW);
+  notifications = MessageCenter::Get()->GetVisibleNotifications();
+  EXPECT_EQ(0u, notifications.size());
+  EXPECT_FALSE(controller->IsSpokenFeedbackEnabled());
+
+  // Connecting a braille display should enable spoken feedback and show
+  // both messages.
+  controller->BrailleDisplayStateChanged(true);
+  EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
+  notifications = MessageCenter::Get()->GetVisibleNotifications();
+  EXPECT_EQ(kBrailleConnectedAndChromeVoxEnabledTitle,
+            (*notifications.begin())->title());
+  EXPECT_EQ(kChromeVoxEnabled, (*notifications.begin())->message());
+}
+
 }  // namespace
 }  // namespace ash

chrome/browser/chromeos/accessibility/accessibility_manager.cc

@@ -11,7 +11,6 @@
 #include <utility>
 #include <vector>
 
-#include "ash/accessibility/accessibility_controller.h"
 #include "ash/accessibility/accessibility_focus_ring_controller.h"
 #include "ash/autoclick/autoclick_controller.h"
 #include "ash/autoclick/mus/public/interfaces/autoclick.mojom.h"
@@ -1254,15 +1253,6 @@ void AccessibilityManager::ActiveUserChanged(
     SetProfile(ProfileManager::GetActiveUserProfile());
 }
 
-void AccessibilityManager::SetProfileForTest(Profile* profile) {
-  SetProfile(profile);
-}
-
-void AccessibilityManager::SetBrailleControllerForTest(
-    BrailleController* controller) {
-  g_braille_controller_for_test = controller;
-}
-
 base::TimeDelta AccessibilityManager::PlayShutdownSound() {
   if (!PlayEarcon(SOUND_SHUTDOWN,
                   PlaySoundOption::ONLY_IF_SPOKEN_FEEDBACK_ENABLED)) {
@@ -1396,21 +1386,9 @@ void AccessibilityManager::Observe(
 void AccessibilityManager::OnBrailleDisplayStateChanged(
     const DisplayState& display_state) {
   braille_display_connected_ = display_state.available;
-  if (braille_display_connected_) {
-    // TODO(crbug.com/594887): Fix for mash by moving notifying accessibility
-    // status change to ash for BrailleDisplayStateChanged.
-    if (GetAshConfig() == ash::Config::MASH)
-      return;
-
-    ash::Shell::Get()->accessibility_controller()->SetSpokenFeedbackEnabled(
-        true, ash::A11Y_NOTIFICATION_SHOW);
-  }
+  accessibility_controller_->BrailleDisplayStateChanged(
+      braille_display_connected_);
   UpdateBrailleImeState();
-
-  AccessibilityStatusEventDetails details(
-      ACCESSIBILITY_BRAILLE_DISPLAY_CONNECTION_STATE_CHANGED,
-      braille_display_connected_, ash::A11Y_NOTIFICATION_SHOW);
-  NotifyAccessibilityStatusChanged(details);
 }
 
 void AccessibilityManager::OnBrailleKeyEvent(const KeyEvent& event) {
@@ -1549,4 +1527,18 @@ void AccessibilityManager::ToggleDictation() {
   dictation_->OnToggleDictation();
 }
 
+void AccessibilityManager::SetProfileForTest(Profile* profile) {
+  SetProfile(profile);
+}
+
+// static
+void AccessibilityManager::SetBrailleControllerForTest(
+    BrailleController* controller) {
+  g_braille_controller_for_test = controller;
+}
+
+void AccessibilityManager::FlushForTesting() {
+  accessibility_controller_.FlushForTesting();
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/accessibility/accessibility_manager.h

@@ -55,7 +55,6 @@ enum AccessibilityNotificationType {
   ACCESSIBILITY_TOGGLE_CURSOR_HIGHLIGHT,
   ACCESSIBILITY_TOGGLE_FOCUS_HIGHLIGHT,
   ACCESSIBILITY_TOGGLE_TAP_DRAGGING,
-  ACCESSIBILITY_BRAILLE_DISPLAY_CONNECTION_STATE_CHANGED
 };
 
 struct AccessibilityStatusEventDetails {
@@ -227,11 +226,6 @@ class AccessibilityManager
   // user_manager::UserManager::UserSessionStateObserver overrides:
   void ActiveUserChanged(const user_manager::User* active_user) override;
 
-  void SetProfileForTest(Profile* profile);
-
-  static void SetBrailleControllerForTest(
-      extensions::api::braille_display_private::BrailleController* controller);
-
   // Initiates play of shutdown sound and returns it's duration.
   base::TimeDelta PlayShutdownSound();
 
@@ -305,6 +299,12 @@ class AccessibilityManager
   // Starts or stops dictation (type what you speak).
   void ToggleDictation();
 
+  // Test helpers:
+  void SetProfileForTest(Profile* profile);
+  static void SetBrailleControllerForTest(
+      extensions::api::braille_display_private::BrailleController* controller);
+  void FlushForTesting();
+
  protected:
   AccessibilityManager();
   ~AccessibilityManager() override;

chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc

@@ -289,6 +289,8 @@ class AccessibilityManagerTest : public InProcessBrowserTest {
     ash::Shell::Get()->accessibility_controller()->SetPrefServiceForTest(
         profile->GetPrefs());
     default_autoclick_delay_ = GetAutoclickDelay();
+    // Spin the message loop to ensure the initial CheckBrailleState() is done.
+    base::RunLoop().RunUntilIdle();
   }
 
   void TearDownOnMainThread() override {
@@ -299,6 +301,7 @@ class AccessibilityManagerTest : public InProcessBrowserTest {
     braille_controller_.SetAvailable(available);
     braille_controller_.GetObserver()->OnBrailleDisplayStateChanged(
         *braille_controller_.GetDisplayState());
+    AccessibilityManager::Get()->FlushForTesting();
   }
 
   int default_autoclick_delay() const { return default_autoclick_delay_; }

chrome/browser/chromeos/system/tray_accessibility_browsertest.cc

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ash/accessibility/accessibility_controller.h"
 #include "ash/login_status.h"
 #include "ash/public/cpp/accessibility_types.h"
 #include "ash/public/cpp/ash_pref_names.h"
@@ -15,15 +14,13 @@
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/run_loop.h"
-#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/login/helper.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
-#include "chrome/browser/extensions/api/braille_display_private/mock_braille_controller.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/session_controller_client.h"
@@ -42,15 +39,10 @@
 #include "content/public/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
-#include "ui/message_center/message_center.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/widget/widget.h"
 
-using extensions::api::braille_display_private::BrailleObserver;
-using extensions::api::braille_display_private::DisplayState;
-using extensions::api::braille_display_private::MockBrailleController;
-using message_center::MessageCenter;
 using testing::Return;
 using testing::_;
 using testing::WithParamInterface;
@@ -146,12 +138,6 @@ class TrayAccessibilityTest
     EXPECT_CALL(provider_, IsInitializationComplete(_))
         .WillRepeatedly(Return(true));
     policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
-    AccessibilityManager::SetBrailleControllerForTest(&braille_controller_);
-  }
-
-  void TearDownOnMainThread() override {
-    AccessibilityManager::SetBrailleControllerForTest(nullptr);
-    InProcessBrowserTest::TearDownOnMainThread();
   }
 
   void SetShowAccessibilityOptionsInSystemTrayMenu(bool value) {
@@ -402,17 +388,10 @@ class TrayAccessibilityTest
            views::Button::STATE_NORMAL;
   }
 
-  void SetBrailleConnected(bool connected) {
-    braille_controller_.SetAvailable(connected);
-    braille_controller_.GetObserver()->OnBrailleDisplayStateChanged(
-        *braille_controller_.GetDisplayState());
-  }
-
   // Disable animations so that tray icons hide immediately.
   ui::ScopedAnimationDurationScaleMode disable_animations_;
 
   policy::MockConfigurationPolicyProvider provider_;
-  MockBrailleController braille_controller_;
 };
 
 using TrayAccessibilityLoginScreenTest = OobeBaseTest;
@@ -967,48 +946,6 @@ IN_PROC_BROWSER_TEST_P(TrayAccessibilityTest, ShowMenuWithShowOnLoginScreen) {
   EXPECT_TRUE(CanCreateMenuItem());
 }
 
-IN_PROC_BROWSER_TEST_P(TrayAccessibilityTest, ShowNotification) {
-  const base::string16 BRAILLE_CONNECTED =
-      base::ASCIIToUTF16("Braille display connected.");
-  const base::string16 CHROMEVOX_ENABLED =
-      base::ASCIIToUTF16("Press Ctrl + Alt + Z to disable spoken feedback.");
-  const base::string16 BRAILLE_CONNECTED_AND_CHROMEVOX_ENABLED_TITLE =
-      base::ASCIIToUTF16("Braille and ChromeVox are enabled");
-
-  EnableSpokenFeedback(true, ash::A11Y_NOTIFICATION_SHOW);
-  EXPECT_TRUE(AccessibilityManager::Get()->IsSpokenFeedbackEnabled());
-  // Connecting a braille display when spoken feedback is already enabled
-  // should only show the message about the braille display.
-  SetBrailleConnected(true);
-  message_center::NotificationList::Notifications notifications =
-      MessageCenter::Get()->GetVisibleNotifications();
-  EXPECT_EQ(1u, notifications.size());
-  EXPECT_EQ(base::string16(), (*notifications.begin())->title());
-  EXPECT_EQ(BRAILLE_CONNECTED, (*notifications.begin())->message());
-
-  // Neither disconnecting a braille display, nor disabling spoken feedback
-  // should show any notification.
-  SetBrailleConnected(false);
-  EXPECT_TRUE(AccessibilityManager::Get()->IsSpokenFeedbackEnabled());
-  notifications = MessageCenter::Get()->GetVisibleNotifications();
-  EXPECT_EQ(0u, notifications.size());
-  EnableSpokenFeedback(false, ash::A11Y_NOTIFICATION_SHOW);
-  notifications = MessageCenter::Get()->GetVisibleNotifications();
-  EXPECT_EQ(0u, notifications.size());
-  EXPECT_FALSE(AccessibilityManager::Get()->IsSpokenFeedbackEnabled());
-
-  // Connecting a braille display should enable spoken feedback and show
-  // both messages.
-  SetBrailleConnected(true);
-  // Spin the run loop to make sure ash see the change.
-  base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(AccessibilityManager::Get()->IsSpokenFeedbackEnabled());
-  notifications = MessageCenter::Get()->GetVisibleNotifications();
-  EXPECT_EQ(BRAILLE_CONNECTED_AND_CHROMEVOX_ENABLED_TITLE,
-            (*notifications.begin())->title());
-  EXPECT_EQ(CHROMEVOX_ENABLED, (*notifications.begin())->message());
-}
-
 IN_PROC_BROWSER_TEST_P(TrayAccessibilityTest, KeepMenuVisibilityOnLockScreen) {
   // Enables high contrast mode.
   EnableHighContrast(true);

chrome/browser/ui/ash/accessibility/accessibility_controller_client_unittest.cc

@@ -38,6 +38,7 @@ class TestAccessibilityController : ash::mojom::AccessibilityController {
     was_client_set_ = true;
   }
   void SetDarkenScreen(bool darken) override {}
+  void BrailleDisplayStateChanged(bool connected) override {}
 
   bool was_client_set() const { return was_client_set_; }