Skip to content

Commit

Permalink
Revert "Re-land: Move focus rings and highlight layers under accessib…
Browse files Browse the repository at this point in the history
…ility bubbles."

This reverts commit 7ffd10d.

Reason for revert: [Sheriff] Suspected to be causing widespread accessibility test failures on MSAN bots due to use of uninitialized value in code that was added to CreateOrUpdateLayer in this CL (line 73, presumably for `stack_at_top`)

Build:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/21677

Sample failure:

[ RUN      ] AccessibilityFocusRingControllerTest.FocusRingWorksOnMultipleDisplays
2020-12-11T22:10:55.266324Z 8322331852 ERROR ash_unittests[24691:24691]: [cras_audio_handler.cc(1611)] Failed to retrieve WBS mic deprioritized flag
==24691==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5653d294dffa in ash::AccessibilityLayer::CreateOrUpdateLayer(aura::Window*, char const*, gfx::Rect const&, bool) ./../../ash/accessibility/accessibility_layer.cc:73:7
    #1 0x5653d294788e in ash::AccessibilityFocusRingLayer::Set(ash::AccessibilityFocusRing const&) ./../../ash/accessibility/accessibility_focus_ring_layer.cc:116:3
    #2 0x5653d29383cb in ash::AccessibilityFocusRingGroup::UpdateFocusRingsFromInfo(ash::AccessibilityLayerDelegate*) ./../../ash/accessibility/accessibility_focus_ring_group.cc:82:25
    #3 0x5653d293baa9 in ash::AccessibilityFocusRingGroup::UpdateFocusRing(std::__1::unique_ptr<ash::AccessibilityFocusRingInfo, std::__1::default_delete<ash::AccessibilityFocusRingInfo> >, ash::AccessibilityLayerDelegate*) ./../../ash/accessibility/accessibility_focus_ring_group.cc:152:3
    #4 0x5653d2932f04 in ash::AccessibilityFocusRingControllerImpl::SetFocusRing(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::unique_ptr<ash::AccessibilityFocusRingInfo, std::__1::default_delete<ash::AccessibilityFocusRingInfo> >) ./../../ash/accessibility/accessibility_focus_ring_controller_impl.cc:67:25
    #5 0x5653cda0dbe2 in ash::AccessibilityFocusRingControllerTest_FocusRingWorksOnMultipleDisplays_Test::TestBody() ./../../ash/accessibility/accessibility_focus_ring_controller_unittest.cc:106:15
    #6 0x5653d24cc4a7 in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #7 0x5653d24cc4a7 in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2689:5
    #8 0x5653d24ceb57 in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2866:11
    #9 0x5653d24d0e64 in testing::TestSuite::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:3020:28
    #10 0x5653d24fb28a in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5730:44
    #11 0x5653d24fa26c in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10
    #12 0x5653d24fa26c in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:5313:10
    #13 0x5653d3c13224 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2485:46
    #14 0x5653d3c13224 in base::TestSuite::Run() ./../../base/test/test_suite.cc:480:16
    #15 0x5653d3c1addf in Run ./../../base/callback.h:101:12
    #16 0x5653d3c1addf in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, unsigned long, bool, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:179:38
    #17 0x5653d3c1a853 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>, unsigned long) ./../../base/test/launcher/unit_test_launcher.cc:249:10
    #18 0x5653cf8bc976 in main ./../../ash/test/ash_unittests.cc:25:10
    #19 0x7fe4936dd83f in __libc_start_main ??:0:0
    #20 0x5653cd7f7409 in _start ??:0:0

  Uninitialized value was created by a heap allocation
    #0 0x5653cd86a7c9 in operator new(unsigned long) /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/msan/msan_new_delete.cpp:45:35
    #1 0x5653d2938051 in make_unique<ash::AccessibilityFocusRingLayer, ash::AccessibilityLayerDelegate *&> ./../../buildtools/third_party/libc++/trunk/include/memory:3043:28
    #2 0x5653d2938051 in ash::AccessibilityFocusRingGroup::UpdateFocusRingsFromInfo(ash::AccessibilityLayerDelegate*) ./../../ash/accessibility/accessibility_focus_ring_group.cc:69:11
    #3 0x5653d293baa9 in ash::AccessibilityFocusRingGroup::UpdateFocusRing(std::__1::unique_ptr<ash::AccessibilityFocusRingInfo, std::__1::default_delete<ash::AccessibilityFocusRingInfo> >, ash::AccessibilityLayerDelegate*) ./../../ash/accessibility/accessibility_focus_ring_group.cc:152:3
    #4 0x5653d2932f04 in ash::AccessibilityFocusRingControllerImpl::SetFocusRing(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::unique_ptr<ash::AccessibilityFocusRingInfo, std::__1::default_delete<ash::AccessibilityFocusRingInfo> >) ./../../ash/accessibility/accessibility_focus_ring_controller_impl.cc:67:25
    #5 0x5653cda0dbe2 in ash::AccessibilityFocusRingControllerTest_FocusRingWorksOnMultipleDisplays_Test::TestBody() ./../../ash/accessibility/accessibility_focus_ring_controller_unittest.cc:106:15
    #6 0x5653d24cc4a7 in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #7 0x5653d24cc4a7 in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2689:5
    #8 0x5653d24ceb57 in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2866:11
    #9 0x5653d24d0e64 in testing::TestSuite::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:3020:28
    #10 0x5653d24fb28a in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5730:44
    #11 0x5653d24fa26c in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10
    #12 0x5653d24fa26c in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:5313:10
    #13 0x5653d3c13224 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2485:46
    #14 0x5653d3c13224 in base::TestSuite::Run() ./../../base/test/test_suite.cc:480:16
    #15 0x5653d3c1addf in Run ./../../base/callback.h:101:12
    #16 0x5653d3c1addf in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, unsigned long, bool, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:179:38
    #17 0x5653d3c1a853 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>, unsigned long) ./../../base/test/launcher/unit_test_launcher.cc:249:10
    #18 0x5653cf8bc976 in main ./../../ash/test/ash_unittests.cc:25:10
    #19 0x7fe4936dd83f in __libc_start_main ??:0:0

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/b/s/w/ir/out/Release/ash_unittests+0x65feffa)


