Fixes possible crash if the window hosting a menu was closed while the

menu was showing. When this happens the window the menu creates is
implicitly destroyed (because the parent is going away).

BUG=25563
TEST=see bug

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44807 0039d316-1c4b-4281-b951-d872f2087c98

17 files changed, 354 insertions, 202 deletions

diff --git a/views/controls/button/menu_button.cc b/views/controls/button/menu_button.cc
index 4a1e28a..b45c2ac 100644
--- a/views/controls/button/menu_button.cc
+++ b/views/controls/button/menu_button.cc
@@ -49,11 +49,14 @@ MenuButton::MenuButton(ButtonListener* listener,
       menu_delegate_(menu_delegate),
       show_menu_marker_(show_menu_marker),
       menu_marker_(ResourceBundle::GetSharedInstance().GetBitmapNamed(
-          IDR_MENU_DROPARROW)) {
+                       IDR_MENU_DROPARROW)),
+      destroyed_flag_(NULL) {
   set_alignment(TextButton::ALIGN_LEFT);
 }
 
 MenuButton::~MenuButton() {
+  if (destroyed_flag_)
+    *destroyed_flag_ = true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -145,7 +148,17 @@ bool MenuButton::Activate() {
     GetRootView()->SetMouseHandler(NULL);
 
     menu_visible_ = true;
+
+    bool destroyed = false;
+    destroyed_flag_ = &destroyed;
+
     menu_delegate_->RunMenu(this, menu_position);
+
+    if (destroyed) {
+      // The menu was deleted while showing. Don't attempt any processing.
+      return false;
+    }
+
     menu_visible_ = false;
     menu_closed_time_ = Time::Now();
 
diff --git a/views/controls/button/menu_button.h b/views/controls/button/menu_button.h
index 3895b03..c4e95e4 100644
--- a/views/controls/button/menu_button.h
+++ b/views/controls/button/menu_button.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -97,6 +97,10 @@ class MenuButton : public TextButton {
   // text buttons.
   const SkBitmap* menu_marker_;
 
+  // If non-null the destuctor sets this to true. This is set while the menu is
+  // showing and used to detect if the menu was deleted while running.
+  bool* destroyed_flag_;
+
   DISALLOW_COPY_AND_ASSIGN(MenuButton);
 };
 
diff --git a/views/controls/menu/menu_controller.cc b/views/controls/menu/menu_controller.cc
index b162c72..dfbb5f1 100644
--- a/views/controls/menu/menu_controller.cc
+++ b/views/controls/menu/menu_controller.cc
@@ -16,8 +16,10 @@
 #include "views/drag_utils.h"
 #include "views/screen.h"
 #include "views/view_constants.h"
+#include "views/views_delegate.h"
 #include "views/widget/root_view.h"
 #include "views/widget/widget.h"
+
 #if defined(OS_LINUX)
 #include "base/keyboard_code_conversion_gtk.h"
 #endif
@@ -191,6 +193,10 @@ MenuItemView* MenuController::Run(gfx::NativeWindow parent,
   DLOG(INFO) << " entering nested loop, depth=" << nested_depth;
 #endif
 
+  // Make sure Chrome doesn't attempt to shut down while the menu is showing.
+  if (ViewsDelegate::views_delegate)
+    ViewsDelegate::views_delegate->AddRef();
+
   MessageLoopForUI* loop = MessageLoopForUI::current();
   if (MenuItemView::allow_task_nesting_during_run_) {
     bool did_allow_task_nesting = loop->NestableTasksAllowed();
@@ -201,6 +207,9 @@ MenuItemView* MenuController::Run(gfx::NativeWindow parent,
     loop->Run(this);
   }
 
+  if (ViewsDelegate::views_delegate)
+    ViewsDelegate::views_delegate->ReleaseRef();
+
 #ifdef DEBUG_MENU
   nested_depth--;
   DLOG(INFO) << " exiting nested loop,  depth=" << nested_depth;
@@ -238,11 +247,14 @@ MenuItemView* MenuController::Run(gfx::NativeWindow parent,
       CloseAllNestedMenus();
 
       // Set exit_all_, which makes sure all nested loops exit immediately.
-      exit_type_ = EXIT_ALL;
+      if (exit_type_ != EXIT_DESTROYED)
+        exit_type_ = EXIT_ALL;
     }
   }
 
-  if (menu_button_) {
+  // If we stopped running because one of the menus was destroyed chances are
+  // the button was also destroyed.
+  if (exit_type_ != EXIT_DESTROYED && menu_button_) {
     menu_button_->SetState(CustomButton::BS_NORMAL);
     menu_button_->SchedulePaint();
   }
@@ -286,7 +298,7 @@ void MenuController::SetSelection(MenuItemView* menu_item,
     StartShowTimer();
 }
 
-void MenuController::Cancel(bool all) {
+void MenuController::Cancel(ExitType type) {
   if (!showing_) {
     // This occurs if we're in the process of notifying the delegate for a drop
     // and the delegate cancels us.
@@ -294,7 +306,7 @@ void MenuController::Cancel(bool all) {
   }
 
   MenuItemView* selected = state_.item;
-  exit_type_ = all ? EXIT_ALL : EXIT_OUTERMOST;
+  exit_type_ = type;
 
   // Hide windows immediately.
   SetSelection(NULL, false, true);
@@ -336,7 +348,7 @@ void MenuController::OnMousePressed(SubmenuView* source,
 #endif
 
     // And close.
-    Cancel(true);
+    Cancel(EXIT_ALL);
     return;
   }
 
@@ -394,7 +406,7 @@ void MenuController::OnMouseDragged(SubmenuView* source,
         if (showing_) {
           // We're still showing, close all menus.
           CloseAllNestedMenus();
-          Cancel(true);
+          Cancel(EXIT_ALL);
         }  // else case, drop was on us.
       }  // else case, someone canceled us, don't do anything
     }
@@ -637,7 +649,7 @@ void MenuController::SetActiveInstance(MenuController* controller) {
 bool MenuController::Dispatch(const MSG& msg) {
   DCHECK(blocking_run_);
 
-  if (exit_type_ == EXIT_ALL) {
+  if (exit_type_ == EXIT_ALL || exit_type_ == EXIT_DESTROYED) {
     // We must translate/dispatch the message here, otherwise we would drop
     // the message on the floor.
     TranslateMessage(&msg);
@@ -678,7 +690,7 @@ bool MenuController::Dispatch(const MSG& msg) {
     case WM_CANCELMODE:
     case WM_SYSKEYDOWN:
       // Exit immediately on system keys.
-      Cancel(true);
+      Cancel(EXIT_ALL);
       return false;
 
     default:
@@ -693,7 +705,7 @@ bool MenuController::Dispatch(const MSG& msg) {
 bool MenuController::Dispatch(GdkEvent* event) {
   gtk_main_do_event(event);
 
-  if (exit_type_ == EXIT_ALL)
+  if (exit_type_ == EXIT_ALL || exit_type_ == EXIT_DESTROYED)
     return false;
 
   switch (event->type) {
@@ -766,7 +778,7 @@ bool MenuController::OnKeyDown(int key_code
            (!state_.item->HasSubmenu() ||
             !state_.item->GetSubmenu()->IsShowing()))) {
         // User pressed escape and only one menu is shown, cancel it.
-        Cancel(false);
+        Cancel(EXIT_OUTERMOST);
         return false;
       } else {
         CloseSubmenu();
diff --git a/views/controls/menu/menu_controller.h b/views/controls/menu/menu_controller.h
index f8a459a..d6518be 100644
--- a/views/controls/menu/menu_controller.h
+++ b/views/controls/menu/menu_controller.h
@@ -38,6 +38,22 @@ class MenuController : public MessageLoopForUI::Dispatcher {
   friend class MenuHostRootView;
   friend class MenuItemView;
 
+  // Enumeration of how the menu should exit.
+  enum ExitType {
+    // Don't exit.
+    EXIT_NONE,
+
+    // All menus, including nested, should be exited.
+    EXIT_ALL,
+
+    // Only the outermost menu should be exited.
+    EXIT_OUTERMOST,
+
+    // This is set if the menu is being closed as the result of one of the menus
+    // being destroyed.
+    EXIT_DESTROYED
+  };
+
   // If a menu is currently active, this returns the controller for it.
   static MenuController* GetActiveInstance();
 
@@ -66,12 +82,12 @@ class MenuController : public MessageLoopForUI::Dispatcher {
                     bool open_submenu,
                     bool update_immediately);
 
-  // Cancels the current Run. If all is true, any nested loops are canceled
-  // as well. This immediately hides all menus.
-  void Cancel(bool all);
+  // Cancels the current Run. See ExitType for a description of what happens
+  // with the various parameters.
+  void Cancel(ExitType type);
 
-  // An alternative to Cancel(true) that can be used with a OneShotTimer.
-  void CancelAll() { return Cancel(true); }
+  // An alternative to Cancel(EXIT_ALL) that can be used with a OneShotTimer.
+  void CancelAll() { Cancel(EXIT_ALL); }
 
   // Various events, forwarded from the submenu.
   //
@@ -98,18 +114,6 @@ class MenuController : public MessageLoopForUI::Dispatcher {
   void OnDragExitedScrollButton(SubmenuView* source);
 
  private:
-  // Enumeration of how the menu should exit.
-  enum ExitType {
-    // Don't exit.
-    EXIT_NONE,
-
-    // All menus, including nested, should be exited.
-    EXIT_ALL,
-
-    // Only the outermost menu should be exited.
-    EXIT_OUTERMOST
-  };
-
   class MenuScrollTask;
 
   // Tracks selection information.
diff --git a/views/controls/menu/menu_host.h b/views/controls/menu/menu_host.h
new file mode 100755
index 0000000..676a93b
--- /dev/null
+++ b/views/controls/menu/menu_host.h
@@ -0,0 +1,64 @@
+// Copyright (c) 2010 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_HOST_H_
+#define VIEWS_CONTROLS_MENU_MENU_HOST_H_
+
+#include "gfx/native_widget_types.h"
+#include "gfx/rect.h"
+
+namespace views {
+
+class SubmenuView;
+class View;
+
+// SubmenuView uses a MenuHost to house the SubmenuView. MenuHost typically
+// extends the native Widget type, but is defined here for clarity of what
+// methods SubmenuView uses.
+//
+// SubmenuView owns the MenuHost. When SubmenuView is done with the MenuHost
+// |DestroyMenuHost| is invoked. The one exception to this is if the native
+// OS destroys the widget out from under us, in which case |MenuHostDestroyed|
+// is invoked back on the SubmenuView and the SubmenuView then drops references
+// to the MenuHost.
+class MenuHost {
+ public:
+  // Creates the platform specific MenuHost. Ownership passes to the caller.
+  static MenuHost* Create(SubmenuView* submenu_view);
+
+  // Initializes and shows the MenuHost.
+  virtual void Init(gfx::NativeWindow parent,
+                    const gfx::Rect& bounds,
+                    View* contents_view,
+                    bool do_capture) = 0;
+
+  // Returns true if the menu host is visible.
+  virtual bool IsMenuHostVisible() = 0;
+
+  // Shows the menu host. If |do_capture| is true the menu host should do a
+  // mouse grab.
+  virtual void ShowMenuHost(bool do_capture) = 0;
+
+  // Hides the menu host.
+  virtual void HideMenuHost() = 0;
+
+  // Destroys and deletes the menu host.
+  virtual void DestroyMenuHost() = 0;
+
+  // Sets the bounds of the menu host.
+  virtual void SetMenuHostBounds(const gfx::Rect& bounds) = 0;
+
+  // Releases a mouse grab installed by |ShowMenuHost|.
+  virtual void ReleaseMenuHostCapture() = 0;
+
+  // Returns the native window of the MenuHost.
+  virtual gfx::NativeWindow GetMenuHostWindow() = 0;
+
+ protected:
+  virtual ~MenuHost() {}
+};
+
+}  // namespace views
+
+#endif  // VIEWS_CONTROLS_MENU_MENU_HOST_H_
diff --git a/views/controls/menu/menu_host_gtk.cc b/views/controls/menu/menu_host_gtk.cc
index bdd8ffe..95fea7b 100644
--- a/views/controls/menu/menu_host_gtk.cc
+++ b/views/controls/menu/menu_host_gtk.cc
@@ -1,8 +1,7 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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_host_gtk.h"
 
 #include <gdk/gdk.h>
@@ -14,9 +13,14 @@
 
 namespace views {
 
-MenuHost::MenuHost(SubmenuView* submenu)
+// static
+MenuHost* MenuHost::Create(SubmenuView* submenu_view) {
+  return new MenuHostGtk(submenu_view);
+}
+
+MenuHostGtk::MenuHostGtk(SubmenuView* submenu)
     : WidgetGtk(WidgetGtk::TYPE_POPUP),
-      closed_(false),
+      destroying_(false),
       submenu_(submenu),
       did_pointer_grab_(false) {
   GdkEvent* event = gtk_get_current_event();
@@ -29,50 +33,92 @@ MenuHost::MenuHost(SubmenuView* submenu)
   }
 }
 
-void MenuHost::Init(gfx::NativeWindow parent,
+MenuHostGtk::~MenuHostGtk() {
+}
+
+void MenuHostGtk::Init(gfx::NativeWindow parent,
                     const gfx::Rect& bounds,
                     View* contents_view,
                     bool do_capture) {
+  make_transient_to_parent();
   WidgetGtk::Init(GTK_WIDGET(parent), bounds);
+  // Make sure we get destroyed when the parent is destroyed.
+  gtk_window_set_destroy_with_parent(GTK_WINDOW(GetNativeView()), TRUE);
   gtk_window_set_type_hint(GTK_WINDOW(GetNativeView()),
                            GDK_WINDOW_TYPE_HINT_MENU);
   SetContentsView(contents_view);
-  Show();
+  ShowMenuHost(do_capture);
+}
+
+bool MenuHostGtk::IsMenuHostVisible() {
+  return IsVisible();
+}
+
+void MenuHostGtk::ShowMenuHost(bool do_capture) {
+  WidgetGtk::Show();
   if (do_capture)
     DoCapture();
 }
 
-gfx::NativeWindow MenuHost::GetNativeWindow() {
+void MenuHostGtk::HideMenuHost() {
+  // Make sure we release capture before hiding.
+  ReleaseMenuHostCapture();
+
+  WidgetGtk::Hide();
+}
+
+void MenuHostGtk::DestroyMenuHost() {
+  HideMenuHost();
+  destroying_ = true;
+  CloseNow();
+}
+
+void MenuHostGtk::SetMenuHostBounds(const gfx::Rect& bounds) {
+  SetBounds(bounds);
+}
+
+void MenuHostGtk::ReleaseMenuHostCapture() {
+  ReleaseGrab();
+}
+
+gfx::NativeWindow MenuHostGtk::GetMenuHostWindow() {
   return GTK_WINDOW(GetNativeView());
 }
 
-void MenuHost::Show() {
-  WidgetGtk::Show();
+RootView* MenuHostGtk::CreateRootView() {
+  return new MenuHostRootView(this, submenu_);
+}
+
+bool MenuHostGtk::ReleaseCaptureOnMouseReleased() {
+  return false;
 }
 
-void MenuHost::Hide() {
-  if (closed_) {
-    // We're already closed, nothing to do.
-    // This is invoked twice if the first time just hid us, and the second
-    // time deleted Closed (deleted) us.
-    return;
+void MenuHostGtk::ReleaseGrab() {
+  WidgetGtk::ReleaseGrab();
+  if (did_pointer_grab_) {
+    did_pointer_grab_ = false;
+    gdk_pointer_ungrab(GDK_CURRENT_TIME);
   }
-  // The menus are freed separately, and possibly before the window is closed,
-  // remove them so that View doesn't try to access deleted objects.
-  static_cast<MenuHostRootView*>(GetRootView())->suspend_events();
-  GetRootView()->RemoveAllChildViews(false);
-  ReleaseGrab();
-  closed_ = true;
-  WidgetGtk::Hide();
 }
 
-void MenuHost::HideWindow() {
-  // Make sure we release capture before hiding.
-  ReleaseGrab();
-  WidgetGtk::Hide();
+void MenuHostGtk::OnDestroy(GtkWidget* object) {
+  if (!destroying_) {
+    // We weren't explicitly told to destroy ourselves, which means the menu was
+    // deleted out from under us (the window we're parented to was closed). Tell
+    // the SubmenuView to drop references to us.
+    submenu_->MenuHostDestroyed();
+  }
+  WidgetGtk::OnDestroy(object);
+}
+
+gboolean MenuHostGtk::OnGrabBrokeEvent(GtkWidget* widget, GdkEvent* event) {
+  // Grab breaking only happens when drag and drop starts. So, we don't try
+  // and ungrab or cancel the menu.
+  did_pointer_grab_ = false;
+  return WidgetGtk::OnGrabBrokeEvent(widget, event);
 }
 
-void MenuHost::DoCapture() {
+void MenuHostGtk::DoCapture() {
   // Release the current grab.
   GtkWidget* current_grab_window = gtk_grab_get_current();
   if (current_grab_window)
@@ -93,38 +139,6 @@ void MenuHost::DoCapture() {
   did_pointer_grab_ = (grab_status == GDK_GRAB_SUCCESS);
   DCHECK(did_pointer_grab_);
   // need keyboard grab.
-#ifdef DEBUG_MENU
-  DLOG(INFO) << "Doing capture";
-#endif
-}
-
-void MenuHost::ReleaseCapture() {
-  ReleaseGrab();
-}
-
-RootView* MenuHost::CreateRootView() {
-  return new MenuHostRootView(this, submenu_);
-}
-
-gboolean MenuHost::OnGrabBrokeEvent(GtkWidget* widget, GdkEvent* event) {
-  // Grab breaking only happens when drag and drop starts. So, we don't try
-  // and ungrab or cancel the menu.
-  did_pointer_grab_ = false;
-  return WidgetGtk::OnGrabBrokeEvent(widget, event);
-}
-
-// Overriden to return false, we do NOT want to release capture on mouse
-// release.
-bool MenuHost::ReleaseCaptureOnMouseReleased() {
-  return false;
-}
-
-void MenuHost::ReleaseGrab() {
-  WidgetGtk::ReleaseGrab();
-  if (did_pointer_grab_) {
-    did_pointer_grab_ = false;
-    gdk_pointer_ungrab(GDK_CURRENT_TIME);
-  }
 }
 
 }  // namespace views
diff --git a/views/controls/menu/menu_host_gtk.h b/views/controls/menu/menu_host_gtk.h
index 4ae1b77..97223f8 100644
--- a/views/controls/menu/menu_host_gtk.h
+++ b/views/controls/menu/menu_host_gtk.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -6,6 +6,7 @@
 #ifndef VIEWS_CONTROLS_MENU_MENU_HOST_GTK_H_
 #define VIEWS_CONTROLS_MENU_MENU_HOST_GTK_H_
 
+#include "views/controls/menu/menu_host.h"
 #include "views/widget/widget_gtk.h"
 
 namespace views {
@@ -13,28 +14,27 @@ namespace views {
 class SubmenuView;
 
 // MenuHost implementation for Gtk.
-class MenuHost : public WidgetGtk {
+class MenuHostGtk : public WidgetGtk, public MenuHost {
  public:
-  explicit MenuHost(SubmenuView* submenu);
+  explicit MenuHostGtk(SubmenuView* submenu);
+  virtual ~MenuHostGtk();
 
+  // MenuHost overrides.
   void Init(gfx::NativeWindow parent,
             const gfx::Rect& bounds,
             View* contents_view,
             bool do_capture);
-
-  gfx::NativeWindow GetNativeWindow();
-
-  void Show();
-  virtual void Hide();
-  virtual void HideWindow();
-  void DoCapture();
-  void ReleaseCapture();
+  virtual bool IsMenuHostVisible();
+  virtual void ShowMenuHost(bool do_capture);
+  virtual void HideMenuHost();
+  virtual void DestroyMenuHost();
+  virtual void SetMenuHostBounds(const gfx::Rect& bounds);
+  virtual void ReleaseMenuHostCapture();
+  virtual gfx::NativeWindow GetMenuHostWindow();
 
  protected:
   virtual RootView* CreateRootView();
 
-  virtual gboolean OnGrabBrokeEvent(GtkWidget* widget, GdkEvent* event);
-
   // Overriden to return false, we do NOT want to release capture on mouse
   // release.
   virtual bool ReleaseCaptureOnMouseReleased();
@@ -42,9 +42,14 @@ class MenuHost : public WidgetGtk {
   // Overriden to also release pointer grab.
   virtual void ReleaseGrab();
 
+  virtual void OnDestroy(GtkWidget* object);
+  virtual gboolean OnGrabBrokeEvent(GtkWidget* widget, GdkEvent* event);
+
  private:
-  // If true, we've been closed.
-  bool closed_;
+  void DoCapture();
+
+  // If true, DestroyMenuHost has been invoked.
+  bool destroying_;
 
   // The view we contain.
   SubmenuView* submenu_;
@@ -52,7 +57,7 @@ class MenuHost : public WidgetGtk {
   // Have we done a pointer grab?
   bool did_pointer_grab_;
 
-  DISALLOW_COPY_AND_ASSIGN(MenuHost);
+  DISALLOW_COPY_AND_ASSIGN(MenuHostGtk);
 };
 
 }  // namespace views
diff --git a/views/controls/menu/menu_host_root_view.cc b/views/controls/menu/menu_host_root_view.cc
index 1812801..720ea9a 100644
--- a/views/controls/menu/menu_host_root_view.cc
+++ b/views/controls/menu/menu_host_root_view.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -56,7 +56,7 @@ void MenuHostRootView::OnMouseReleased(const MouseEvent& event,
   if (forward_drag_to_menu_controller_) {
     forward_drag_to_menu_controller_ = false;
     if (canceled) {
-      GetMenuController()->Cancel(true);
+      GetMenuController()->Cancel(MenuController::EXIT_ALL);
     } else {
       GetMenuController()->OnMouseReleased(submenu_, event);
     }
diff --git a/views/controls/menu/menu_host_win.cc b/views/controls/menu/menu_host_win.cc
index f2c2c2c..384229f 100644
--- a/views/controls/menu/menu_host_win.cc
+++ b/views/controls/menu/menu_host_win.cc
@@ -12,8 +12,13 @@
 
 namespace views {
 
-MenuHost::MenuHost(SubmenuView* submenu)
-    : closed_(false),
+// static
+MenuHost* MenuHost::Create(SubmenuView* submenu_view) {
+  return new MenuHostWin(submenu_view);
+}
+
+MenuHostWin::MenuHostWin(SubmenuView* submenu)
+    : destroying_(false),
       submenu_(submenu),
       owns_capture_(false) {
   set_window_style(WS_POPUP);
@@ -30,88 +35,96 @@ MenuHost::MenuHost(SubmenuView* submenu)
   set_window_ex_style(WS_EX_TOPMOST | WS_EX_NOACTIVATE);
 }
 
-void MenuHost::Init(HWND parent,
-                    const gfx::Rect& bounds,
-                    View* contents_view,
-                    bool do_capture) {
+MenuHostWin::~MenuHostWin() {
+}
+
+void MenuHostWin::Init(HWND parent,
+                       const gfx::Rect& bounds,
+                       View* contents_view,
+                       bool do_capture) {
   WidgetWin::Init(parent, bounds);
   SetContentsView(contents_view);
-  Show();
-  if (do_capture)
-    DoCapture();
+  ShowMenuHost(do_capture);
 }
 
-void MenuHost::Show() {
+bool MenuHostWin::IsMenuHostVisible() {
+  return IsVisible();
+}
+
+void MenuHostWin::ShowMenuHost(bool do_capture) {
   // We don't want to take focus away from the hosting window.
   ShowWindow(SW_SHOWNA);
-}
 
-void MenuHost::Hide() {
-  if (closed_) {
-    // We're already closed, nothing to do.
-    // This is invoked twice if the first time just hid us, and the second
-    // time deleted Closed (deleted) us.
-    return;
-  }
-  // The menus are freed separately, and possibly before the window is closed,
-  // remove them so that View doesn't try to access deleted objects.
-  static_cast<MenuHostRootView*>(GetRootView())->suspend_events();
-  GetRootView()->RemoveAllChildViews(false);
-  closed_ = true;
-  ReleaseCapture();
-  WidgetWin::Hide();
+  if (do_capture)
+    DoCapture();
 }
 
-void MenuHost::HideWindow() {
+void MenuHostWin::HideMenuHost() {
   // Make sure we release capture before hiding.
-  ReleaseCapture();
+  ReleaseMenuHostCapture();
+
   WidgetWin::Hide();
 }
 
-void MenuHost::OnCaptureChanged(HWND hwnd) {
-  WidgetWin::OnCaptureChanged(hwnd);
-  owns_capture_ = false;
-#ifdef DEBUG_MENU
-  DLOG(INFO) << "Capture changed";
-#endif
+void MenuHostWin::DestroyMenuHost() {
+  HideMenuHost();
+  destroying_ = true;
+  CloseNow();
 }
 
-void MenuHost::DoCapture() {
-  owns_capture_ = true;
-  SetCapture();
-  has_capture_ = true;
-#ifdef DEBUG_MENU
-  DLOG(INFO) << "Doing capture";
-#endif
+void MenuHostWin::SetMenuHostBounds(const gfx::Rect& bounds) {
+  SetBounds(bounds);
 }
 
-void MenuHost::ReleaseCapture() {
+void MenuHostWin::ReleaseMenuHostCapture() {
   if (owns_capture_) {
-#ifdef DEBUG_MENU
-    DLOG(INFO) << "released capture";
-#endif
     owns_capture_ = false;
     ::ReleaseCapture();
   }
 }
 
-RootView* MenuHost::CreateRootView() {
-  return new MenuHostRootView(this, submenu_);
+gfx::NativeWindow MenuHostWin::GetMenuHostWindow() {
+  return GetNativeView();
+}
+
+void MenuHostWin::OnDestroy() {
+  if (!destroying_) {
+    // We weren't explicitly told to destroy ourselves, which means the menu was
+    // deleted out from under us (the window we're parented to was closed). Tell
+    // the SubmenuView to drop references to us.
+    submenu_->MenuHostDestroyed();
+  }
+  WidgetWin::OnDestroy();
 }
 
-void MenuHost::OnCancelMode() {
-  if (!closed_) {
+void MenuHostWin::OnCaptureChanged(HWND hwnd) {
+  WidgetWin::OnCaptureChanged(hwnd);
+  owns_capture_ = false;
 #ifdef DEBUG_MENU
-    DLOG(INFO) << "OnCanceMode, closing menu";
+  DLOG(INFO) << "Capture changed";
 #endif
-    submenu_->GetMenuItem()->GetMenuController()->Cancel(true);
-  }
 }
 
-// Overriden to return false, we do NOT want to release capture on mouse
-// release.
-bool MenuHost::ReleaseCaptureOnMouseReleased() {
+void MenuHostWin::OnCancelMode() {
+  submenu_->GetMenuItem()->GetMenuController()->Cancel(
+      MenuController::EXIT_ALL);
+}
+
+RootView* MenuHostWin::CreateRootView() {
+  return new MenuHostRootView(this, submenu_);
+}
+
+bool MenuHostWin::ReleaseCaptureOnMouseReleased() {
   return false;
 }
 
+void MenuHostWin::DoCapture() {
+  owns_capture_ = true;
+  SetCapture();
+  has_capture_ = true;
+#ifdef DEBUG_MENU
+  DLOG(INFO) << "Doing capture";
+#endif
+}
+
 }  // namespace views
diff --git a/views/controls/menu/menu_host_win.h b/views/controls/menu/menu_host_win.h
index ec5d060..f7d9cb4 100644
--- a/views/controls/menu/menu_host_win.h
+++ b/views/controls/menu/menu_host_win.h
@@ -6,6 +6,7 @@
 #ifndef VIEWS_CONTROLS_MENU_MENU_HOST_WIN_H_
 #define VIEWS_CONTROLS_MENU_MENU_HOST_WIN_H_
 
+#include "views/controls/menu/menu_host.h"
 #include "views/widget/widget_win.h"
 
 namespace views {
@@ -13,36 +14,41 @@ namespace views {
 class SubmenuView;
 
 // MenuHost implementation for windows.
-class MenuHost : public WidgetWin {
+class MenuHostWin : public WidgetWin, public MenuHost {
  public:
-  explicit MenuHost(SubmenuView* submenu);
-
-  void Init(HWND parent,
-            const gfx::Rect& bounds,
-            View* contents_view,
-            bool do_capture);
-
-  gfx::NativeWindow GetNativeWindow() { return GetNativeView(); }
-
-  void Show();
-  virtual void Hide();
-  virtual void HideWindow();
+  explicit MenuHostWin(SubmenuView* submenu);
+  virtual ~MenuHostWin();
+
+  // MenuHost overrides:
+  virtual void Init(HWND parent,
+                    const gfx::Rect& bounds,
+                    View* contents_view,
+                    bool do_capture);
+  virtual bool IsMenuHostVisible();
+  virtual void ShowMenuHost(bool do_capture);
+  virtual void HideMenuHost();
+  virtual void DestroyMenuHost();
+  virtual void SetMenuHostBounds(const gfx::Rect& bounds);
+  virtual void ReleaseMenuHostCapture();
+  virtual gfx::NativeWindow GetMenuHostWindow();
+
+  // WidgetWin overrides:
+  virtual void OnDestroy();
   virtual void OnCaptureChanged(HWND hwnd);
-  void DoCapture();
-  void ReleaseCapture();
+  virtual void OnCancelMode();
 
  protected:
   virtual RootView* CreateRootView();
 
-  virtual void OnCancelMode();
-
   // Overriden to return false, we do NOT want to release capture on mouse
   // release.
   virtual bool ReleaseCaptureOnMouseReleased();
 
  private:
-  // If true, we've been closed.
-  bool closed_;
+  void DoCapture();
+
+  // If true, DestroyMenuHost has been invoked.
+  bool destroying_;
 
   // If true, we own the capture and need to release it.
   bool owns_capture_;
@@ -50,7 +56,7 @@ class MenuHost : public WidgetWin {
   // The view we contain.
   SubmenuView* submenu_;
 
-  DISALLOW_COPY_AND_ASSIGN(MenuHost);
+  DISALLOW_COPY_AND_ASSIGN(MenuHostWin);
 };
 
 }  // namespace views
diff --git a/views/controls/menu/menu_item_view.cc b/views/controls/menu/menu_item_view.cc
index 79f4306..64b8270 100644
--- a/views/controls/menu/menu_item_view.cc
+++ b/views/controls/menu/menu_item_view.cc
@@ -98,7 +98,7 @@ void MenuItemView::RunMenuAt(gfx::NativeWindow parent,
     // A menu is already showing, but it isn't a blocking menu. Cancel it.
     // We can get here during drag and drop if the user right clicks on the
     // menu quickly after the drop.
-    controller->Cancel(true);
+    controller->Cancel(MenuController::EXIT_ALL);
     controller = NULL;
   }
   bool owns_controller = false;
@@ -145,7 +145,7 @@ void MenuItemView::RunMenuForDropAt(gfx::NativeWindow parent,
   // If there is a menu, hide it so that only one menu is shown during dnd.
   MenuController* current_controller = MenuController::GetActiveInstance();
   if (current_controller) {
-    current_controller->Cancel(true);
+    current_controller->Cancel(MenuController::EXIT_ALL);
   }
 
   // Always create a new controller for non-blocking.
@@ -160,7 +160,7 @@ void MenuItemView::RunMenuForDropAt(gfx::NativeWindow parent,
 void MenuItemView::Cancel() {
   if (controller_ && !canceled_) {
     canceled_ = true;
-    controller_->Cancel(true);
+    controller_->Cancel(MenuController::EXIT_ALL);
   }
 }
 
diff --git a/views/controls/menu/submenu_view.cc b/views/controls/menu/submenu_view.cc
index 8136794..52c9101 100644
--- a/views/controls/menu/submenu_view.cc
+++ b/views/controls/menu/submenu_view.cc
@@ -6,15 +6,10 @@
 
 #include "gfx/canvas.h"
 #include "views/controls/menu/menu_controller.h"
+#include "views/controls/menu/menu_host.h"
 #include "views/controls/menu/menu_scroll_view_container.h"
 #include "views/widget/root_view.h"
 
-#if defined(OS_WIN)
-#include "views/controls/menu/menu_host_win.h"
-#elif defined(OS_LINUX)
-#include "views/controls/menu/menu_host_gtk.h"
-#endif
-
 // Height of the drop indicator. This should be an even number.
 static const int kDropIndicatorHeight = 2;
 
@@ -208,20 +203,18 @@ bool SubmenuView::OnMouseWheel(const MouseWheelEvent& e) {
 }
 
 bool SubmenuView::IsShowing() {
-  return host_ && host_->IsVisible();
+  return host_ && host_->IsMenuHostVisible();
 }
 
 void SubmenuView::ShowAt(gfx::NativeWindow parent,
                          const gfx::Rect& bounds,
                          bool do_capture) {
   if (host_) {
-    host_->Show();
-    if (do_capture)
-      host_->DoCapture();
+    host_->ShowMenuHost(do_capture);
     return;
   }
 
-  host_ = new MenuHost(this);
+  host_ = MenuHost::Create(this);
   // Force construction of the scroll view container.
   GetScrollViewContainer();
   // Make sure the first row is visible.
@@ -230,23 +223,25 @@ void SubmenuView::ShowAt(gfx::NativeWindow parent,
 }
 
 void SubmenuView::Reposition(const gfx::Rect& bounds) {
-  host_->SetBounds(bounds);
+  if (host_)
+    host_->SetMenuHostBounds(bounds);
 }
 
 void SubmenuView::Close() {
   if (host_) {
-    host_->Close();
+    host_->DestroyMenuHost();
     host_ = NULL;
   }
 }
 
 void SubmenuView::Hide() {
   if (host_)
-    host_->HideWindow();
+    host_->HideMenuHost();
 }
 
 void SubmenuView::ReleaseCapture() {
-  host_->ReleaseCapture();
+  if (host_)
+    host_->ReleaseMenuHostCapture();
 }
 
 bool SubmenuView::SkipDefaultKeyEventProcessing(const views::KeyEvent& e) {
@@ -282,7 +277,12 @@ MenuScrollViewContainer* SubmenuView::GetScrollViewContainer() {
 }
 
 gfx::NativeWindow SubmenuView::native_window() const {
-  return host_ ? host_->GetNativeWindow() : NULL;
+  return host_ ? host_->GetMenuHostWindow() : NULL;
+}
+
+void SubmenuView::MenuHostDestroyed() {
+  host_ = NULL;
+  GetMenuItem()->GetMenuController()->Cancel(MenuController::EXIT_DESTROYED);
 }
 
 void SubmenuView::PaintDropIndicator(gfx::Canvas* canvas,
diff --git a/views/controls/menu/submenu_view.h b/views/controls/menu/submenu_view.h
index 1e82f73..fd2bd02 100644
--- a/views/controls/menu/submenu_view.h
+++ b/views/controls/menu/submenu_view.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -118,6 +118,11 @@ class SubmenuView : public View {
   // Returns the NativeWindow host of the menu, or NULL if not showing.
   gfx::NativeWindow native_window() const;
 
+  // Invoked if the menu is prematurely destroyed. This can happen if the window
+  // closes while the menu is shown. If invoked the SubmenuView must drop all
+  // references to the MenuHost as the MenuHost is about to be deleted.
+  void MenuHostDestroyed();
+
   // Padding around the edges of the submenu.
   static const int kSubmenuBorderSize;
 
@@ -138,7 +143,8 @@ class SubmenuView : public View {
   // Parent menu item.
   MenuItemView* parent_menu_item_;
 
-  // Widget subclass used to show the children.
+  // Widget subclass used to show the children. This is deleted when we invoke
+  // |DestroyMenuHost|, or |MenuHostDestroyed| is invoked back on us.
   MenuHost* host_;
 
   // If non-null, indicates a drop is in progress and drop_item is the item
diff --git a/views/view_unittest.cc b/views/view_unittest.cc
index 8832c43..617d25c 100644
--- a/views/view_unittest.cc
+++ b/views/view_unittest.cc
@@ -687,6 +687,8 @@ class TestViewsDelegate : public views::ViewsDelegate {
   virtual HICON GetDefaultWindowIcon() const {
     return NULL;
   }
+  virtual void AddRef() {}
+  virtual void ReleaseRef() {}
 
  private:
   mutable scoped_ptr<Clipboard> clipboard_;
diff --git a/views/views.gyp b/views/views.gyp
index 3369546..950fb86 100644
--- a/views/views.gyp
+++ b/views/views.gyp
@@ -111,6 +111,7 @@
         'controls/menu/menu_delegate.h',
         'controls/menu/menu_gtk.cc',
         'controls/menu/menu_gtk.h',
+        'controls/menu/menu_host.h',
         'controls/menu/menu_host_root_view.cc',
         'controls/menu/menu_host_root_view.h',
         'controls/menu/menu_host_win.cc',
diff --git a/views/views_delegate.h b/views/views_delegate.h
index 6fe7f74..d78304c 100644
--- a/views/views_delegate.h
+++ b/views/views_delegate.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved. Use of this
+// Copyright (c) 2010 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.
 
@@ -49,6 +49,11 @@ class ViewsDelegate {
   virtual HICON GetDefaultWindowIcon() const = 0;
 #endif
 
+  // AddRef/ReleaseRef are invoked while a menu is visible. They are used to
+  // ensure we don't attempt to exit while a menu is showing.
+  virtual void AddRef() = 0;
+  virtual void ReleaseRef() = 0;
+
   // The active ViewsDelegate used by the views system.
   static ViewsDelegate* views_delegate;
 };
diff --git a/views/widget/widget_gtk.cc b/views/widget/widget_gtk.cc
index d850b90..006a089 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -989,6 +989,9 @@ gboolean WidgetGtk::OnGrabBrokeEvent(GtkWidget* widget, GdkEvent* event) {
 }
 
 void WidgetGtk::OnGrabNotify(GtkWidget* widget, gboolean was_grabbed) {
+  if (!window_contents_)
+    return;  // Grab broke after window destroyed, don't try processing it.
+
   gtk_grab_remove(window_contents_);
   HandleGrabBroke();
 }