Reland Fix MenuController Heap-use-after-free

ui/views/controls/menu/menu_controller_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/menu/menu_controller.h"
 
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/single_thread_task_runner.h"
@@ skipping 11 matching lines @@
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/views/controls/menu/menu_controller_delegate.h"
 #include "ui/views/controls/menu/menu_delegate.h"
 #include "ui/views/controls/menu/menu_host.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_message_loop.h"
 #include "ui/views/controls/menu/menu_scroll_view_container.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/test/menu_test_utils.h"
+#include "ui/views/test/test_views_delegate.h"
 #include "ui/views/test/views_test_base.h"
 
 #if defined(USE_AURA)
 #include "ui/aura/client/drag_drop_client.h"
 #include "ui/aura/scoped_window_targeter.h"
 #include "ui/aura/window.h"
 #include "ui/views/controls/menu/menu_pre_target_handler.h"
 #endif
 
 #if defined(USE_X11)
@@ skipping 188 matching lines @@
 
 void TestDragDropClient::DragCancel() {
   drag_in_progress_ = false;
 }
 bool TestDragDropClient::IsDragDropInProgress() {
   return drag_in_progress_;
 }
 
 #endif  // defined(USE_AURA)
 
+// Test implementation of TestViewsDelegate which overrides ReleaseRef in order
+// to test destruction order. This simulates Chrome shutting down upon the
+// release of the ref. Associated tests should not crash.
+class DestructingTestViewsDelegate : public TestViewsDelegate {
+ public:
+  DestructingTestViewsDelegate() {}
+  ~DestructingTestViewsDelegate() override {}
+
+  void set_release_ref_callback(const base::Closure& release_ref_callback) {
+    release_ref_callback_ = release_ref_callback;
+  }
+
+  // TestViewsDelegate:
+  void ReleaseRef() override;
+
+ private:
+  base::Closure release_ref_callback_;
+  DISALLOW_COPY_AND_ASSIGN(DestructingTestViewsDelegate);
+};
+
+void DestructingTestViewsDelegate::ReleaseRef() {
+  if (!release_ref_callback_.is_null())
+    release_ref_callback_.Run();
+}
+
 }  // namespace
 
 class TestMenuItemViewShown : public MenuItemView {
  public:
   TestMenuItemViewShown(MenuDelegate* delegate) : MenuItemView(delegate) {
     submenu_ = new SubmenuViewShown(this);
   }
   ~TestMenuItemViewShown() override {}
 
   void SetController(MenuController* controller) {
     set_controller(controller);
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestMenuItemViewShown);
 };
 
 class MenuControllerTest : public ViewsTestBase {
  public:
   MenuControllerTest() : menu_controller_(nullptr) {
   }
 
   ~MenuControllerTest() override {}
 
   // ViewsTestBase:
   void SetUp() override {
+    std::unique_ptr<DestructingTestViewsDelegate> views_delegate(
+        new DestructingTestViewsDelegate());
+    test_views_delegate_ = views_delegate.get();
+    // ViewsTestBase takes ownership, destroying during Teardown.
+    set_views_delegate(std::move(views_delegate));
     ViewsTestBase::SetUp();
     Init();
     ASSERT_TRUE(base::MessageLoopForUI::IsCurrent());
   }
 
   void TearDown() override {
     owner_->CloseNow();
     DestroyMenuController();
     ViewsTestBase::TearDown();
   }
@@ skipping 119 matching lines @@
     menu_controller_->owner_ = owner_.get();
     menu_controller_->showing_ = true;
   }
 
   // Tests that the menu does not destroy itself when canceled during a drag.
   void TestCancelAllDuringDrag() {
     menu_controller_->CancelAll();
     EXPECT_EQ(0, menu_controller_delegate_->on_menu_closed_called());
   }
 
+  // Tests that destroying the menu during ViewsDelegate::ReleaseRef does not
+  // cause a crash.
+  void TestDestroyedDuringViewsRelease() {
+    // |test_views_delegate_| is owned by views::ViewsTestBase and not deleted
+    // until TearDown. MenuControllerTest outlives it.
+    test_views_delegate_->set_release_ref_callback(base::Bind(
+        &MenuControllerTest::DestroyMenuController, base::Unretained(this)));
+    menu_controller_->ExitAsyncRun();
+  }
+
  protected:
   void SetPendingStateItem(MenuItemView* item) {
     menu_controller_->pending_state_.item = item;
     menu_controller_->pending_state_.submenu_open = true;
   }
 
   void ResetSelection() {
     menu_controller_->SetSelection(
         nullptr,
         MenuController::SELECTION_EXIT |
@@ skipping 191 matching lines @@
     menu_controller_delegate_.reset(new TestMenuControllerDelegate);
     menu_controller_ =
         new MenuController(true, menu_controller_delegate_.get());
     menu_controller_->owner_ = owner_.get();
     menu_controller_->showing_ = true;
     menu_controller_->SetSelection(
         menu_item_.get(), MenuController::SELECTION_UPDATE_IMMEDIATELY);
     menu_item_->SetController(menu_controller_);
   }
 
+  // Not owned.
+  DestructingTestViewsDelegate* test_views_delegate_;
+
   std::unique_ptr<Widget> owner_;
   std::unique_ptr<ui::test::EventGenerator> event_generator_;
   std::unique_ptr<TestMenuItemViewShown> menu_item_;
   std::unique_ptr<TestMenuControllerDelegate> menu_controller_delegate_;
   std::unique_ptr<MenuDelegate> menu_delegate_;
   MenuController* menu_controller_;
   TestMenuMessageLoop* test_message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuControllerTest);
 };
@@ skipping 825 matching lines @@
   controller->SetAsyncRun(true);
 
   TestDragDropClient drag_drop_client(base::Bind(
       &MenuControllerTest::TestCancelAllDuringDrag, base::Unretained(this)));
   aura::client::SetDragDropClient(owner()->GetNativeWindow()->GetRootWindow(),
                                   &drag_drop_client);
   AddButtonMenuItems();
   StartDrag();
 }
 
+// Tests that when releasing the ref on ViewsDelegate and MenuController is
+// deleted, that shutdown occurs without crashing.
+TEST_F(MenuControllerTest, DestroyedDuringViewsRelease) {
+  ExitMenuRun();
+  MenuController* controller = menu_controller();
+  controller->SetAsyncRun(true);
+
+  int mouse_event_flags = 0;
+  MenuItemView* run_result =
+      controller->Run(owner(), nullptr, menu_item(), gfx::Rect(),
+                      MENU_ANCHOR_TOPLEFT, false, false, &mouse_event_flags);
+  EXPECT_EQ(run_result, nullptr);
+  TestDestroyedDuringViewsRelease();
+}
+
 #endif  // defined(USE_AURA)
 
 }  // namespace test
 }  // namespace views