Original change's description:
> Re-land: Move focus rings and highlight layers under accessibility bubbles.
>
> Original patch: http://crrev.com/c/2568547
> Revert: http://crrev.com/c/2583106
>
> Fix for issue where focus ring appears below switch access panel. Now clients can specify if they want their focus ring to appear above or below accessibility bubble panels. STS will specify they appear below, though other clients will (by default) specify they appear above all panels. This fixes an issue with the switch access focus ring didn't appear above the switch access panel.
>
> Original description:
>
> Move focus rings and highlight layers under accessibility bubbles.
>
> Will allow Select-to-speak panel UI to appear above focus ring and highlight layers. This also avoids the need to create more window containers.
>
> Bug: 1157261, 1143814
> Change-Id: Ibd8dd577ecd4fb5767b628c2eff679a2f6931b9c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582997
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Joel Riley <joelriley@google.com>
> Cr-Commit-Position: refs/heads/master@{#836222}

TBR=xiyuan@chromium.org,dmazzoni@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,shannc@google.com,joelriley@google.com

Change-Id: I08b819849d13fb5d01f67c5bbd3083ed0e445e7e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1157261
Bug: 1143814
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587604
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836386}
  • Loading branch information
Alex Moshchuk authored and Chromium LUCI CQ committed Dec 12, 2020
1 parent 463b5fd commit 4a0a6a0
Show file tree
Hide file tree
Showing 16 changed files with 23 additions and 118 deletions.
3 changes: 1 addition & 2 deletions ash/accessibility/accessibility_cursor_ring_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ void AccessibilityCursorRingLayer::Set(const gfx::Point& location) {
display::Screen::GetScreen()->GetDisplayMatching(bounds);
aura::Window* root_window = Shell::GetRootWindowForDisplayId(display.id());
::wm::ConvertRectFromScreen(root_window, &bounds);
CreateOrUpdateLayer(root_window, "AccessibilityCursorRing", bounds,
/*stack_at_top=*/true);
CreateOrUpdateLayer(root_window, "AccessibilityCursorRing", bounds);
}

