Revert "chromeos: Move AXHostService into ash"

This reverts commit e392dddd.

Reason for revert:
Follow-up design discussions with dmazzoni and dtseng suggest that
this might not be the right direction to go.

Instead we might want to use AXHostService as the way that both
out-of-process ash and remote app windows/widgets talk to the
browser. We may also have a different mechanism for assigning
AX child tree IDs per-widget.

Original change's description:
> chromeos: Move AXHostService into ash
> 
> This is a step toward supporting multiple remote apps and widgets. We
> need this for browser windows under single-process-mash.
> 
> Under mash the ash process is responsible for top-level window creation
> for remote apps. It will need to route accessibility actions (like hit
> testing) to the correct remote app. Likewise it manages window frames
> and focus. Therefore it should own the mojo AXHostService, not the
> browser.
> 
> Accessibility events from the remote app must be routed out of ash
> into chrome to the accessibility extension. For now we do this with
> the in-process AccessibilityDelegate. I didn't introduce a new delegate
> because the mojo service is lazily created and it's hard to connect
> a new delegate at startup. Subsequent CLs will:
> * Always create the service at ash startup
> * Convert the delegate methods to mojo
> 
> See go/mash-ax-node-tree for the full plan.
> 
> Bug: 888147
> Test: ash_unittests, manually use ChromeVox and STS with shortcut viewer
> Change-Id: Ia9e63689a8cb407b66397778be4617479fdf6c92
> Reviewed-on: https://chromium-review.googlesource.com/1249920
> Commit-Queue: James Cook <jamescook@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: David Tseng <dtseng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#594949}

TBR=jamescook@chromium.org,sky@chromium.org,dtseng@chromium.org

Change-Id: I156656b5114a28693308252ee531bbf4c3db867a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 888147
Reviewed-on: https://chromium-review.googlesource.com/1252629Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595203}

ash/BUILD.gn

