Commit bd599611 authored by Peter Boström's avatar Peter Boström Committed by Commit Bot

Add weak_ptr check to ContextMenuController

Prevents use-after-free if ContextMenuController gets destroyed while
the context menu is running (nested runloop or as a result of using the
context menu).

Bug: chromium:1003313
Change-Id: Ib289adf97cd1b5d27507ee4d4c581c1bb3f51bdf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815828
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699860}
parent a7fc60bd
...@@ -8,6 +8,10 @@ ...@@ -8,6 +8,10 @@
namespace views { namespace views {
ContextMenuController::ContextMenuController() = default;
ContextMenuController::~ContextMenuController() = default;
void ContextMenuController::ShowContextMenuForView( void ContextMenuController::ShowContextMenuForView(
View* source, View* source,
const gfx::Point& point, const gfx::Point& point,
...@@ -15,9 +19,18 @@ void ContextMenuController::ShowContextMenuForView( ...@@ -15,9 +19,18 @@ void ContextMenuController::ShowContextMenuForView(
// Use a boolean flag to early-exit out of re-entrant behavior. // Use a boolean flag to early-exit out of re-entrant behavior.
if (is_opening_) if (is_opening_)
return; return;
base::AutoReset<bool> auto_reset_is_opening(&is_opening_, true);
// We might get deleted while showing the context menu (including as a result
// of showing it). If so, we need to make sure we're not accessing
// |is_opening_|.
auto weak_ptr = weak_factory_.GetWeakPtr();
ShowContextMenuForViewImpl(source, point, source_type); ShowContextMenuForViewImpl(source, point, source_type);
if (!weak_ptr)
return;
is_opening_ = false;
} }
} // namespace views } // namespace views
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef UI_VIEWS_CONTEXT_MENU_CONTROLLER_H_ #ifndef UI_VIEWS_CONTEXT_MENU_CONTROLLER_H_
#define UI_VIEWS_CONTEXT_MENU_CONTROLLER_H_ #define UI_VIEWS_CONTEXT_MENU_CONTROLLER_H_
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "ui/base/ui_base_types.h" #include "ui/base/ui_base_types.h"
#include "ui/views/views_export.h" #include "ui/views/views_export.h"
...@@ -28,6 +30,8 @@ class View; ...@@ -28,6 +30,8 @@ class View;
// implementation for mouse processing. // implementation for mouse processing.
class VIEWS_EXPORT ContextMenuController { class VIEWS_EXPORT ContextMenuController {
public: public:
ContextMenuController();
// Invoked to show the context menu for |source|. |point| is in screen // Invoked to show the context menu for |source|. |point| is in screen
// coordinates. This method also prevents reentrant calls. // coordinates. This method also prevents reentrant calls.
void ShowContextMenuForView(View* source, void ShowContextMenuForView(View* source,
...@@ -35,7 +39,7 @@ class VIEWS_EXPORT ContextMenuController { ...@@ -35,7 +39,7 @@ class VIEWS_EXPORT ContextMenuController {
ui::MenuSourceType source_type); ui::MenuSourceType source_type);
protected: protected:
virtual ~ContextMenuController() = default; virtual ~ContextMenuController();
private: private:
// Subclasses should override this method. // Subclasses should override this method.
...@@ -48,6 +52,10 @@ class VIEWS_EXPORT ContextMenuController { ...@@ -48,6 +52,10 @@ class VIEWS_EXPORT ContextMenuController {
// spins a nested message loop that processes input events, which may attempt // spins a nested message loop that processes input events, which may attempt
// to trigger another context menu. // to trigger another context menu.
bool is_opening_ = false; bool is_opening_ = false;
base::WeakPtrFactory<ContextMenuController> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(ContextMenuController);
}; };
} // namespace views } // namespace views
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment