chromeos: Fix turning on ChromeVox after shortcut viewer app is open

If ChromeVox has ever been turned on during a user session then
AutomationManagerAura and AXHostService will be enabled, even if
ChromeVox is turned off later. When the shortcut viewer app launches
it sees automation is enabled and AXRemoteHost caches the enabled state.

When ChromeVox is turned on again AXHostService will generate another
OnAutomationEnabled() message to the remote app. Instead of just
early-exiting because "automation is already on", re-serialize the
app's node tree and send load complete.

This is consistent with how AutomationManagerAura behaves today.

Bug: 876407
Test: added to views_mus_unittests
Change-Id: Ib906da907ca20bbaa3509af72b137462ba006e28
Reviewed-on: https://chromium-review.googlesource.com/1185897Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585533}

ui/views/mus/ax_remote_host.cc

@@ -57,7 +57,8 @@ void AXRemoteHost::StartMonitoringWidget(Widget* widget) {
   // Check if we're already tracking a widget.
   // TODO(jamescook): Support multiple widgets.
   if (widget_)
-    return;
+    StopMonitoringWidget();
+
   widget_ = widget;
   widget_->AddObserver(this);
 
@@ -174,9 +175,11 @@ void AXRemoteHost::BindAndSetRemote() {
 }
 
 void AXRemoteHost::Enable() {
-  // Extensions can send multiple enable events.
-  if (enabled_)
-    return;
+  // Don't early-exit if already enabled. AXRemoteHost can start up in the
+  // "enabled" state even if ChromeVox is on at the moment the app launches.
+  // Turning on ChromeVox later will generate another OnAutomationEnabled()
+  // call and we need to serialize the node tree again. This is similar to
+  // AutomationManagerAura's behavior. https://crbug.com/876407
   enabled_ = true;
 
   std::set<aura::Window*> roots =
@@ -188,7 +191,6 @@ void AXRemoteHost::Enable() {
     // widget.
     if (widget) {
       // TODO(jamescook): Support multiple roots.
-      DCHECK(!widget_);
       StartMonitoringWidget(widget);
     }
   }

ui/views/mus/ax_remote_host_unittest.cc

@@ -34,6 +34,14 @@ class TestAXHostService : public ax::mojom::AXHost {
     return ptr;
   }
 
+  void ResetCounts() {
+    add_client_count_ = 0;
+    event_count_ = 0;
+    last_tree_id_ = 0;
+    last_updates_.clear();
+    last_event_ = ui::AXEvent();
+  }
+
   // ax::mojom::AXHost:
   void SetRemoteHost(ax::mojom::AXRemoteHostPtr client) override {
     ++add_client_count_;
@@ -135,6 +143,28 @@ TEST_F(AXRemoteHostTest, AutomationEnabled) {
             service.last_event_.id);
 }
 
+// Regression test for https://crbug.com/876407
+TEST_F(AXRemoteHostTest, AutomationEnabledTwice) {
+  // If ChromeVox has ever been open during the user session then the remote app
+  // can see automation enabled at startup.
+  TestAXHostService service(true /*automation_enabled*/);
+  AXRemoteHost* remote = CreateRemote(&service);
+  std::unique_ptr<Widget> widget = CreateTestWidget();
+  remote->FlushForTesting();
+
+  // Remote host sent load complete.
+  EXPECT_EQ(ax::mojom::Event::kLoadComplete, service.last_event_.event_type);
+  service.ResetCounts();
+
+  // Simulate host service asking to enable again. This happens if ChromeVox is
+  // re-enabled after the remote app is open.
+  remote->OnAutomationEnabled(true);
+  remote->FlushForTesting();
+
+  // Load complete was sent again after the second enable.
+  EXPECT_EQ(ax::mojom::Event::kLoadComplete, service.last_event_.event_type);
+}
+
 // Views can trigger accessibility events during Widget construction before the
 // AXRemoteHost starts monitoring the widget. This happens with the material
 // design focus ring on text fields. Verify we don't crash in this case.