@@ -34,7 +34,6 @@ component("ash") {
     "accelerators/accelerator_controller.h",
     "accessibility/accessibility_controller.h",
     "accessibility/accessibility_delegate.h",
-    "accessibility/ax_host_service.h",
     "accessibility/focus_ring_controller.h",
     "app_list/app_list_controller_impl.h",
     "detachable_base/detachable_base_handler.h",
@@ -132,7 +131,6 @@ component("ash") {
     "accessibility/accessibility_observer.h",
     "accessibility/accessibility_panel_layout_manager.cc",
     "accessibility/accessibility_panel_layout_manager.h",
-    "accessibility/ax_host_service.cc",
     "accessibility/default_accessibility_delegate.cc",
     "accessibility/default_accessibility_delegate.h",
     "accessibility/focus_ring_controller.cc",
@@ -1689,7 +1687,6 @@ test("ash_unittests") {
     "accessibility/accessibility_focus_ring_group_unittest.cc",
     "accessibility/accessibility_highlight_controller_unittest.cc",
     "accessibility/accessibility_panel_layout_manager_unittest.cc",
-    "accessibility/ax_host_service_unittest.cc",
     "accessibility/key_accessibility_enabler_unittest.cc",
     "accessibility/touch_accessibility_enabler_unittest.cc",
     "accessibility/touch_exploration_controller_unittest.cc",

ash/accessibility/accessibility_delegate.h

@@ -5,15 +5,9 @@
 #ifndef ASH_ACCESSIBILITY_ACCESSIBILITY_DELEGATE_H_
 #define ASH_ACCESSIBILITY_ACCESSIBILITY_DELEGATE_H_
 
-#include <vector>
-
 #include "ash/ash_export.h"
-#include "ui/accessibility/ax_tree_id.h"
-#include "ui/accessibility/ax_tree_update.h"
-
-namespace ui {
-struct AXEvent;
-}
+#include "base/time/time.h"
+#include "ui/accessibility/ax_enums.mojom.h"
 
 namespace ash {
 
@@ -42,14 +36,6 @@ class ASH_EXPORT AccessibilityDelegate {
   // is not saved, return a negative value.
   virtual double GetSavedScreenMagnifierScale() = 0;
 
-  // Automation API support for remote mojo apps.
-  // TODO(jamescook): Convert to mojo API.
-  virtual void DispatchAccessibilityEvent(
-      const ui::AXTreeID& tree_id,
-      const std::vector<ui::AXTreeUpdate>& updates,
-      const ui::AXEvent& event) = 0;
-  virtual void DispatchTreeDestroyedEvent(const ui::AXTreeID& tree_id) = 0;
-
   // NOTE: Prefer adding methods to AccessibilityController, see class comment.
 };
 

ash/accessibility/default_accessibility_delegate.cc

@@ -40,12 +40,4 @@ double DefaultAccessibilityDelegate::GetSavedScreenMagnifierScale() {
   return std::numeric_limits<double>::min();
 }
 
-void DefaultAccessibilityDelegate::DispatchAccessibilityEvent(
-    const ui::AXTreeID& tree_id,
-    const std::vector<ui::AXTreeUpdate>& updates,
-    const ui::AXEvent& event) {}
-
-void DefaultAccessibilityDelegate::DispatchTreeDestroyedEvent(
-    const ui::AXTreeID& tree_id) {}
-
 }  // namespace ash

ash/accessibility/default_accessibility_delegate.h

@@ -11,7 +11,6 @@
 
 namespace ash {
 
-// Used in tests and non-production code like ash_shell_with_content.
 class ASH_EXPORT DefaultAccessibilityDelegate : public AccessibilityDelegate {
  public:
   DefaultAccessibilityDelegate();
@@ -22,10 +21,6 @@ class ASH_EXPORT DefaultAccessibilityDelegate : public AccessibilityDelegate {
   bool ShouldShowAccessibilityMenu() const override;
   void SaveScreenMagnifierScale(double scale) override;
   double GetSavedScreenMagnifierScale() override;
-  void DispatchAccessibilityEvent(const ui::AXTreeID& tree_id,
-                                  const std::vector<ui::AXTreeUpdate>& updates,
-                                  const ui::AXEvent& event) override;
-  void DispatchTreeDestroyedEvent(const ui::AXTreeID& tree_id) override;
 
  private:
   bool screen_magnifier_enabled_ = false;

ash/ash_service.cc

@@ -203,7 +203,6 @@ void AshService::CreateService(
     service_manager::mojom::ServiceRequest service,
     const std::string& name,
     service_manager::mojom::PIDReceiverPtr pid_receiver) {
-  // TODO(jamescook): Create the AXHostService here under mash.
   DCHECK_EQ(name, ws::mojom::kServiceName);
   Shell::Get()->window_service_owner()->BindWindowService(std::move(service));
   if (base::FeatureList::IsEnabled(features::kMash)) {

chrome/browser/DEPS

@@ -215,8 +215,11 @@ include_rules = [
 
 specific_include_rules = {
   "ash_service_registry\.cc": [
+    # The following are needed for classic mode to create the WindowService.
+    # Once Ash runs out of process no longer needed.
+    "+ash/shell.h",
+
     # Needed for classic mode.
-    "+ash/accessibility/ax_host_service.h",
     "+ash/ash_service.h",
   ],
   # TODO(mash): Fix. https://crbug.com/768439, https://crbug.com/768395.

chrome/browser/ash_service_registry.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/ash_service_registry.h"
 
-#include "ash/accessibility/ax_host_service.h"
 #include "ash/ash_service.h"
 #include "ash/components/quick_launch/public/mojom/constants.mojom.h"
 #include "ash/components/shortcut_viewer/public/mojom/shortcut_viewer.mojom.h"
@@ -16,6 +15,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
+#include "chrome/browser/chromeos/accessibility/ax_host_service.h"
 #include "chrome/browser/chromeos/prefs/pref_connector_service.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/common/service_manager_connection.h"
@@ -84,20 +84,20 @@ void RegisterInProcessServices(
     (*services)[ash::mojom::kPrefConnectorServiceName] = info;
   }
 
-  if (features::IsMultiProcessMash())
-    return;
-
   {
     // Register the accessibility host service.
     service_manager::EmbeddedServiceInfo info;
     info.task_runner = base::ThreadTaskRunnerHandle::Get();
     info.factory =
         base::BindRepeating([]() -> std::unique_ptr<service_manager::Service> {
-          return std::make_unique<ash::AXHostService>();
+          return std::make_unique<AXHostService>();
         });
     (*services)[ax::mojom::kAXHostServiceName] = info;
   }
 
+  if (features::IsMultiProcessMash())
+    return;
+
   (*services)[ash::mojom::kServiceName] =
       ash::AshService::CreateEmbeddedServiceInfo();
 }

chrome/browser/chromeos/BUILD.gn

@@ -265,6 +265,8 @@ source_set("chromeos") {
     "accessibility/accessibility_manager.h",
     "accessibility/accessibility_panel.cc",
     "accessibility/accessibility_panel.h",
+    "accessibility/ax_host_service.cc",
+    "accessibility/ax_host_service.h",
     "accessibility/chromevox_panel.cc",
     "accessibility/chromevox_panel.h",
     "accessibility/dictation_chromeos.cc",
@@ -2021,6 +2023,7 @@ source_set("unit_tests") {
     "../policy/default_geolocation_policy_handler_unittest.cc",
     "../resources/chromeos/zip_archiver/test/char_coding_test.cc",
     "../ui/browser_finder_chromeos_unittest.cc",
+    "accessibility/ax_host_service_unittest.cc",
     "app_mode/startup_app_launcher_unittest.cc",
     "apps/intent_helper/apps_navigation_throttle_unittest.cc",
     "apps/intent_helper/page_transition_util_unittest.cc",

ash/accessibility/ax_host_service.cc → chrome/browser/chromeos/accessibility/ax_host_service.cc

@@ -2,19 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ash/accessibility/ax_host_service.h"
+#include "chrome/browser/chromeos/accessibility/ax_host_service.h"
 
 #include "ash/accessibility/accessibility_controller.h"
-#include "ash/accessibility/accessibility_delegate.h"
 #include "ash/shell.h"
 #include "base/bind.h"
+#include "chrome/browser/extensions/api/automation_internal/automation_event_router.h"
+#include "chrome/common/extensions/chrome_extension_messages.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/service_manager/public/cpp/service_context.h"
 #include "ui/accessibility/ax_event.h"
+#include "ui/aura/env.h"
 #include "ui/views/mus/ax_remote_host.h"
 
-namespace ash {
-
 AXHostService* AXHostService::instance_ = nullptr;
 
 bool AXHostService::automation_enabled_ = false;
@@ -23,8 +23,12 @@ AXHostService::AXHostService() {
   // AX tree ID is automatically assigned.
   DCHECK(!tree_id().empty());
 
-  // TODO(jamescook): Eliminate this when multiple remote trees are supported.
-  Shell::Get()->accessibility_controller()->set_remote_ax_tree_id(tree_id());
+  // ash::Shell may not exist in tests.
+  if (ash::Shell::HasInstance()) {
+    // TODO(jamescook): Eliminate this when tree ID assignment is handed in ash.
+    ash::Shell::Get()->accessibility_controller()->set_remote_ax_tree_id(
+        tree_id());
+  }
 
   DCHECK(!instance_);
   instance_ = this;
@@ -67,12 +71,16 @@ void AXHostService::HandleAccessibilityEvent(
     const std::vector<ui::AXTreeUpdate>& updates,
     const ui::AXEvent& event) {
   CHECK_EQ(tree_id, this->tree_id());
-  // Use an in-process delegate back to Chrome for efficiency in classic ash.
-  // For mash we'll need to pass around a mojo interface pointer such that a
-  // remote app can talk directly to the browser process and not ping-pong
-  // through the ash process.
-  Shell::Get()->accessibility_delegate()->DispatchAccessibilityEvent(
-      tree_id, updates, event);
+  ExtensionMsg_AccessibilityEventBundleParams event_bundle;
+  event_bundle.tree_id = tree_id;
+  for (const ui::AXTreeUpdate& update : updates)
+    event_bundle.updates.push_back(update);
+  event_bundle.events.push_back(event);
+  event_bundle.mouse_location = aura::Env::GetInstance()->last_mouse_location();
+
+  // Forward the tree updates and the event to the accessibility extension.
+  extensions::AutomationEventRouter::GetInstance()->DispatchAccessibilityEvents(
+      event_bundle);
 }
 
 void AXHostService::PerformAction(const ui::AXActionData& data) {
@@ -96,7 +104,6 @@ void AXHostService::NotifyAutomationEnabled() {
 }
 
 void AXHostService::OnRemoteHostDisconnected() {
-  Shell::Get()->accessibility_delegate()->DispatchTreeDestroyedEvent(tree_id());
+  extensions::AutomationEventRouter::GetInstance()->DispatchTreeDestroyedEvent(
+      tree_id(), nullptr /* browser_context */);
 }
-
-}  // namespace ash

ash/accessibility/ax_host_service.h → chrome/browser/chromeos/accessibility/ax_host_service.h

@@ -2,30 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef ASH_ACCESSIBILITY_AX_HOST_SERVICE_H_
-#define ASH_ACCESSIBILITY_AX_HOST_SERVICE_H_
+#ifndef CHROME_BROWSER_CHROMEOS_ACCESSIBILITY_AX_HOST_SERVICE_H_
+#define CHROME_BROWSER_CHROMEOS_ACCESSIBILITY_AX_HOST_SERVICE_H_
 
 #include <memory>
 
-#include "ash/ash_export.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "mojo/public/cpp/bindings/interface_ptr_set.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/service.h"
 #include "ui/accessibility/ax_host_delegate.h"
-#include "ui/accessibility/ax_tree_id.h"
 #include "ui/accessibility/mojom/ax_host.mojom.h"
 
-namespace ash {
-
 // Forwards accessibility events from clients in other processes that use aura
 // and views (e.g. the Chrome OS keyboard shortcut_viewer) to accessibility
 // extensions. Renderers, PDF, etc. use a different path. Created when the first
 // client connects over mojo. Implements AXHostDelegate by routing actions over
 // mojo to the remote process.
-class ASH_EXPORT AXHostService : public service_manager::Service,
-                                 public ax::mojom::AXHost,
-                                 public ui::AXHostDelegate {
+class AXHostService : public service_manager::Service,
+                      public ax::mojom::AXHost,
+                      public ui::AXHostDelegate {
  public:
   AXHostService();
   ~AXHostService() override;
@@ -70,6 +66,4 @@ class ASH_EXPORT AXHostService : public service_manager::Service,
   DISALLOW_COPY_AND_ASSIGN(AXHostService);
 };
 
-}  // namespace ash
-
-#endif  // ASH_ACCESSIBILITY_AX_HOST_SERVICE_H_
+#endif  // CHROME_BROWSER_CHROMEOS_ACCESSIBILITY_AX_HOST_SERVICE_H_

ash/accessibility/ax_host_service_unittest.cc → chrome/browser/chromeos/accessibility/ax_host_service_unittest.cc

@@ -2,9 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ash/accessibility/ax_host_service.h"
+#include "chrome/browser/chromeos/accessibility/ax_host_service.h"
 
-#include "ash/test/ash_test_base.h"
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/test/scoped_task_environment.h"
@@ -12,7 +11,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/accessibility/mojom/ax_host.mojom.h"
 
-namespace ash {
 namespace {
 
 class TestAXRemoteHost : ax::mojom::AXRemoteHost {
@@ -53,7 +51,16 @@ class TestAXRemoteHost : ax::mojom::AXRemoteHost {
   DISALLOW_COPY_AND_ASSIGN(TestAXRemoteHost);
 };
 
-using AXHostServiceTest = AshTestBase;
+class AXHostServiceTest : public testing::Test {
+ public:
+  AXHostServiceTest() = default;
+  ~AXHostServiceTest() override = default;
+
+ private:
+  base::test::ScopedTaskEnvironment scoped_task_enviroment_;
+
+  DISALLOW_COPY_AND_ASSIGN(AXHostServiceTest);
+};
 
 TEST_F(AXHostServiceTest, AddClientThenEnable) {
   AXHostService service;
@@ -113,4 +120,3 @@ TEST_F(AXHostServiceTest, PerformAction) {
 }
 
 }  // namespace
-}  // namespace ash

chrome/browser/ui/ash/chrome_accessibility_delegate.cc

@@ -8,9 +8,6 @@
 
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
-#include "chrome/browser/extensions/api/automation_internal/automation_event_router.h"
-#include "chrome/common/extensions/chrome_extension_messages.h"
-#include "ui/aura/env.h"
 
 using chromeos::AccessibilityManager;
 using chromeos::MagnificationManager;
@@ -45,25 +42,3 @@ double ChromeAccessibilityDelegate::GetSavedScreenMagnifierScale() {
 
   return std::numeric_limits<double>::min();
 }
-
-void ChromeAccessibilityDelegate::DispatchAccessibilityEvent(
-    const ui::AXTreeID& tree_id,
-    const std::vector<ui::AXTreeUpdate>& updates,
-    const ui::AXEvent& event) {
-  ExtensionMsg_AccessibilityEventBundleParams event_bundle;
-  event_bundle.tree_id = tree_id;
-  for (const ui::AXTreeUpdate& update : updates)
-    event_bundle.updates.push_back(update);
-  event_bundle.events.push_back(event);
-  event_bundle.mouse_location = aura::Env::GetInstance()->last_mouse_location();
-
-  // Forward the tree updates and the event to the accessibility extension.
-  extensions::AutomationEventRouter::GetInstance()->DispatchAccessibilityEvents(
-      event_bundle);
-}
-
-void ChromeAccessibilityDelegate::DispatchTreeDestroyedEvent(
-    const ui::AXTreeID& tree_id) {
-  extensions::AutomationEventRouter::GetInstance()->DispatchTreeDestroyedEvent(
-      tree_id, nullptr /* browser_context */);
-}

chrome/browser/ui/ash/chrome_accessibility_delegate.h

@@ -20,10 +20,6 @@ class ChromeAccessibilityDelegate : public ash::AccessibilityDelegate {
   bool ShouldShowAccessibilityMenu() const override;
   void SaveScreenMagnifierScale(double scale) override;
   double GetSavedScreenMagnifierScale() override;
-  void DispatchAccessibilityEvent(const ui::AXTreeID& tree_id,
-                                  const std::vector<ui::AXTreeUpdate>& updates,
-                                  const ui::AXEvent& event) override;
-  void DispatchTreeDestroyedEvent(const ui::AXTreeID& tree_id) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ChromeAccessibilityDelegate);

chrome/browser/ui/aura/accessibility/DEPS

 specific_include_rules = {
   "automation_manager_aura\.cc": [
     # TODO(mash): Fix. https://crbug.com/756054
-    "+ash/accessibility/ax_host_service.h",
     "+ash/shell.h",
     "+ash/wm/window_util.h",
   ],

chrome/browser/ui/aura/accessibility/automation_manager_aura.cc

@@ -25,9 +25,9 @@
 #include "ui/views/widget/widget.h"
 
 #if defined(OS_CHROMEOS)
-#include "ash/accessibility/ax_host_service.h"
 #include "ash/shell.h"
 #include "ash/wm/window_util.h"
+#include "chrome/browser/chromeos/accessibility/ax_host_service.h"
 #include "ui/base/ui_base_features.h"
 #include "ui/views/widget/widget_delegate.h"
 #endif
@@ -58,8 +58,7 @@ void AutomationManagerAura::Enable() {
     }
   }
   // Gain access to out-of-process native windows.
-  // TODO(mash): Split AXHostService into chrome and ash parts.
-  ash::AXHostService::SetAutomationEnabled(true);
+  AXHostService::SetAutomationEnabled(true);
 #endif
 }
 
@@ -68,7 +67,7 @@ void AutomationManagerAura::Disable() {
   Reset(true);
 
 #if defined(OS_CHROMEOS)
-  ash::AXHostService::SetAutomationEnabled(false);
+  AXHostService::SetAutomationEnabled(false);
 #endif
 }