• Alex Moshchuk's avatar
    Revert "Re-land: Move focus rings and highlight layers under accessibility bubbles." · 4a0a6a0e
    Alex Moshchuk authored
    This reverts commit 7ffd10d3.
    
    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/+/2587604Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
    Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#836386}
    4a0a6a0e
accessibility_cursor_ring_layer.cc 2.44 KB