1. 15 Aug, 2018 39 commits
  2. 14 Aug, 2018 1 commit
    • Chris Mumford's avatar
      Revert "cros: Enable touchable app context menus by default." · 32cd8fcf
      Chris Mumford authored
      This reverts commit b58ccf6d.
      
      Reason for revert: Possibly introduced a use-after-free bug.
      
      This is a speculative fix for a use-after-free bug in two ash_unittests:
      
      1. OverflowButtonInkDropTest.MouseContextMenu
      2. OverflowButtonActiveInkDropTest.MouseContextMenu
      
      [ RUN      ] OverflowButtonInkDropTest.MouseContextMenu
      =================================================================
      ==28149==ERROR: AddressSanitizer: heap-use-after-free on address 0x613000031f68 at pc 0x000008c82106 bp 0x7ffea3cd7190 sp 0x7ffea3cd7188
      READ of size 8 at 0x613000031f68 thread T0
          #0 0x8c82105 in begin buildtools/third_party/libc++/trunk/include/vector:1479:30
          #1 0x8c82105 in begin<std::__1::vector<aura::WindowObserver *, std::__1::allocator<aura::WindowObserver *> > > buildtools/third_party/libc++/trunk/include/iterator:1670
          #2 0x8c82105 in ContainsValue<std::__1::vector<aura::WindowObserver *, std::__1::allocator<aura::WindowObserver *> >, const aura::WindowObserver *, 0> base/stl_util.h:168
          #3 0x8c82105 in HasObserver base/observer_list.h:267
          #4 0x8c82105 in aura::Window::HasObserver(aura::WindowObserver const*) const ui/aura/window.cc:543
          #5 0x8c9d363 in aura::WindowObserver::OnUnobservingWindow(aura::Window*) ui/aura/window_observer.cc:25:15
          #6 0x8c799d7 in aura::Window::RemoveObserver(aura::WindowObserver*) ui/aura/window.cc:538:13
          #7 0xc2f40e in RemoveAll ui/base/window_tracker_template.h:49:15
          #8 0xc2f40e in ui::WindowTrackerTemplate<aura::Window, aura::WindowObserver>::~WindowTrackerTemplate() ui/base/window_tracker_template.h:33
          #9 0x9225f3b in ~CompoundEventFilter ui/wm/core/compound_event_filter.cc:49:1
          #10 0x9225f3b in wm::CompoundEventFilter::~CompoundEventFilter() ui/wm/core/compound_event_filter.cc:45
          #11 0x6b0ae30 in operator() buildtools/third_party/libc++/trunk/include/memory:2321:5
          #12 0x6b0ae30 in reset buildtools/third_party/libc++/trunk/include/memory:2634
          #13 0x6b0ae30 in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2588
          #14 0x6b0ae30 in ash::Shell::~Shell() ash/shell.cc:969
          #15 0x6b0c81d in ash::Shell::~Shell() ash/shell.cc:737:17
          #16 0xc26b344 in ash::AshTestHelper::TearDown() ash/test/ash_test_helper.cc:294:3
          #17 0xc26444c in ash::AshTestBase::TearDown() ash/test/ash_test_base.cc:192:21
          #18 0x4724c64 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2682:11
          #19 0x4726036 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2800:28
          #20 0x474b566 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5124:43
          #21 0x474a7b5 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc
          #22 0x72b431a in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2331:46
          #23 0x72b431a in base::TestSuite::Run() base/test/test_suite.cc:277
          #24 0x72b9892 in Run base/callback.h:99:12
          #25 0x72b9892 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:224
          #26 0x72b9330 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:10
          #27 0x178df94 in main ash/test/ash_unittests.cc:37:10
          #28 0x7f78e2b62f44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287
      
      0x613000031f68 is located 296 bytes inside of 328-byte region [0x613000031e40,0x613000031f88)
      freed by thread T0 here:
          #0 0x6b9062 in operator delete(void*) /b/swarming/w/ir/kitchen-workdir/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:167:3
          #1 0x6a547a0 in ash::RootWindowController::CloseChildWindows() ash/root_window_controller.cc:498:7
          #2 0x6b0c5cf in ash::Shell::CloseAllRootWindowChildWindows() ash/shell.cc:1387:19
          #3 0x6b06637 in ash::Shell::~Shell() ash/shell.cc:837:3
          #4 0x6b0c81d in ash::Shell::~Shell() ash/shell.cc:737:17
          #5 0xc26b344 in ash::AshTestHelper::TearDown() ash/test/ash_test_helper.cc:294:3
          #6 0xc26444c in ash::AshTestBase::TearDown() ash/test/ash_test_base.cc:192:21
          #7 0x4724c64 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2682:11
          #8 0x4726036 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2800:28
          #9 0x474b566 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5124:43
          #10 0x474a7b5 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc
          #11 0x72b431a in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2331:46
          #12 0x72b431a in base::TestSuite::Run() base/test/test_suite.cc:277
          #13 0x72b9892 in Run base/callback.h:99:12
          #14 0x72b9892 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:224
          #15 0x72b9330 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:10
          #16 0x178df94 in main ash/test/ash_unittests.cc:37:10
          #17 0x7f78e2b62f44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287
      
      previously allocated by thread T0 here:
          #0 0x6b8422 in operator new(unsigned long) /b/swarming/w/ir/kitchen-workdir/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:106:3
          #1 0x91cb856 in views::NativeWidgetAura::NativeWidgetAura(views::internal::NativeWidgetDelegate*, bool, aura::Env*) ui/views/widget/native_widget_aura.cc:109:15
          #2 0x91d615d in views::internal::NativeWidgetPrivate::CreateNativeWidget(views::Widget::InitParams const&, views::internal::NativeWidgetDelegate*) ui/views/widget/native_widget_aura.cc:1113:14
          #3 0x917dabd in CreateNativeWidget ui/views/widget/widget.cc:73:10
          #4 0x917dabd in views::Widget::Init(views::Widget::InitParams const&) ui/views/widget/widget.cc:334
          #5 0x6af249a in ash::ShelfWidget::ShelfWidget(aura::Window*, ash::Shelf*) ash/shelf/shelf_widget.cc:216:3
          #6 0x6a96eaa in ash::Shelf::CreateShelfWidget(aura::Window*) ash/shelf/shelf.cc:73:27
          #7 0x6a5847a in ash::RootWindowController::InitLayoutManagers() ash/root_window_controller.cc:699:11
          #8 0x6a52166 in ash::RootWindowController::Init(ash::RootWindowController::RootWindowType) ash/root_window_controller.cc:663:3
          #9 0x68ef135 in ash::WindowTreeHostManager::InitHosts() ash/display/window_tree_host_manager.cc:238:3
          #10 0x6afadce in ash::Shell::Init(ui::ContextFactory*, ui::ContextFactoryPrivate*, std::__1::unique_ptr<base::Value, std::__1::default_delete<base::Value> >, std::__1::unique_ptr<ui::ws2::GpuInterfaceProvider, std::__1::default_delete<ui::ws2::GpuInterfaceProvider> >) ash/shell.cc:1250:30
          #11 0x6af7163 in ash::Shell::CreateInstance(ash::ShellInitParams) ash/shell.cc:276:14
          #12 0xc26ac31 in ash::AshTestHelper::CreateShell() ash/test/ash_test_helper.cc:390:3
          #13 0xc26a2a0 in ash::AshTestHelper::SetUp(bool, bool) ash/test/ash_test_helper.cc:245:3
          #14 0xc263f8d in ash::AshTestBase::SetUp() ash/test/ash_test_base.cc:159:21
          #15 0x1322b16 in ash::ShelfViewTest::SetUp() ash/shelf/shelf_view_unittest.cc:262:18
          #16 0x1326366 in SetUp ash/shelf/shelf_view_unittest.cc:2369:20
          #17 0x1326366 in ash::OverflowButtonInkDropTest::SetUp() ash/shelf/shelf_view_unittest.cc:2778
          #18 0x4722b42 in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc
          #19 0x4724c64 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2682:11
          #20 0x4726036 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2800:28
          #21 0x474b566 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5124:43
          #22 0x474a7b5 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc
          #23 0x72b431a in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2331:46
          #24 0x72b431a in base::TestSuite::Run() base/test/test_suite.cc:277
          #25 0x72b9892 in Run base/callback.h:99:12
          #26 0x72b9892 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:224
          #27 0x72b9330 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:10
          #28 0x178df94 in main ash/test/ash_unittests.cc:37:10
          #29 0x7f78e2b62f44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287
      
      SUMMARY: AddressSanitizer: heap-use-after-free buildtools/third_party/libc++/trunk/include/vector:1479:30 in begin
      Shadow bytes around the buggy address:
        0x0c267fffe390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        0x0c267fffe3a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        0x0c267fffe3b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
        0x0c267fffe3c0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
        0x0c267fffe3d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      =>0x0c267fffe3e0: fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd
        0x0c267fffe3f0: fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
        0x0c267fffe400: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
        0x0c267fffe410: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
        0x0c267fffe420: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
        0x0c267fffe430: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
      Shadow byte legend (one shadow byte represents 8 application bytes):
        Addressable:           00
        Partially addressable: 01 02 03 04 05 06 07 
        Heap left redzone:       fa
        Freed heap region:       fd
        Stack left redzone:      f1
        Stack mid redzone:       f2
        Stack right redzone:     f3
        Stack after return:      f5
        Stack use after scope:   f8
        Global redzone:          f9
        Global init order:       f6
        Poisoned by user:        f7
        Container overflow:      fc
        Array cookie:            ac
        Intra object redzone:    bb
        ASan internal:           fe
        Left alloca redzone:     ca
        Right alloca redzone:    cb
        Shadow gap:              cc
      ==28149==ABORTING
      
      Original change's description:
      > cros: Enable touchable app context menus by default.
      > 
      > This will enable:
      >  - New context menu UI for:
      >   - Application icons
      >   - Shelf context menus
      >   - Desktop context menus
      > 
      > Note:
      >  - Removed tests whose functionality were already being tested.
      >    - MouseContextMenu already tests whether the ink drop is being
      >      shown, so there is no need for
      >      ShelfButtonShowsInkDropHighlightOnMenuShow.
      > 
      > Bug: 871843
      > Change-Id: I4c92ae52f75b25bc9edd1dd67778c4785d8f4058
      > Reviewed-on: https://chromium-review.googlesource.com/1170985
      > Reviewed-by: Scott Violet <sky@chromium.org>
      > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
      > Commit-Queue: Alex Newcomer <newcomer@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#582978}
      
      TBR=xiyuan@chromium.org,sky@chromium.org,newcomer@chromium.org
      
      Change-Id: Ib85676861251f41c84f48f956d36c01edb353266
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Bug: 871843
      Reviewed-on: https://chromium-review.googlesource.com/1174641Reviewed-by: default avatarChris Mumford <cmumford@chromium.org>
      Commit-Queue: Chris Mumford <cmumford@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#583088}
      32cd8fcf