ChromeVox: Disable following a11y events after screen off alert is received

chrome/browser/ui/ash/chrome_shell_delegate.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/chrome_shell_delegate.h"
 
 #include <stddef.h>
 
 #include <limits>
 
@@ skipping 259 matching lines @@
   double GetSavedScreenMagnifierScale() override {
     if (chromeos::MagnificationManager::Get()) {
       return chromeos::MagnificationManager::Get()
           ->GetSavedScreenMagnifierScale();
     }
     return std::numeric_limits<double>::min();
   }
 
   void TriggerAccessibilityAlert(ash::AccessibilityAlert alert) override {
     Profile* profile = ProfileManager::GetActiveUserProfile();
+    bool screen_off_alert = false;
     if (profile) {
       int msg = 0;
       switch (alert) {
         case ash::A11Y_ALERT_CAPS_ON:
           msg = IDS_A11Y_ALERT_CAPS_ON;
           break;
         case ash::A11Y_ALERT_CAPS_OFF:
           msg = IDS_A11Y_ALERT_CAPS_OFF;
           break;
         case ash::A11Y_ALERT_SCREEN_ON:
+          SetAutomationManagerEnabled(profile, true);
           msg = IDS_A11Y_ALERT_SCREEN_ON;
           break;
         case ash::A11Y_ALERT_SCREEN_OFF:
+          screen_off_alert = true;
           msg = IDS_A11Y_ALERT_SCREEN_OFF;
           break;
         case ash::A11Y_ALERT_WINDOW_NEEDED:
           msg = IDS_A11Y_ALERT_WINDOW_NEEDED;
           break;
         case ash::A11Y_ALERT_WINDOW_OVERVIEW_MODE_ENTERED:
           msg = IDS_A11Y_ALERT_WINDOW_OVERVIEW_MODE_ENTERED;
           break;
         case ash::A11Y_ALERT_NONE:
           msg = 0;
           break;
       }
 
       if (msg) {
         AutomationManagerAura::GetInstance()->HandleAlert(
             profile, l10n_util::GetStringUTF8(msg));
+        if (screen_off_alert)

[James Cook, 2017/02/04 00:00:38]

Can you just check alert == ash::A11Y_ALERT_SCREEN_OFF instead of introducing a
new local variable?

Also, this code needs comments about why you are doing this. It's not obvious to
me from reading the code why you are turning things on and off.

[Qiang(Joe) Xu, 2017/02/06 03:13:57]

Done.

+          SetAutomationManagerEnabled(profile, false);
       }
     }
   }
 
   ash::AccessibilityAlert GetLastAccessibilityAlert() override {
     return ash::A11Y_ALERT_NONE;
   }
 
   bool ShouldToggleSpokenFeedbackViaTouch() override {
     DCHECK(AccessibilityManager::Get());
@@ skipping 13 matching lines @@
 
   base::TimeDelta PlayShutdownSound() const override {
     return AccessibilityManager::Get()->PlayShutdownSound();
   }
 
   void HandleAccessibilityGesture(ui::AXGesture gesture) override {
     AccessibilityManager::Get()->HandleAccessibilityGesture(gesture);
   }
 
  private:
+  void SetAutomationManagerEnabled(content::BrowserContext* context,
+                                   bool enabled) {
+    AutomationManagerAura* manager = AutomationManagerAura::GetInstance();

[James Cook, 2017/02/04 00:00:38]

optional: DCHECK(context); so the reader knows it cannot be null.

[Qiang(Joe) Xu, 2017/02/06 03:13:57]

Done.

+    enabled ? manager->Enable(context) : manager->Disable();

[James Cook, 2017/02/04 00:00:38]

Use if (enabled) not a trigraph.

[Qiang(Joe) Xu, 2017/02/06 03:13:57]

Done.

+  }
+
   DISALLOW_COPY_AND_ASSIGN(AccessibilityDelegateImpl);
 };
 
 }  // namespace
 
 ChromeShellDelegate::ChromeShellDelegate()
     : shelf_delegate_(NULL) {
   PlatformInit();
 }
 
@@ skipping 225 matching lines @@
       NOTREACHED() << "Unexpected notification " << type;
   }
 }
 
 void ChromeShellDelegate::PlatformInit() {
   registrar_.Add(this, chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED,
                  content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_SESSION_STARTED,
                  content::NotificationService::AllSources());
 }