Merge 83444 - Fix crash in RenderViewContextMenuViews::RunMenuAt. As part of fixing

this I'm centralizing how we notify the view of menu showing/hiding.

BUG=80099 80879
TEST=none
R=estade@chromium.org,ben@chromium.org

Review URL: http://codereview.chromium.org/6901075

TBR=sky@chromium.org
Review URL: http://codereview.chromium.org/6889006

git-svn-id: svn://svn.chromium.org/chrome/branches/742/src@83780 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 58 insertions, 58 deletions

diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index dcf613e..8cc53f7 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -51,6 +51,7 @@
 #include "chrome/common/url_constants.h"
 #include "content/browser/child_process_security_policy.h"
 #include "content/browser/renderer_host/render_view_host.h"
+#include "content/browser/renderer_host/render_widget_host_view.h"
 #include "content/browser/tab_contents/navigation_entry.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "grit/generated_resources.h"
@@ -1498,9 +1499,20 @@ void RenderViewContextMenu::ExecuteCommand(int id) {
   }
 }
 
+void RenderViewContextMenu::MenuWillShow() {
+  RenderWidgetHostView* view = source_tab_contents_->GetRenderWidgetHostView();
+  if (view)
+    view->ShowingContextMenu(true);
+}
+
 void RenderViewContextMenu::MenuClosed() {
-  source_tab_contents_->render_view_host()->ContextMenuClosed(
-      params_.custom_context);
+  RenderWidgetHostView* view = source_tab_contents_->GetRenderWidgetHostView();
+  if (view)
+    view->ShowingContextMenu(false);
+  if (source_tab_contents_->render_view_host()) {
+    source_tab_contents_->render_view_host()->ContextMenuClosed(
+        params_.custom_context);
+  }
 }
 
 bool RenderViewContextMenu::IsDevCommandEnabled(int id) const {
diff --git a/chrome/browser/tab_contents/render_view_context_menu.h b/chrome/browser/tab_contents/render_view_context_menu.h
index 91f35f9..947fb65 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.h
+++ b/chrome/browser/tab_contents/render_view_context_menu.h
@@ -44,10 +44,11 @@ class RenderViewContextMenu : public ui::SimpleMenuModel::Delegate {
   void Init();
 
   // SimpleMenuModel::Delegate implementation.
-  virtual bool IsCommandIdChecked(int command_id) const;
-  virtual bool IsCommandIdEnabled(int command_id) const;
-  virtual void ExecuteCommand(int command_id);
-  virtual void MenuClosed();
+  virtual bool IsCommandIdChecked(int command_id) const OVERRIDE;
+  virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE;
+  virtual void ExecuteCommand(int command_id) OVERRIDE;
+  virtual void MenuWillShow() OVERRIDE;
+  virtual void MenuClosed() OVERRIDE;
 
  protected:
   void InitMenu();
diff --git a/chrome/browser/tab_contents/render_view_context_menu_gtk.cc b/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
index a24109e..f2e9f94 100644
--- a/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -41,18 +41,9 @@ bool RenderViewContextMenuGtk::GetAcceleratorForCommandId(
 }
 
 void RenderViewContextMenuGtk::Popup(const gfx::Point& point) {
-  RenderWidgetHostView* rwhv = source_tab_contents_->GetRenderWidgetHostView();
-  if (rwhv)
-    rwhv->ShowingContextMenu(true);
   menu_gtk_->PopupAsContext(point, triggering_event_time_);
 }
 
-void RenderViewContextMenuGtk::StoppedShowing() {
-  RenderWidgetHostView* rwhv = source_tab_contents_->GetRenderWidgetHostView();
-  if (rwhv)
-    rwhv->ShowingContextMenu(false);
-}
-
 bool RenderViewContextMenuGtk::AlwaysShowIconForCmd(int command_id) const {
   return command_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
       command_id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST;
diff --git a/chrome/browser/tab_contents/render_view_context_menu_gtk.h b/chrome/browser/tab_contents/render_view_context_menu_gtk.h
index 1c89ce5..031be00 100644
--- a/chrome/browser/tab_contents/render_view_context_menu_gtk.h
+++ b/chrome/browser/tab_contents/render_view_context_menu_gtk.h
@@ -26,7 +26,6 @@ class RenderViewContextMenuGtk : public RenderViewContextMenu,
   void Popup(const gfx::Point& point);
 
   // Menu::Delegate implementation ---------------------------------------------
-  virtual void StoppedShowing();
   virtual bool AlwaysShowIconForCmd(int command_id) const;
 
  protected:
diff --git a/chrome/browser/ui/gtk/menu_gtk.cc b/chrome/browser/ui/gtk/menu_gtk.cc
index 19e4d12..d120579 100644
--- a/chrome/browser/ui/gtk/menu_gtk.cc
+++ b/chrome/browser/ui/gtk/menu_gtk.cc
@@ -718,6 +718,7 @@ void MenuGtk::ExecuteCommand(ui::MenuModel* model, int id) {
 }
 
 void MenuGtk::OnMenuShow(GtkWidget* widget) {
+  model_->MenuWillShow();
   MessageLoop::current()->PostTask(FROM_HERE,
       factory_.NewRunnableMethod(&MenuGtk::UpdateMenu));
 }
diff --git a/chrome/browser/ui/gtk/menu_gtk.h b/chrome/browser/ui/gtk/menu_gtk.h
index 381993b..ecfd364 100644
--- a/chrome/browser/ui/gtk/menu_gtk.h
+++ b/chrome/browser/ui/gtk/menu_gtk.h
@@ -27,7 +27,7 @@ class MenuGtk {
   // Delegate class that lets another class control the status of the menu.
   class Delegate {
    public:
-    virtual ~Delegate() { }
+    virtual ~Delegate() {}
 
     // Called before a command is executed. This exists for the case where a
     // model is handling the actual execution of commands, but the delegate
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 92939f3..e03d7eb0 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
@@ -19,30 +19,14 @@
 RenderViewContextMenuViews::RenderViewContextMenuViews(
     TabContents* tab_contents,
     const ContextMenuParams& params)
-    : RenderViewContextMenu(tab_contents, params),
-      destroyed_flag_(NULL) {
+    : RenderViewContextMenu(tab_contents, params) {
 }
 
 RenderViewContextMenuViews::~RenderViewContextMenuViews() {
-  if (destroyed_flag_)
-    *destroyed_flag_ = true;
 }
 
 void RenderViewContextMenuViews::RunMenuAt(int x, int y) {
-  RenderWidgetHostView* rwhv = source_tab_contents_->GetRenderWidgetHostView();
-  if (rwhv)
-    rwhv->ShowingContextMenu(true);
-  bool destroyed = false;
-  // TODO(sky): tracking destruction is tedious and error prone. We should make
-  // the menus not run a nested message loop so that folks don't have to worry
-  // about being destroyed while the menu is up.
-  destroyed_flag_ = &destroyed;
   menu_->RunContextMenuAt(gfx::Point(x, y));
-  if (destroyed)
-    return;
-  destroyed_flag_ = NULL;
-  if (rwhv)
-    rwhv->ShowingContextMenu(false);
 }
 
 #if defined(OS_WIN)
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 d648a26..5b74407 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
@@ -41,11 +41,6 @@ class RenderViewContextMenuViews : public RenderViewContextMenu {
   // The context menu itself and its contents.
   scoped_ptr<views::Menu2> menu_;
 
-  // If non-null the destructor sets this to true. This is set to non-null while
-  // the menu is showing. It is used to detect if the menu was deleted while
-  // running.
-  bool* destroyed_flag_;
-
   DISALLOW_COPY_AND_ASSIGN(RenderViewContextMenuViews);
 };
 
diff --git a/ui/base/models/simple_menu_model.cc b/ui/base/models/simple_menu_model.cc
index 28c1b01..57f6ec3e 100644
--- a/ui/base/models/simple_menu_model.cc
+++ b/ui/base/models/simple_menu_model.cc
@@ -46,6 +46,9 @@ bool SimpleMenuModel::Delegate::GetIconForCommandId(
 void SimpleMenuModel::Delegate::CommandIdHighlighted(int command_id) {
 }
 
+void SimpleMenuModel::Delegate::MenuWillShow() {
+}
+
 void SimpleMenuModel::Delegate::MenuClosed() {
 }
 
@@ -291,6 +294,11 @@ MenuModel* SimpleMenuModel::GetSubmenuModelAt(int index) const {
   return items_.at(FlipIndex(index)).submenu;
 }
 
+void SimpleMenuModel::MenuWillShow() {
+  if (delegate_)
+    delegate_->MenuWillShow();
+}
+
 void SimpleMenuModel::MenuClosed() {
   // Due to how menus work on the different platforms, ActivatedAt will be
   // called after this.  It's more convenient for the delegate to be called
diff --git a/ui/base/models/simple_menu_model.h b/ui/base/models/simple_menu_model.h
index 59b752f..e8ccb40 100644
--- a/ui/base/models/simple_menu_model.h
+++ b/ui/base/models/simple_menu_model.h
@@ -49,6 +49,9 @@ class SimpleMenuModel : public MenuModel {
     // Performs the action associated with the specified command id.
     virtual void ExecuteCommand(int command_id) = 0;
 
+    // Notifies the delegate that the menu is about to show.
+    virtual void MenuWillShow();
+
     // Notifies the delegate that the menu has closed.
     virtual void MenuClosed();
 
@@ -102,25 +105,28 @@ class SimpleMenuModel : public MenuModel {
   int GetIndexOfCommandId(int command_id);
 
   // Overridden from MenuModel:
-  virtual bool HasIcons() const;
-  virtual int GetItemCount() const;
-  virtual ItemType GetTypeAt(int index) const;
-  virtual int GetCommandIdAt(int index) const;
-  virtual string16 GetLabelAt(int index) const;
-  virtual bool IsItemDynamicAt(int index) const;
+  virtual bool HasIcons() const OVERRIDE;
+  virtual int GetItemCount() const OVERRIDE;
+  virtual ItemType GetTypeAt(int index) const OVERRIDE;
+  virtual int GetCommandIdAt(int index) const OVERRIDE;
+  virtual string16 GetLabelAt(int index) const OVERRIDE;
+  virtual bool IsItemDynamicAt(int index) const OVERRIDE;
   virtual bool GetAcceleratorAt(int index,
-                                ui::Accelerator* accelerator) const;
-  virtual bool IsItemCheckedAt(int index) const;
-  virtual int GetGroupIdAt(int index) const;
-  virtual bool GetIconAt(int index, SkBitmap* icon);
-  virtual ui::ButtonMenuItemModel* GetButtonMenuItemAt(int index) const;
-  virtual bool IsEnabledAt(int index) const;
-  virtual bool IsVisibleAt(int index) const;
-  virtual void HighlightChangedTo(int index);
-  virtual void ActivatedAt(int index);
-  virtual MenuModel* GetSubmenuModelAt(int index) const;
-  virtual void MenuClosed();
-  virtual void SetMenuModelDelegate(ui::MenuModelDelegate* menu_model_delegate);
+                                ui::Accelerator* accelerator) const OVERRIDE;
+  virtual bool IsItemCheckedAt(int index) const OVERRIDE;
+  virtual int GetGroupIdAt(int index) const OVERRIDE;
+  virtual bool GetIconAt(int index, SkBitmap* icon) OVERRIDE;
+  virtual ui::ButtonMenuItemModel* GetButtonMenuItemAt(
+      int index) const OVERRIDE;
+  virtual bool IsEnabledAt(int index) const OVERRIDE;
+  virtual bool IsVisibleAt(int index) const OVERRIDE;
+  virtual void HighlightChangedTo(int index) OVERRIDE;
+  virtual void ActivatedAt(int index) OVERRIDE;
+  virtual MenuModel* GetSubmenuModelAt(int index) const OVERRIDE;
+  virtual void MenuWillShow() OVERRIDE;
+  virtual void MenuClosed() OVERRIDE;
+  virtual void SetMenuModelDelegate(
+      ui::MenuModelDelegate* menu_model_delegate) OVERRIDE;
 
  protected:
   // Some variants of this model (SystemMenuModel) relies on items to be
diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index fbb1440..635909b 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -136,6 +136,8 @@ void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
       g_signal_connect_after(menu_, "move-current",
                              G_CALLBACK(AfterMenuMoveCurrentThunk), this);
 
+  model_->MenuWillShow();
+
   // Block until menu is no longer shown by running a nested message loop.
   nested_dispatcher_ = new NestedDispatcherGtk(this, true);
   bool deleted = nested_dispatcher_->RunAndSelfDestruct();
diff --git a/views/controls/menu/native_menu_win.cc b/views/controls/menu/native_menu_win.cc
index 97a063b..7520306 100644
--- a/views/controls/menu/native_menu_win.cc
+++ b/views/controls/menu/native_menu_win.cc
@@ -380,6 +380,7 @@ void NativeMenuWin::RunMenuAt(const gfx::Point& point, int alignment) {
   menu_to_select_factory_.RevokeAll();
   bool destroyed = false;
   destroyed_flag_ = &destroyed;
+  model_->MenuWillShow();
   TrackPopupMenu(menu_, flags, point.x(), point.y(), 0, host_window_->hwnd(),
                  NULL);
   UnhookWindowsHookEx(hhook);