Skip to content

Commit

Permalink
[Lacros A11y] Send STS context clicks from Lacros to Ash
Browse files Browse the repository at this point in the history
Select to Speak can now be started from Lacros context menu
shown when there is a document selection.

This changes adds the crosapi plumbing that sends the context
menu click from EmbeddedA11yManagerLacros to AccessibilityManager
in Ash.

See go/lacros-embedded-a11y-extensions (subsection of larger
Lacros a11y doc) for background and design.

Note the command line flag to Lacros must be set in
ash_browser_test_starter because it needs to be combined
with other Lacros flags (rather than being sent separately
from the JS test). Since this flag only influences whether
an API is available for Select to Speak in Lacros, it seems
safe to turn on for all tests.

Bug: b/286413297
Test: Manual, new
Change-Id: If9ae1ed1a2b7272304fe4de258af0da43369b424
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4711673
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184779}
  • Loading branch information
Katie Dektar authored and Chromium LUCI CQ committed Aug 17, 2023
1 parent 119b4fa commit 8b4a740
Show file tree
Hide file tree
Showing 16 changed files with 186 additions and 16 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/ash/crosapi/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ source_set("crosapi") {
"drive_integration_service_ash.h",
"echo_private_ash.cc",
"echo_private_ash.h",
"embedded_accessibility_helper_client_ash.cc",
"embedded_accessibility_helper_client_ash.h",
"emoji_picker_ash.cc",
"emoji_picker_ash.h",
"environment_provider.cc",
Expand Down
13 changes: 13 additions & 0 deletions chrome/browser/ash/crosapi/crosapi_ash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "chrome/browser/ash/crosapi/download_status_updater_ash.h"
#include "chrome/browser/ash/crosapi/drive_integration_service_ash.h"
#include "chrome/browser/ash/crosapi/echo_private_ash.h"
#include "chrome/browser/ash/crosapi/embedded_accessibility_helper_client_ash.h"
#include "chrome/browser/ash/crosapi/emoji_picker_ash.h"
#include "chrome/browser/ash/crosapi/extension_info_private_ash.h"
#include "chrome/browser/ash/crosapi/feedback_ash.h"
Expand Down Expand Up @@ -125,6 +126,7 @@
#include "chromeos/components/sensors/ash/sensor_hal_dispatcher.h"
#include "chromeos/crosapi/mojom/device_local_account_extension_service.mojom.h"
#include "chromeos/crosapi/mojom/drive_integration_service.mojom.h"
#include "chromeos/crosapi/mojom/embedded_accessibility_helper.mojom.h"
#include "chromeos/crosapi/mojom/feedback.mojom.h"
#include "chromeos/crosapi/mojom/file_manager.mojom.h"
#include "chromeos/crosapi/mojom/firewall_hole.mojom.h"
Expand Down Expand Up @@ -218,6 +220,8 @@ CrosapiAsh::CrosapiAsh(CrosapiDependencyRegistry* registry)
drive_integration_service_ash_(
std::make_unique<DriveIntegrationServiceAsh>()),
echo_private_ash_(std::make_unique<EchoPrivateAsh>()),
embedded_accessibility_helper_client_ash_(
std::make_unique<EmbeddedAccessibilityHelperClientAsh>()),
emoji_picker_ash_(std::make_unique<EmojiPickerAsh>()),
extension_info_private_ash_(std::make_unique<ExtensionInfoPrivateAsh>()),
feedback_ash_(std::make_unique<FeedbackAsh>()),
Expand Down Expand Up @@ -498,6 +502,15 @@ void CrosapiAsh::BindEchoPrivate(
mojo::PendingReceiver<mojom::EchoPrivate> receiver) {
echo_private_ash_->BindReceiver(std::move(receiver));
}

void CrosapiAsh::BindEmbeddedAccessibilityHelperClientFactory(
mojo::PendingReceiver<mojom::EmbeddedAccessibilityHelperClientFactory>
receiver) {
embedded_accessibility_helper_client_ash_
->BindEmbeddedAccessibilityHelperClientFactoryReceiver(
std::move(receiver));
}

void CrosapiAsh::BindEmojiPicker(
mojo::PendingReceiver<mojom::EmojiPicker> receiver) {
emoji_picker_ash_->BindReceiver(std::move(receiver));
Expand Down
12 changes: 12 additions & 0 deletions chrome/browser/ash/crosapi/crosapi_ash.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class DownloadControllerAsh;
class DownloadStatusUpdaterAsh;
class DriveIntegrationServiceAsh;
class EchoPrivateAsh;
class EmbeddedAccessibilityHelperClientAsh;
class EmojiPickerAsh;
class ExtensionInfoPrivateAsh;
class FeedbackAsh;
Expand Down Expand Up @@ -213,6 +214,10 @@ class CrosapiAsh : public mojom::Crosapi {
mojo::PendingReceiver<mojom::DriveIntegrationService> receiver) override;
void BindEchoPrivate(
mojo::PendingReceiver<mojom::EchoPrivate> receiver) override;
void BindEmbeddedAccessibilityHelperClientFactory(
mojo::PendingReceiver<
::crosapi::mojom::EmbeddedAccessibilityHelperClientFactory> receiver)
override;
void BindEmojiPicker(
mojo::PendingReceiver<mojom::EmojiPicker> receiver) override;
void BindExtensionInfoPrivate(
Expand Down Expand Up @@ -422,6 +427,11 @@ class CrosapiAsh : public mojom::Crosapi {

EchoPrivateAsh* echo_private_ash() { return echo_private_ash_.get(); }

EmbeddedAccessibilityHelperClientAsh*
embedded_accessibility_helper_client_ash() {
return embedded_accessibility_helper_client_ash_.get();
}

EmojiPickerAsh* emoji_picker_ash() { return emoji_picker_ash_.get(); }

ExtensionInfoPrivateAsh* extension_info_private_ash() {
Expand Down Expand Up @@ -580,6 +590,8 @@ class CrosapiAsh : public mojom::Crosapi {
std::unique_ptr<DownloadStatusUpdaterAsh> download_status_updater_ash_;
std::unique_ptr<DriveIntegrationServiceAsh> drive_integration_service_ash_;
std::unique_ptr<EchoPrivateAsh> echo_private_ash_;
std::unique_ptr<EmbeddedAccessibilityHelperClientAsh>
embedded_accessibility_helper_client_ash_;
std::unique_ptr<EmojiPickerAsh> emoji_picker_ash_;
std::unique_ptr<ExtensionInfoPrivateAsh> extension_info_private_ash_;
std::unique_ptr<FeedbackAsh> feedback_ash_;
Expand Down
5 changes: 4 additions & 1 deletion chrome/browser/ash/crosapi/crosapi_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
#include "chromeos/crosapi/mojom/download_status_updater.mojom.h"
#include "chromeos/crosapi/mojom/drive_integration_service.mojom.h"
#include "chromeos/crosapi/mojom/echo_private.mojom.h"
#include "chromeos/crosapi/mojom/embedded_accessibility_helper.mojom.h"
#include "chromeos/crosapi/mojom/emoji_picker.mojom.h"
#include "chromeos/crosapi/mojom/extension_info_private.mojom.h"
#include "chromeos/crosapi/mojom/feedback.mojom.h"
Expand Down Expand Up @@ -292,7 +293,7 @@ constexpr InterfaceVersionEntry MakeInterfaceVersionEntry() {
return {T::Uuid_, T::Version_};
}

static_assert(crosapi::mojom::Crosapi::Version_ == 114,
static_assert(crosapi::mojom::Crosapi::Version_ == 115,
"If you add a new crosapi, please add it to "
"kInterfaceVersionEntries below.");

Expand Down Expand Up @@ -418,6 +419,8 @@ constexpr InterfaceVersionEntry kInterfaceVersionEntries[] = {
MakeInterfaceVersionEntry<media_session::mojom::AudioFocusManager>(),
MakeInterfaceVersionEntry<media_session::mojom::AudioFocusManagerDebug>(),
MakeInterfaceVersionEntry<crosapi::mojom::ParentAccess>(),
MakeInterfaceVersionEntry<
crosapi::mojom::EmbeddedAccessibilityHelperClientFactory>(),
};

constexpr bool HasDuplicatedUuid() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ash/crosapi/embedded_accessibility_helper_client_ash.h"

#include "chrome/browser/ash/accessibility/accessibility_manager.h"
#include "chromeos/crosapi/mojom/embedded_accessibility_helper.mojom.h"

namespace crosapi {

EmbeddedAccessibilityHelperClientAsh::EmbeddedAccessibilityHelperClientAsh() =
default;
EmbeddedAccessibilityHelperClientAsh::~EmbeddedAccessibilityHelperClientAsh() =
default;

void EmbeddedAccessibilityHelperClientAsh::SpeakSelectedText() {
ash::AccessibilityManager::Get()->OnSelectToSpeakContextMenuClick();
}

void EmbeddedAccessibilityHelperClientAsh::
BindEmbeddedAccessibilityHelperClientFactoryReceiver(
mojo::PendingReceiver<
crosapi::mojom::EmbeddedAccessibilityHelperClientFactory>
receiver) {
embedded_ax_helper_factory_receivers_.Add(this, std::move(receiver));
}

void EmbeddedAccessibilityHelperClientAsh::
BindEmbeddedAccessibilityHelperClient(
mojo::PendingReceiver<crosapi::mojom::EmbeddedAccessibilityHelperClient>
embeded_ax_helper_client) {
embedded_ax_helper_receivers_.Add(this, std::move(embeded_ax_helper_client));
}

} // namespace crosapi
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_ASH_CROSAPI_EMBEDDED_ACCESSIBILITY_HELPER_CLIENT_ASH_H_
#define CHROME_BROWSER_ASH_CROSAPI_EMBEDDED_ACCESSIBILITY_HELPER_CLIENT_ASH_H_

#include "chromeos/crosapi/mojom/embedded_accessibility_helper.mojom.h"
#include "mojo/public/cpp/bindings/receiver_set.h"

namespace crosapi {

class EmbeddedAccessibilityHelperClientAsh
: public crosapi::mojom::EmbeddedAccessibilityHelperClientFactory,
public crosapi::mojom::EmbeddedAccessibilityHelperClient {
public:
EmbeddedAccessibilityHelperClientAsh();
EmbeddedAccessibilityHelperClientAsh(
const EmbeddedAccessibilityHelperClientAsh&) = delete;
EmbeddedAccessibilityHelperClientAsh& operator=(
const EmbeddedAccessibilityHelperClientAsh&) = delete;
~EmbeddedAccessibilityHelperClientAsh() override;

// crosapi::mojom::EmbeddedAccessibilityHelperClientFactory:
void BindEmbeddedAccessibilityHelperClient(
mojo::PendingReceiver<crosapi::mojom::EmbeddedAccessibilityHelperClient>
embeded_ax_helper_client) override;

// crosapi::mojom::EmbeddedAccessibilityHelperClient:
void SpeakSelectedText() override;

void BindEmbeddedAccessibilityHelperClientFactoryReceiver(
mojo::PendingReceiver<
crosapi::mojom::EmbeddedAccessibilityHelperClientFactory> receiver);

private:
mojo::ReceiverSet<crosapi::mojom::EmbeddedAccessibilityHelperClientFactory>
embedded_ax_helper_factory_receivers_;
mojo::ReceiverSet<crosapi::mojom::EmbeddedAccessibilityHelperClient>
embedded_ax_helper_receivers_;
};

} // namespace crosapi

#endif // CHROME_BROWSER_ASH_CROSAPI_EMBEDDED_ACCESSIBILITY_HELPER_CLIENT_ASH_H_
16 changes: 15 additions & 1 deletion chrome/browser/lacros/embedded_a11y_manager_lacros.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension_constants.h"
#include "chrome/common/pref_names.h"
#include "chromeos/crosapi/mojom/embedded_accessibility_helper.mojom.h"
#include "chromeos/lacros/lacros_service.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/extension_file_task_runner.h"
Expand Down Expand Up @@ -96,6 +98,14 @@ void EmbeddedA11yManagerLacros::Init() {
&EmbeddedA11yManagerLacros::OnSwitchAccessEnabledChanged,
weak_ptr_factory_.GetWeakPtr()));

chromeos::LacrosService* impl = chromeos::LacrosService::Get();
if (impl->IsAvailable<
crosapi::mojom::EmbeddedAccessibilityHelperClientFactory>()) {
impl->GetRemote<crosapi::mojom::EmbeddedAccessibilityHelperClientFactory>()
->BindEmbeddedAccessibilityHelperClient(
a11y_helper_remote_.BindNewPipeAndPassReceiver());
}

pdf_ocr_always_active_observer_ = std::make_unique<CrosapiPrefObserver>(
crosapi::mojom::PrefPath::kAccessibilityPdfOcrAlwaysActive,
base::BindRepeating(
Expand All @@ -116,7 +126,11 @@ void EmbeddedA11yManagerLacros::Init() {
}

void EmbeddedA11yManagerLacros::SpeakSelectedText() {
// TODO(b/271633121): Forward through crosapi to Ash.
// Check the remote is bound. It might not be bound on older versions
// of Ash.
if (a11y_helper_remote_.is_bound()) {
a11y_helper_remote_->SpeakSelectedText();
}
if (speak_selected_text_callback_for_test_) {
speak_selected_text_callback_for_test_.Run();
}
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/lacros/embedded_a11y_manager_lacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile_manager_observer.h"
#include "chrome/browser/profiles/profile_observer.h"
#include "chromeos/crosapi/mojom/embedded_accessibility_helper.mojom.h"
#include "chromeos/lacros/crosapi_pref_observer.h"
#include "mojo/public/cpp/bindings/remote.h"

namespace extensions {
class ComponentLoader;
Expand Down Expand Up @@ -111,6 +113,9 @@ class EmbeddedA11yManagerLacros : public ProfileObserver,
base::ScopedObservation<ProfileManager, ProfileManagerObserver>
profile_manager_observation_{this};

mojo::Remote<crosapi::mojom::EmbeddedAccessibilityHelperClient>
a11y_helper_remote_;

base::WeakPtrFactory<EmbeddedA11yManagerLacros> weak_ptr_factory_{this};

friend struct base::DefaultSingletonTraits<EmbeddedA11yManagerLacros>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
#include "chromeos/crosapi/mojom/test_controller.mojom.h"
#include "chromeos/lacros/lacros_service.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/test/accessibility_notification_waiter.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_utils.h"
#include "extensions/browser/extension_host_test_helper.h"
#include "extensions/browser/extension_system.h"
Expand Down Expand Up @@ -199,16 +199,11 @@ class EmbeddedA11yManagerLacrosTest : public InProcessBrowserTest {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();

content::AccessibilityNotificationWaiter waiter(
web_contents, ui::kAXModeComplete, ax::mojom::Event::kLoadComplete);
CHECK(ui_test_utils::NavigateToURL(
browser(), GURL(("data:text/html;charset=utf-8,<p "
"id='selected'>This is some selected text</p>"))));
std::ignore = waiter.WaitForNotification();
EXPECT_TRUE(content::WaitForLoadStop(web_contents));

content::AccessibilityNotificationWaiter selection_waiter(
web_contents, ui::kAXModeComplete,
ui::AXEventGenerator::Event::DOCUMENT_SELECTION_CHANGED);
content::BoundingBoxUpdateWaiter bounding_box_waiter(web_contents);

BrowserView* browser_view =
Expand All @@ -224,9 +219,7 @@ class EmbeddedA11yManagerLacrosTest : public InProcessBrowserTest {

// Set selection with ctrl+a.
generator.PressAndReleaseKey(ui::VKEY_A, ui::EF_CONTROL_DOWN);

bounding_box_waiter.Wait();
CHECK(selection_waiter.WaitForNotification());

ContextMenuWaiter menu_waiter;
generator.PressRightButton();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ AX_TEST_F(
const menuName = chrome.i18n.getMessage(
'select_to_speak_listen_context_menu_option_text');
const listener = (change) => {
if (change.type === 'nodeCreated' && change.target.name === menuName) {
chrome.automation.removeTreeChangeObserver(listener);
// On Lacros, the context menu might be changed before we can click on
// it, whereas on Ash we can usually click on it first. Easiest to
// look for just node changed rather than created.
if (change.type === chrome.automation.TreeChangeType.NODE_CHANGED &&
change.target.name === menuName) {
change.target.doDefault();
}
};
Expand All @@ -54,6 +57,7 @@ AX_TEST_F(
assertEquals(this.mockTts.pendingUtterances().length, 1);
this.assertEqualsCollapseWhitespace(utterance, 'This is some text');
this.mockTts.finishPendingUtterance();
chrome.automation.removeTreeChangeObserver(listener);
})]);
chrome.automation.addTreeChangeObserver('allTreeChanges', listener);
textNode.showContextMenu();
Expand Down
1 change: 1 addition & 0 deletions chrome/test/base/chromeos/ash_browser_test_starter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ bool AshBrowserTestStarter::PrepareEnvironmentForLacros() {
// workaround for fixing crbug/1371655.
lacros_args.emplace_back(base::StringPrintf("--%s=%s", switches::kGaiaUrl,
base_url().spec().c_str()));
lacros_args.emplace_back("--enable-features=ApiAccessibilityServicePrivate");
command_line->AppendSwitchASCII(ash::switches::kLacrosChromeAdditionalArgs,
base::JoinString(lacros_args, "####"));

Expand Down
1 change: 1 addition & 0 deletions chromeos/crosapi/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ mojom("mojom") {
"download_status_updater.mojom",
"drive_integration_service.mojom",
"echo_private.mojom",
"embedded_accessibility_helper.mojom",
"emoji_picker.mojom",
"extension_info_private.mojom",
"extension_keeplist.mojom",
Expand Down
11 changes: 9 additions & 2 deletions chromeos/crosapi/mojom/crosapi.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ import "mojo/public/mojom/base/values.mojom";
import "services/device/public/mojom/hid.mojom";
import "services/media_session/public/mojom/audio_focus.mojom";
import "services/media_session/public/mojom/media_controller.mojom";
import "chromeos/crosapi/mojom/embedded_accessibility_helper.mojom";
import "url/mojom/url.mojom";

// BrowserInfo is a set of parameters passed to ash from browser (such as
Expand Down Expand Up @@ -144,8 +145,8 @@ struct BrowserInfo {
// please note the milestone when you added it, to help us reason about
// compatibility between the client applications and older ash-chrome binaries.
//
// Next version: 115
// Next method id: 118
// Next version: 116
// Next method id: 119
[Stable, Uuid="8b79c34f-2bf8-4499-979a-b17cac522c1e",
RenamedFrom="crosapi.mojom.AshChromeService"]
interface Crosapi {
Expand Down Expand Up @@ -327,6 +328,12 @@ interface Crosapi {
[MinVersion=71] BindEchoPrivate@74(
pending_receiver<EchoPrivate> receiver);

// Binds the accessibility helper client factory interface which allows Lacros
// to send information to Ash needed for accessibility features.
[MinVersion=115]
BindEmbeddedAccessibilityHelperClientFactory@118(
pending_receiver<EmbeddedAccessibilityHelperClientFactory> receiver);

// EmojiPicker is a service to allow lacros to open the system emoji picker.
[MinVersion=85] BindEmojiPicker@89(pending_receiver<EmojiPicker> receiver);

Expand Down
28 changes: 28 additions & 0 deletions chromeos/crosapi/mojom/embedded_accessibility_helper.mojom
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

module crosapi.mojom;

// Interface for EmbeddedAccessibilityHelperClients. Implemented by ash-chrome.
// Used by lacros-chrome (the "embedded helper") to inform when accessibility
// features have received certain events from lacros-chrome so that
// features can act on these events.
[Stable, Uuid="5d670d98-5c5f-4805-ba83-b77690f5f68c"]
interface EmbeddedAccessibilityHelperClient {
// Called by lacros-chrome when a context menu (added with chrome.contextMenus
// API) is clicked for the Select to Speak feature to request that selected
// text is spoken out loud for the user. This is in place of unsupported
// chrome.contextMenus, which is not piped through for Ash extension APIs to
// act in lacros-chrome.
SpeakSelectedText@0();
};

// A factory living in ash-chrome which brokers connections to other
// processes for EmbeddedAccessibilityHelperClient.
[Stable, Uuid="b6216633-c0d5-4eab-848e-d9fdfeaa3c33"]
interface EmbeddedAccessibilityHelperClientFactory {
// Binds EmbeddedAccessibilityHelperClient's receiver in lacros-chrome.
BindEmbeddedAccessibilityHelperClient@0(
pending_receiver<EmbeddedAccessibilityHelperClient> embedded_ax_helper_client);
};

0 comments on commit 8b4a740

Please sign in to comment.