Skip to content

Commit

Permalink
chromeos: Move AXHostService into ash
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
James Cook authored and Commit Bot committed Sep 28, 2018
1 parent 367235e commit e392ddd
Show file tree
Hide file tree
Showing 15 changed files with 105 additions and 56 deletions.
3 changes: 3 additions & 0 deletions ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ 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",
Expand Down Expand Up @@ -131,6 +132,7 @@ 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",
Expand Down Expand Up @@ -1687,6 +1689,7 @@ 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",
Expand Down
18 changes: 16 additions & 2 deletions ash/accessibility/accessibility_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
#ifndef ASH_ACCESSIBILITY_ACCESSIBILITY_DELEGATE_H_
#define ASH_ACCESSIBILITY_ACCESSIBILITY_DELEGATE_H_

#include <vector>

#include "ash/ash_export.h"
#include "base/time/time.h"
#include "ui/accessibility/ax_enums.mojom.h"
#include "ui/accessibility/ax_tree_id.h"
#include "ui/accessibility/ax_tree_update.h"

namespace ui {
struct AXEvent;
}

namespace ash {

Expand Down Expand Up @@ -36,6 +42,14 @@ 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.
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 "chrome/browser/chromeos/accessibility/ax_host_service.h"
#include "ash/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;
Expand All @@ -23,12 +23,8 @@ AXHostService::AXHostService() {
// AX tree ID is automatically assigned.
DCHECK(!tree_id().empty());

// 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());
}
// TODO(jamescook): Eliminate this when multiple remote trees are supported.
Shell::Get()->accessibility_controller()->set_remote_ax_tree_id(tree_id());

DCHECK(!instance_);
instance_ = this;
Expand Down Expand Up @@ -71,16 +67,12 @@ void AXHostService::HandleAccessibilityEvent(
const std::vector<ui::AXTreeUpdate>& updates,
const ui::AXEvent& event) {
CHECK_EQ(tree_id, this->tree_id());
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);
// 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);
}

void AXHostService::PerformAction(const ui::AXActionData& data) {
Expand All @@ -104,6 +96,7 @@ void AXHostService::NotifyAutomationEnabled() {
}

void AXHostService::OnRemoteHostDisconnected() {
extensions::AutomationEventRouter::GetInstance()->DispatchTreeDestroyedEvent(
tree_id(), nullptr /* browser_context */);
Shell::Get()->accessibility_delegate()->DispatchTreeDestroyedEvent(tree_id());
}

} // namespace ash
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,30 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_CHROMEOS_ACCESSIBILITY_AX_HOST_SERVICE_H_
#define CHROME_BROWSER_CHROMEOS_ACCESSIBILITY_AX_HOST_SERVICE_H_
#ifndef ASH_ACCESSIBILITY_AX_HOST_SERVICE_H_
#define ASH_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 AXHostService : public service_manager::Service,
public ax::mojom::AXHost,
public ui::AXHostDelegate {
class ASH_EXPORT AXHostService : public service_manager::Service,
public ax::mojom::AXHost,
public ui::AXHostDelegate {
public:
AXHostService();
~AXHostService() override;
Expand Down Expand Up @@ -66,4 +70,6 @@ class AXHostService : public service_manager::Service,
DISALLOW_COPY_AND_ASSIGN(AXHostService);
};

#endif // CHROME_BROWSER_CHROMEOS_ACCESSIBILITY_AX_HOST_SERVICE_H_
} // namespace ash

#endif // ASH_ACCESSIBILITY_AX_HOST_SERVICE_H_
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/chromeos/accessibility/ax_host_service.h"
#include "ash/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"
#include "mojo/public/cpp/bindings/binding.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/accessibility/mojom/ax_host.mojom.h"

namespace ash {
namespace {

class TestAXRemoteHost : ax::mojom::AXRemoteHost {
Expand Down Expand Up @@ -51,16 +53,7 @@ class TestAXRemoteHost : ax::mojom::AXRemoteHost {
DISALLOW_COPY_AND_ASSIGN(TestAXRemoteHost);
};

class AXHostServiceTest : public testing::Test {
public:
AXHostServiceTest() = default;
~AXHostServiceTest() override = default;

private:
base::test::ScopedTaskEnvironment scoped_task_enviroment_;

DISALLOW_COPY_AND_ASSIGN(AXHostServiceTest);
};
using AXHostServiceTest = AshTestBase;

TEST_F(AXHostServiceTest, AddClientThenEnable) {
AXHostService service;
Expand Down Expand Up @@ -120,3 +113,4 @@ TEST_F(AXHostServiceTest, PerformAction) {
}

} // namespace
} // namespace ash
8 changes: 8 additions & 0 deletions ash/accessibility/default_accessibility_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,12 @@ 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
5 changes: 5 additions & 0 deletions ash/accessibility/default_accessibility_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace ash {

// Used in tests and non-production code like ash_shell_with_content.
class ASH_EXPORT DefaultAccessibilityDelegate : public AccessibilityDelegate {
public:
DefaultAccessibilityDelegate();
Expand All @@ -21,6 +22,10 @@ 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;
Expand Down
1 change: 1 addition & 0 deletions ash/ash_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ 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)) {
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,8 @@ 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.
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/ash_service_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#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"
Expand All @@ -15,7 +16,6 @@
#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"
Expand Down Expand Up @@ -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<AXHostService>();
return std::make_unique<ash::AXHostService>();
});
(*services)[ax::mojom::kAXHostServiceName] = info;
}

if (features::IsMultiProcessMash())
return;

(*services)[ash::mojom::kServiceName] =
ash::AshService::CreateEmbeddedServiceInfo();
}
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,6 @@ 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",
Expand Down Expand Up @@ -2019,7 +2017,6 @@ 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",
Expand Down
25 changes: 25 additions & 0 deletions chrome/browser/ui/ash/chrome_accessibility_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

#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;
Expand Down Expand Up @@ -42,3 +45,25 @@ 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 */);
}
4 changes: 4 additions & 0 deletions chrome/browser/ui/ash/chrome_accessibility_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ 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);
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/aura/accessibility/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
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",
],
Expand Down
Loading

0 comments on commit e392ddd

Please sign in to comment.