Skip to content

Commit

Permalink
mash cleanup: Remove accessibility_controller.mojom
Browse files Browse the repository at this point in the history
Replace the mojo interfaces with a c++ virtual interfaces.
Move the mojo enum definitions to c++ enum classes.

Move two functions to the public interface for Chrome's manager.
Rename Ash's impl to AccessibilityControllerImpl.
Make the client register itself with the controller automatically.
Make delegates un-register themselves automatically on destruction.
Async->sync simplification; lifetime robustness; other minor cleanup.

TODO: Cleanup accessibility_controller_impl.cc function ordering.

Bug: 958129
Test: Automated; no Chrome OS a11y regressions.
Change-Id: Ia058c6db98018895482b8ff8d2a92f63b724245c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1680723
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#673533}
  • Loading branch information
Mike Wasserman authored and Commit Bot committed Jun 28, 2019
1 parent 60f9b57 commit d13e95a
Show file tree
Hide file tree
Showing 117 changed files with 1,280 additions and 1,616 deletions.
5 changes: 3 additions & 2 deletions ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ component("ash") {
"ash_service.h",

# TODO: move the following to source. Do NOT add new files here.
"accessibility/accessibility_controller.h",
"accessibility/accessibility_controller_impl.h",
"accessibility/accessibility_delegate.h",
"accessibility/focus_ring_controller.h",
"app_list/app_list_controller_impl.h",
Expand Down Expand Up @@ -124,7 +124,7 @@ component("ash") {
"accelerometer/accelerometer_reader.h",
"accelerometer/accelerometer_types.cc",
"accelerometer/accelerometer_types.h",
"accessibility/accessibility_controller.cc",
"accessibility/accessibility_controller_impl.cc",
"accessibility/accessibility_cursor_ring_layer.cc",
"accessibility/accessibility_cursor_ring_layer.h",
"accessibility/accessibility_focus_ring.cc",
Expand Down Expand Up @@ -1381,6 +1381,7 @@ component("ash") {
"//third_party/qcms",
"//third_party/re2",
"//ui/accessibility",
"//ui/accessibility:ax_enums_mojo",
"//ui/base",
"//ui/base:ui_data_pack",
"//ui/base/ime/chromeos",
Expand Down
13 changes: 6 additions & 7 deletions ash/accelerators/accelerator_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include "ash/accelerators/accelerator_commands.h"
#include "ash/accelerators/accelerator_confirmation_dialog.h"
#include "ash/accelerators/debug_commands.h"
#include "ash/accessibility/accessibility_controller.h"
#include "ash/accessibility/accessibility_controller_impl.h"
#include "ash/app_list/app_list_controller_impl.h"
#include "ash/assistant/assistant_controller.h"
#include "ash/assistant/assistant_ui_controller.h"
Expand All @@ -38,7 +38,6 @@
#include "ash/public/cpp/notification_utils.h"
#include "ash/public/cpp/toast_data.h"
#include "ash/public/cpp/voice_interaction_controller.h"
#include "ash/public/interfaces/accessibility_controller.mojom.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/root_window_controller.h"
#include "ash/rotator/window_rotation.h"
Expand Down Expand Up @@ -965,7 +964,7 @@ bool CanHandleToggleDictation() {
void HandleToggleDictation() {
base::RecordAction(UserMetricsAction("Accel_Toggle_Dictation"));
Shell::Get()->accessibility_controller()->ToggleDictationFromSource(
mojom::DictationToggleSource::kKeyboard);
DictationToggleSource::kKeyboard);
}

bool CanHandleToggleOverview() {
Expand Down Expand Up @@ -1074,7 +1073,7 @@ void SetFullscreenMagnifierEnabled(bool enabled) {
}

void SetHighContrastEnabled(bool enabled) {
AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();
accessibility_controller->SetHighContrastEnabled(enabled);
// Value could differ from one that were set because of higher-priority pref
Expand All @@ -1095,7 +1094,7 @@ void SetHighContrastEnabled(bool enabled) {
void HandleToggleHighContrast() {
base::RecordAction(UserMetricsAction("Accel_Toggle_High_Contrast"));

AccessibilityController* controller =
AccessibilityControllerImpl* controller =
Shell::Get()->accessibility_controller();
const bool current_enabled = controller->high_contrast_enabled();
const bool dialog_ever_accepted =
Expand Down Expand Up @@ -1142,7 +1141,7 @@ void HandleToggleFullscreenMagnifier() {
void HandleToggleSpokenFeedback() {
base::RecordAction(UserMetricsAction("Accel_Toggle_Spoken_Feedback"));

AccessibilityController* controller =
AccessibilityControllerImpl* controller =
Shell::Get()->accessibility_controller();
bool old_value = controller->spoken_feedback_enabled();
controller->SetSpokenFeedbackEnabled(!controller->spoken_feedback_enabled(),
Expand Down Expand Up @@ -2060,7 +2059,7 @@ AcceleratorControllerImpl::GetAcceleratorProcessingRestriction(
->BuildMruWindowList(kActiveDesk)
.empty()) {
Shell::Get()->accessibility_controller()->TriggerAccessibilityAlert(
mojom::AccessibilityAlert::WINDOW_NEEDED);
AccessibilityAlert::WINDOW_NEEDED);
return RESTRICTION_PREVENT_PROCESSING_AND_PROPAGATION;
}
return RESTRICTION_NONE;
Expand Down
40 changes: 15 additions & 25 deletions ash/accelerators/accelerator_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "ash/accelerators/accelerator_confirmation_dialog.h"
#include "ash/accelerators/accelerator_table.h"
#include "ash/accelerators/pre_target_accelerator_handler.h"
#include "ash/accessibility/accessibility_controller.h"
#include "ash/accessibility/accessibility_controller_impl.h"
#include "ash/accessibility/test_accessibility_controller_client.h"
#include "ash/app_list/app_list_metrics.h"
#include "ash/app_list/test/app_list_test_helper.h"
Expand Down Expand Up @@ -553,7 +553,7 @@ TEST_F(AcceleratorControllerTest, RotateScreen) {
display::Display::Rotation initial_rotation =
GetActiveDisplayRotation(display.id());
ui::test::EventGenerator* generator = GetEventGenerator();
AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();

EXPECT_FALSE(accessibility_controller
Expand Down Expand Up @@ -860,7 +860,7 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) {
}

TEST_F(AcceleratorControllerTest, GlobalAcceleratorsToggleAppList) {
AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();

// The press event should not toggle the AppList, the release should instead.
Expand Down Expand Up @@ -1625,20 +1625,16 @@ TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) {
}

TEST_F(AcceleratorControllerTest, DisallowedWithNoWindow) {
TestAccessibilityControllerClient client;
AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();
accessibility_controller->SetClient(client.CreateInterfacePtrAndBind());
TestAccessibilityControllerClient client;

for (size_t i = 0; i < kActionsNeedingWindowLength; ++i) {
accessibility_controller->TriggerAccessibilityAlert(
mojom::AccessibilityAlert::NONE);
accessibility_controller->FlushMojoForTest();
AccessibilityAlert::NONE);
EXPECT_TRUE(
controller_->PerformActionIfEnabled(kActionsNeedingWindow[i], {}));
accessibility_controller->FlushMojoForTest();
EXPECT_EQ(mojom::AccessibilityAlert::WINDOW_NEEDED,
client.last_a11y_alert());
EXPECT_EQ(AccessibilityAlert::WINDOW_NEEDED, client.last_a11y_alert());
}

// Make sure we don't alert if we do have a window.
Expand All @@ -1647,12 +1643,9 @@ TEST_F(AcceleratorControllerTest, DisallowedWithNoWindow) {
window.reset(CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
wm::ActivateWindow(window.get());
accessibility_controller->TriggerAccessibilityAlert(
mojom::AccessibilityAlert::NONE);
accessibility_controller->FlushMojoForTest();
AccessibilityAlert::NONE);
controller_->PerformActionIfEnabled(kActionsNeedingWindow[i], {});
accessibility_controller->FlushMojoForTest();
EXPECT_NE(mojom::AccessibilityAlert::WINDOW_NEEDED,
client.last_a11y_alert());
EXPECT_NE(AccessibilityAlert::WINDOW_NEEDED, client.last_a11y_alert());
}

// Don't alert if we have a minimized window either.
Expand All @@ -1661,19 +1654,16 @@ TEST_F(AcceleratorControllerTest, DisallowedWithNoWindow) {
wm::ActivateWindow(window.get());
controller_->PerformActionIfEnabled(WINDOW_MINIMIZE, {});
accessibility_controller->TriggerAccessibilityAlert(
mojom::AccessibilityAlert::NONE);
accessibility_controller->FlushMojoForTest();
AccessibilityAlert::NONE);
controller_->PerformActionIfEnabled(kActionsNeedingWindow[i], {});
accessibility_controller->FlushMojoForTest();
EXPECT_NE(mojom::AccessibilityAlert::WINDOW_NEEDED,
client.last_a11y_alert());
EXPECT_NE(AccessibilityAlert::WINDOW_NEEDED, client.last_a11y_alert());
}
}

TEST_F(AcceleratorControllerTest, TestDialogCancel) {
ui::Accelerator accelerator(ui::VKEY_H,
ui::EF_COMMAND_DOWN | ui::EF_CONTROL_DOWN);
AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();
// Pressing cancel on the dialog should have no effect.
EXPECT_FALSE(
Expand All @@ -1693,7 +1683,7 @@ TEST_F(AcceleratorControllerTest, TestToggleHighContrast) {
ui::EF_COMMAND_DOWN | ui::EF_CONTROL_DOWN);
// High Contrast Mode Enabled dialog and notification should be shown.
EXPECT_FALSE(IsConfirmationDialogOpen());
AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();
EXPECT_FALSE(
accessibility_controller->HasHighContrastAcceleratorDialogBeenAccepted());
Expand Down Expand Up @@ -1900,7 +1890,7 @@ TEST_F(MagnifiersAcceleratorsTester, TestToggleFullscreenMagnifier) {
EXPECT_FALSE(fullscreen_magnifier_controller()->IsEnabled());
EXPECT_FALSE(IsConfirmationDialogOpen());

AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();
// Toggle the fullscreen magnifier on/off, dialog should be shown on first use
// of accelerator.
Expand Down Expand Up @@ -1942,7 +1932,7 @@ TEST_F(MagnifiersAcceleratorsTester, TestToggleDockedMagnifier) {
EXPECT_FALSE(fullscreen_magnifier_controller()->IsEnabled());
EXPECT_FALSE(IsConfirmationDialogOpen());

AccessibilityController* accessibility_controller =
AccessibilityControllerImpl* accessibility_controller =
Shell::Get()->accessibility_controller();
// Toggle the docked magnifier on/off, dialog should be shown on first use of
// accelerator.
Expand Down
4 changes: 2 additions & 2 deletions ash/accelerators/spoken_feedback_toggler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <utility>

#include "ash/accelerators/key_hold_detector.h"
#include "ash/accessibility/accessibility_controller.h"
#include "ash/accessibility/accessibility_controller_impl.h"
#include "ash/shell.h"
#include "ui/events/event.h"

Expand Down Expand Up @@ -52,7 +52,7 @@ bool SpokenFeedbackToggler::ShouldStopEventPropagation() const {
void SpokenFeedbackToggler::OnKeyHold(const ui::KeyEvent* event) {
if (!toggled_) {
toggled_ = true;
AccessibilityController* controller =
AccessibilityControllerImpl* controller =
Shell::Get()->accessibility_controller();
controller->SetSpokenFeedbackEnabled(!controller->spoken_feedback_enabled(),
A11Y_NOTIFICATION_SHOW);
Expand Down
4 changes: 2 additions & 2 deletions ash/accelerators/spoken_feedback_toggler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

#include "ash/accelerators/spoken_feedback_toggler.h"
#include "ash/accessibility/accessibility_controller.h"
#include "ash/accessibility/accessibility_controller_impl.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "ash/wm/window_util.h"
Expand All @@ -17,7 +17,7 @@ using SpokenFeedbackTogglerTest = AshTestBase;

TEST_F(SpokenFeedbackTogglerTest, Basic) {
SpokenFeedbackToggler::ScopedEnablerForTest scoped;
AccessibilityController* controller =
AccessibilityControllerImpl* controller =
Shell::Get()->accessibility_controller();
ui::test::EventGenerator* generator = GetEventGenerator();
EXPECT_FALSE(controller->spoken_feedback_enabled());
Expand Down
Loading

0 comments on commit d13e95a

Please sign in to comment.