summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-07-21 00:08:19 +0000
committersky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-07-21 00:08:19 +0000
commitb05a08b74150acf8fc021263355a7a3e0d389f6c (patch)
tree06c193911fed406da4ab282a8c2c9b3abf17d958
parent5701bb0620cf5f2c49767adcb2c1faa1b229285e (diff)
downloadchromium_src-b05a08b74150acf8fc021263355a7a3e0d389f6c.zip
chromium_src-b05a08b74150acf8fc021263355a7a3e0d389f6c.tar.gz
chromium_src-b05a08b74150acf8fc021263355a7a3e0d389f6c.tar.bz2
Fixes crash in menu code. The crash occurs because
RenderViewContextMenuView may try to delete the MenuItemView while it's running the nested message loop, which means many frames have deleted objects on them so that when the stack unwinds there's trouble. This is really just a temporary solution. The right fix is 57890, but as the branch point is close I'm going with this solution for now. BUG=89751 57890 TEST=none R=jcivelli@chromium.org Review URL: http://codereview.chromium.org/7465002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@93293 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc12
-rw-r--r--chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h4
-rw-r--r--views/controls/menu/menu_item_view.h4
-rw-r--r--views/controls/menu/menu_runner.cc92
-rw-r--r--views/controls/menu/menu_runner.h49
-rw-r--r--views/views.gyp2
6 files changed, 156 insertions, 7 deletions
diff --git a/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc b/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
index 577db98..9070005 100644
--- a/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
+++ b/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
@@ -14,6 +14,7 @@
#include "views/accelerator.h"
#include "views/controls/menu/menu_item_view.h"
#include "views/controls/menu/menu_model_adapter.h"
+#include "views/controls/menu/menu_runner.h"
////////////////////////////////////////////////////////////////////////////////
// RenderViewContextMenuViews, public:
@@ -21,14 +22,16 @@
RenderViewContextMenuViews::RenderViewContextMenuViews(
TabContents* tab_contents,
const ContextMenuParams& params)
- : RenderViewContextMenu(tab_contents, params) {
+ : RenderViewContextMenu(tab_contents, params),
+ menu_(NULL) {
}
RenderViewContextMenuViews::~RenderViewContextMenuViews() {
}
void RenderViewContextMenuViews::RunMenuAt(int x, int y) {
- menu_->RunMenuAt(source_tab_contents_->view()->GetTopLevelNativeWindow(),
+ menu_runner_->RunMenuAt(
+ source_tab_contents_->view()->GetTopLevelNativeWindow(),
NULL, gfx::Rect(gfx::Point(x, y), gfx::Size()),
views::MenuItemView::TOPLEFT, true);
}
@@ -40,7 +43,7 @@ void RenderViewContextMenuViews::SetExternal() {
#endif
void RenderViewContextMenuViews::UpdateMenuItemStates() {
- menu_delegate_->BuildMenu(menu_.get());
+ menu_delegate_->BuildMenu(menu_);
}
////////////////////////////////////////////////////////////////////////////////
@@ -48,7 +51,8 @@ void RenderViewContextMenuViews::UpdateMenuItemStates() {
void RenderViewContextMenuViews::PlatformInit() {
menu_delegate_.reset(new views::MenuModelAdapter(&menu_model_));
- menu_.reset(new views::MenuItemView(menu_delegate_.get()));
+ menu_ = new views::MenuItemView(menu_delegate_.get());
+ menu_runner_.reset(new views::MenuRunner(menu_));
UpdateMenuItemStates();
}
diff --git a/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h b/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h
index 2daa957..023a6c0 100644
--- a/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h
+++ b/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h
@@ -13,6 +13,7 @@
namespace views {
class MenuItemView;
class MenuModelAdapter;
+class MenuRunner;
} // namespace views
class RenderViewContextMenuViews : public RenderViewContextMenu {
@@ -40,7 +41,8 @@ class RenderViewContextMenuViews : public RenderViewContextMenu {
private:
// The context menu itself and its contents.
scoped_ptr<views::MenuModelAdapter> menu_delegate_;
- scoped_ptr<views::MenuItemView> menu_;
+ views::MenuItemView* menu_; // Owned by menu_runner_.
+ scoped_ptr<views::MenuRunner> menu_runner_;
DISALLOW_COPY_AND_ASSIGN(RenderViewContextMenuViews);
};
diff --git a/views/controls/menu/menu_item_view.h b/views/controls/menu/menu_item_view.h
index 3f952f7..ba2a365 100644
--- a/views/controls/menu/menu_item_view.h
+++ b/views/controls/menu/menu_item_view.h
@@ -114,8 +114,8 @@ class MenuItemView : public View {
virtual ~MenuItemView();
// Overridden from View:
- virtual bool GetTooltipText(const gfx::Point& p, std::wstring* tooltip)
- OVERRIDE;
+ virtual bool GetTooltipText(const gfx::Point& p,
+ std::wstring* tooltip) OVERRIDE;
virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
// Returns the preferred height of menu items. This is only valid when the
diff --git a/views/controls/menu/menu_runner.cc b/views/controls/menu/menu_runner.cc
new file mode 100644
index 0000000..78cb005
--- /dev/null
+++ b/views/controls/menu/menu_runner.cc
@@ -0,0 +1,92 @@
+// Copyright (c) 2011 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 "views/controls/menu/menu_runner.h"
+
+namespace views {
+
+// Manages the menu. To destroy a Holder invoke Release(). Release() deletes
+// immediately if the menu isn't showing. If the menu is showing Release()
+// cancels the menu and when the nested RunMenuAt() call returns, deletes itself
+// and the menu.
+class MenuRunner::Holder {
+ public:
+ explicit Holder(MenuItemView* menu);
+
+ // See description above class for details.
+ void Release();
+
+ // Runs the menu.
+ void RunMenuAt(gfx::NativeWindow parent,
+ MenuButton* button,
+ const gfx::Rect& bounds,
+ MenuItemView::AnchorPosition anchor,
+ bool has_mnemonics);
+
+ private:
+ ~Holder();
+
+ scoped_ptr<MenuItemView> menu_;
+
+ // Are we in run waiting for it to return?
+ bool running_;
+
+ // Set if |running_| and Release() has been invoked.
+ bool delete_after_run_;
+
+ DISALLOW_COPY_AND_ASSIGN(Holder);
+};
+
+MenuRunner::Holder::Holder(MenuItemView* menu)
+ : menu_(menu),
+ running_(false),
+ delete_after_run_(false) {
+}
+
+void MenuRunner::Holder::Release() {
+ if (running_) {
+ // The menu is running a nested message loop, we can't delete it now
+ // otherwise the stack would be in a really bad state (many frames would
+ // have deleted objects on them). Instead cancel the menu, when it returns
+ // Holder will delete itself.
+ delete_after_run_ = true;
+ menu_->Cancel();
+ } else {
+ delete this;
+ }
+}
+
+void MenuRunner::Holder::RunMenuAt(gfx::NativeWindow parent,
+ MenuButton* button,
+ const gfx::Rect& bounds,
+ MenuItemView::AnchorPosition anchor,
+ bool has_mnemonics) {
+ running_ = true;
+ menu_->RunMenuAt(parent, button, bounds, anchor, has_mnemonics);
+ if (delete_after_run_) {
+ delete this;
+ return;
+ }
+ running_ = false;
+}
+
+MenuRunner::Holder::~Holder() {
+}
+
+MenuRunner::MenuRunner(MenuItemView* menu) : holder_(new Holder(menu)) {
+}
+
+MenuRunner::~MenuRunner() {
+ holder_->Release();
+}
+
+void MenuRunner::RunMenuAt(gfx::NativeWindow parent,
+ MenuButton* button,
+ const gfx::Rect& bounds,
+ MenuItemView::AnchorPosition anchor,
+ bool has_mnemonics) {
+ holder_->RunMenuAt(parent, button, bounds, anchor, has_mnemonics);
+}
+
+} // namespace views
diff --git a/views/controls/menu/menu_runner.h b/views/controls/menu/menu_runner.h
new file mode 100644
index 0000000..4892b1e
--- /dev/null
+++ b/views/controls/menu/menu_runner.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2011 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.
+
+#ifndef VIEWS_CONTROLS_MENU_MENU_RUNNER_H_
+#define VIEWS_CONTROLS_MENU_MENU_RUNNER_H_
+#pragma once
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "views/controls/menu/menu_item_view.h"
+
+namespace views {
+
+class MenuButton;
+
+// MenuRunner handles the lifetime of the root MenuItemView. MenuItemView runs a
+// nested message loop, which means care must be taken when the MenuItemView
+// needs to be deleted. MenuRunner makes sure the menu is deleted after the
+// nested message loop completes.
+//
+// MenuRunner can be deleted at any time and will correctly handle deleting the
+// underlying menu.
+//
+// TODO: this is a work around for 57890. If we fix it this class shouldn't be
+// needed.
+class MenuRunner {
+ public:
+ explicit MenuRunner(MenuItemView* menu);
+ ~MenuRunner();
+
+ // Runs the menu.
+ void RunMenuAt(gfx::NativeWindow parent,
+ MenuButton* button,
+ const gfx::Rect& bounds,
+ MenuItemView::AnchorPosition anchor,
+ bool has_mnemonics);
+
+ private:
+ class Holder;
+
+ Holder* holder_;
+
+ DISALLOW_COPY_AND_ASSIGN(MenuRunner);
+};
+
+} // namespace views
+
+#endif // VIEWS_CONTROLS_MENU_MENU_RUNNER_H_
diff --git a/views/views.gyp b/views/views.gyp
index ab1ff5a..92859ae 100644
--- a/views/views.gyp
+++ b/views/views.gyp
@@ -134,6 +134,8 @@
'controls/menu/menu_item_view_win.cc',
'controls/menu/menu_model_adapter.cc',
'controls/menu/menu_model_adapter.h',
+ 'controls/menu/menu_runner.cc',
+ 'controls/menu/menu_runner.h',
'controls/menu/menu_scroll_view_container.cc',
'controls/menu/menu_scroll_view_container.h',
'controls/menu/menu_separator.h',