void AccessibilityCursorRingLayer::OnPaintLayer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include "ash/accessibility/accessibility_cursor_ring_layer.h"
#include "ash/public/cpp/accessibility_focus_ring_info.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -111,9 +110,7 @@ TEST_F(AccessibilityFocusRingControllerTest, FocusRingWorksOnMultipleDisplays) {
controller->GetFocusRingGroupForTesting("catsRCute")
->focus_layers_for_testing();
EXPECT_EQ(1u, layers.size());
aura::Window* window0_container = Shell::GetContainer(
root_windows[0], kShellWindowId_AccessibilityBubbleContainer);
EXPECT_EQ(window0_container, layers[0]->root_window());
EXPECT_EQ(root_windows[0], layers[0]->root_window());
// The focus ring has some padding, so just check the center point is where
// we would expect it.
EXPECT_EQ(layers[0]->layer()->GetTargetBounds().CenterPoint(),
Expand All @@ -130,9 +127,7 @@ TEST_F(AccessibilityFocusRingControllerTest, FocusRingWorksOnMultipleDisplays) {
moved_layers = controller->GetFocusRingGroupForTesting("catsRCute")
->focus_layers_for_testing();
EXPECT_EQ(1u, moved_layers.size());
aura::Window* window1_container = Shell::GetContainer(
root_windows[1], kShellWindowId_AccessibilityBubbleContainer);
EXPECT_EQ(window1_container, moved_layers[0]->root_window());
EXPECT_EQ(root_windows[1], moved_layers[0]->root_window());
EXPECT_EQ(moved_layers[0]->layer()->GetTargetBounds().CenterPoint(),
gfx::Rect(100, 50, 10, 10).CenterPoint());
}
Expand Down
5 changes: 2 additions & 3 deletions ash/accessibility/accessibility_focus_ring_group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ void AccessibilityFocusRingGroup::UpdateFocusRingsFromInfo(

for (size_t i = 0; i < focus_rings_.size(); ++i) {
focus_layers_[i]->SetAppearance(
focus_ring_info_->type, focus_ring_info_->stacking_order,
focus_ring_info_->color, focus_ring_info_->secondary_color,
focus_ring_info_->background_color);
focus_ring_info_->type, focus_ring_info_->color,
focus_ring_info_->secondary_color, focus_ring_info_->background_color);
}
}

Expand Down
21 changes: 6 additions & 15 deletions ash/accessibility/accessibility_focus_ring_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "ash/accessibility/accessibility_focus_ring_layer.h"

#include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h"
#include "base/bind.h"
#include "third_party/skia/include/core/SkPaint.h"
Expand Down Expand Up @@ -101,31 +100,23 @@ void AccessibilityFocusRingLayer::Set(const AccessibilityFocusRing& ring) {
display::Display display =
display::Screen::GetScreen()->GetDisplayMatching(bounds);
aura::Window* root_window = Shell::GetRootWindowForDisplayId(display.id());
aura::Window* container = Shell::GetContainer(
root_window, kShellWindowId_AccessibilityBubbleContainer);

if (SkColorGetA(background_color_) > 0) {
bounds = display.bounds();
} else {
int inset = kGradientWidth;
bounds.Inset(-inset, -inset, -inset, -inset);
}
::wm::ConvertRectFromScreen(container, &bounds);
bool stack_at_top =
(stacking_order_ == FocusRingStackingOrder::ABOVE_ACCESSIBILITY_BUBBLES);
CreateOrUpdateLayer(container, "AccessibilityFocusRing", bounds,
stack_at_top);
::wm::ConvertRectFromScreen(root_window, &bounds);
CreateOrUpdateLayer(root_window, "AccessibilityFocusRing", bounds);
}

void AccessibilityFocusRingLayer::SetAppearance(
FocusRingType type,
FocusRingStackingOrder stacking_order,
SkColor color,
SkColor secondary_color,
SkColor background_color) {
void AccessibilityFocusRingLayer::SetAppearance(FocusRingType type,
SkColor color,
SkColor secondary_color,
SkColor background_color) {
SetColor(color);
type_ = type;
stacking_order_ = stacking_order;
secondary_color_ = secondary_color;
background_color_ = background_color;
}
Expand Down
3 changes: 0 additions & 3 deletions ash/accessibility/accessibility_focus_ring_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class ASH_EXPORT AccessibilityFocusRingLayer : public FocusRingLayer {
void Set(const AccessibilityFocusRing& ring);

void SetAppearance(FocusRingType type,
FocusRingStackingOrder stacking_order,
SkColor color,
SkColor secondary_color,
SkColor background_alpha);
Expand All @@ -46,8 +45,6 @@ class ASH_EXPORT AccessibilityFocusRingLayer : public FocusRingLayer {
AccessibilityFocusRing ring_;
// The type of focus ring.
FocusRingType type_;
// How the focus ring should be stacked relative to other layers.
FocusRingStackingOrder stacking_order_;
// The secondary color.
SkColor secondary_color_;
// The color of the background. When fully transparent, no background will be
Expand Down
8 changes: 2 additions & 6 deletions ash/accessibility/accessibility_highlight_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "ash/accessibility/accessibility_highlight_layer.h"

#include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h"
#include "third_party/skia/include/core/SkPaint.h"
#include "third_party/skia/include/core/SkPath.h"
Expand Down Expand Up @@ -50,11 +49,8 @@ void AccessibilityHighlightLayer::Set(const std::vector<gfx::Rect>& rects,
display::Display display =
display::Screen::GetScreen()->GetDisplayMatching(bounds);
aura::Window* root_window = Shell::GetRootWindowForDisplayId(display.id());
aura::Window* container = Shell::GetContainer(
root_window, kShellWindowId_AccessibilityPanelContainer);
::wm::ConvertRectFromScreen(container, &bounds);
CreateOrUpdateLayer(container, "AccessibilityHighlight", bounds,
/*stack_at_top=*/false);
::wm::ConvertRectFromScreen(root_window, &bounds);
CreateOrUpdateLayer(root_window, "AccessibilityHighlight", bounds);
}

bool AccessibilityHighlightLayer::CanAnimate() const {
Expand Down
15 changes: 4 additions & 11 deletions ash/accessibility/accessibility_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,13 @@ AccessibilityLayer::~AccessibilityLayer() {
}

void AccessibilityLayer::Set(aura::Window* root_window,
const gfx::Rect& bounds,
bool stack_at_top) {
const gfx::Rect& bounds) {
DCHECK(root_window);
layer_rect_ = bounds;
gfx::Rect layer_bounds = bounds;
int inset = -(GetInset());
layer_bounds.Inset(inset, inset, inset, inset);
CreateOrUpdateLayer(root_window, "AccessibilityLayer", layer_bounds,
stack_at_top);
CreateOrUpdateLayer(root_window, "AccessibilityLayer", layer_bounds);
}

void AccessibilityLayer::SetOpacity(float opacity) {
Expand All @@ -53,8 +51,7 @@ void AccessibilityLayer::SetSubpixelPositionOffset(

void AccessibilityLayer::CreateOrUpdateLayer(aura::Window* root_window,
const char* layer_name,
const gfx::Rect& bounds,
bool stack_at_top) {
const gfx::Rect& bounds) {
if (!layer_ || root_window != root_window_) {
root_window_ = root_window;
ui::Layer* root_layer = root_window->layer();
Expand All @@ -70,11 +67,7 @@ void AccessibilityLayer::CreateOrUpdateLayer(aura::Window* root_window,

// Keep moving it to the top in case new layers have been added
// since we created this layer.
if (stack_at_top) {
layer_->parent()->StackAtTop(layer_.get());
} else {
layer_->parent()->StackAtBottom(layer_.get());
}
layer_->parent()->StackAtTop(layer_.get());

layer_->SetBounds(bounds);
gfx::Rect layer_bounds(0, 0, bounds.width(), bounds.height());
Expand Down
9 changes: 3 additions & 6 deletions ash/accessibility/accessibility_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ class AccessibilityLayer : public ui::LayerDelegate,

// Move the accessibility layer to the given bounds in the coordinates of
// the given root window.
void Set(aura::Window* root_window,
const gfx::Rect& bounds,
bool stack_at_top);
void Set(aura::Window* root_window, const gfx::Rect& bounds);

// Set the layer's opacity.
void SetOpacity(float opacity);
Expand All @@ -72,11 +70,10 @@ class AccessibilityLayer : public ui::LayerDelegate,
protected:
// Updates |root_window_| and creates |layer_| if it doesn't exist,
// or if the root window has changed. Moves the layer to the top if
// |stack_at_top| is true, otherwise moves layer to the bottom.
// it wasn't there already.
void CreateOrUpdateLayer(aura::Window* root_window,
const char* layer_name,
const gfx::Rect& bounds,
bool stack_at_top);
const gfx::Rect& bounds);

// The current root window containing the focused object.
aura::Window* root_window_ = nullptr;
Expand Down
6 changes: 1 addition & 5 deletions ash/accessibility/focus_ring_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#include "ash/accessibility/focus_ring_controller.h"

#include "ash/accessibility/focus_ring_layer.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h"
#include "ash/system/tray/actionable_view.h"
#include "ash/system/tray/tray_background_view.h"
#include "ash/wm/window_util.h"
Expand Down Expand Up @@ -85,9 +83,7 @@ void FocusRingController::UpdateFocusRing() {
// Update the focus ring layer.
if (!focus_ring_layer_)
focus_ring_layer_.reset(new FocusRingLayer(this));
aura::Window* container = Shell::GetContainer(
root_window, kShellWindowId_AccessibilityBubbleContainer);
focus_ring_layer_->Set(container, view_bounds, /*stack_at_top=*/true);
focus_ring_layer_->Set(root_window, view_bounds);
}

void FocusRingController::OnDeviceScaleFactorChanged() {
Expand Down
3 changes: 1 addition & 2 deletions ash/accessibility/point_scan_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ PointScanLayer::PointScanLayer(AccessibilityLayerDelegate* delegate)
: AccessibilityLayer(delegate) {
aura::Window* root_window =
Shell::GetRootWindowForDisplayId(GetPrimaryDisplay().id());
CreateOrUpdateLayer(root_window, "PointScanning", gfx::Rect(),
/*stack_at_top=*/true);
CreateOrUpdateLayer(root_window, "PointScanning", gfx::Rect());
SetOpacity(1.0);
bounds_ = GetPrimaryDisplay().bounds();
layer()->SetBounds(bounds_);
Expand Down
3 changes: 1 addition & 2 deletions ash/public/cpp/accessibility_focus_ring_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ bool AccessibilityFocusRingInfo::operator==(
const AccessibilityFocusRingInfo& other) const {
return rects_in_screen == other.rects_in_screen &&
behavior == other.behavior && type == other.type &&
stacking_order == other.stacking_order && color == other.color &&
secondary_color == other.secondary_color &&
color == other.color && secondary_color == other.secondary_color &&
background_color == other.background_color;
}

Expand Down
10 changes: 0 additions & 10 deletions ash/public/cpp/accessibility_focus_ring_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@ enum class ASH_PUBLIC_EXPORT FocusRingBehavior { FADE_OUT, PERSIST };
// The visual style of the focus ring.
enum class ASH_PUBLIC_EXPORT FocusRingType { GLOW, SOLID, DASHED };

// How focus rings are layered.
enum class ASH_PUBLIC_EXPORT FocusRingStackingOrder {
// Above most UI, including accessibility bubble panels.
ABOVE_ACCESSIBILITY_BUBBLES,
// Above most UI, except below accessibility bubble panels.
BELOW_ACCESSIBILITY_BUBBLES
};

// Defines a specific focus ring by specifying:
// - |rects_in_screen| the regions around which to draw the focus ring (in
// screen coordinates).
Expand All @@ -48,8 +40,6 @@ struct ASH_PUBLIC_EXPORT AccessibilityFocusRingInfo {
std::vector<gfx::Rect> rects_in_screen;
FocusRingBehavior behavior = FocusRingBehavior::FADE_OUT;
FocusRingType type = FocusRingType::GLOW;
FocusRingStackingOrder stacking_order =
FocusRingStackingOrder::ABOVE_ACCESSIBILITY_BUBBLES;
SkColor color = SK_ColorTRANSPARENT;
SkColor secondary_color = SK_ColorTRANSPARENT;
SkColor background_color = SK_ColorTRANSPARENT;
Expand Down
17 changes: 0 additions & 17 deletions chrome/browser/accessibility/accessibility_extension_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,23 +149,6 @@ AccessibilityPrivateSetFocusRingsFunction::Run() {
NOTREACHED();
}

if (focus_ring_info.stacking_order) {
switch (focus_ring_info.stacking_order) {
case accessibility_private::
FOCUS_RING_STACKING_ORDER_ABOVEACCESSIBILITYBUBBLES:
focus_ring->stacking_order =
ash::FocusRingStackingOrder::ABOVE_ACCESSIBILITY_BUBBLES;
break;
case accessibility_private::
FOCUS_RING_STACKING_ORDER_BELOWACCESSIBILITYBUBBLES:
focus_ring->stacking_order =
ash::FocusRingStackingOrder::BELOW_ACCESSIBILITY_BUBBLES;
break;
default:
NOTREACHED();
}
}

if (focus_ring_info.background_color &&
!extensions::image_util::ParseHexColorString(
*(focus_ring_info.background_color),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ var RoleType = chrome.automation.RoleType;
const AccessibilityFeature = chrome.accessibilityPrivate.AccessibilityFeature;
const SelectToSpeakPanelAction =
chrome.accessibilityPrivate.SelectToSpeakPanelAction;
const FocusRingStackingOrder =
chrome.accessibilityPrivate.FocusRingStackingOrder;

// This must be the same as in ash/system/accessibility/select_to_speak_tray.cc:
// ash::kSelectToSpeakTrayClassName.
Expand Down Expand Up @@ -765,15 +763,9 @@ class SelectToSpeak {
if (drawBackground && this.prefsManager_.backgroundShadingEnabled()) {
color = DEFAULT_BACKGROUND_SHADING_COLOR;
}
// If we're also showing a navigation panel, ensure the focus ring appears
// below the panel UI.
const stackingOrder = this.shouldShowNavigationControls_() ?
FocusRingStackingOrder.BELOW_ACCESSIBILITY_BUBBLES :
FocusRingStackingOrder.ABOVE_ACCESSIBILITY_BUBBLES;
chrome.accessibilityPrivate.setFocusRings([{
rects,
type: chrome.accessibilityPrivate.FocusType.GLOW,
stackingOrder,
color: this.prefsManager_.focusRingColor(),
backgroundColor: color,
}]);
Expand Down
11 changes: 0 additions & 11 deletions chrome/common/extensions/api/accessibility_private.json
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,6 @@
"description": "The type of visual appearance for the focus ring.",
"enum": ["glow", "solid", "dashed"]
},
{
"id": "FocusRingStackingOrder",
"type": "string",
"description": "Whether to stack focus rings above or below accessibility bubble panels. Note: focus rings will be stacked above most other UI in either case",
"enum": ["aboveAccessibilityBubbles", "belowAccessibilityBubbles"]
},
{
"id": "FocusRingInfo",
"type": "object",
Expand Down Expand Up @@ -191,11 +185,6 @@
"description": "A RGB hex-value color string (e.g. #803F82E4) that describes the color drawn outside of the focus ring and over the rest of the display.",
"optional": true
},
"stackingOrder": {
"$ref": "FocusRingStackingOrder",
"description": "The FocusType for the ring.",
"optional": true
},
"id": {
"type": "string",
"description": "An identifier for this focus ring, unique within the extension.",
Expand Down
10 changes: 0 additions & 10 deletions third_party/closure_compiler/externs/accessibility_private.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,23 +197,13 @@ chrome.accessibilityPrivate.FocusType = {
DASHED: 'dashed',
};

/**
* @enum {string}
*/
chrome.accessibilityPrivate.FocusRingStackingOrder = {
ABOVE_ACCESSIBILITY_BUBBLES: 'aboveAccessibilityBubbles',
BELOW_ACCESSIBILITY_BUBBLES: 'belowAccessibilityBubbles',
};

/**
* @typedef {{
* rects: !Array<!chrome.accessibilityPrivate.ScreenRect>,
* type: !chrome.accessibilityPrivate.FocusType,
* color: string,
* secondaryColor: (string|undefined),
* backgroundColor: (string|undefined),
* stackingOrder:
* (!chrome.accessibilityPrivate.FocusRingStackingOrder|undefined),
* id: (string|undefined)
* }}
*/
Expand Down

0 comments on commit 4a0a6a0

Please sign in to comment.