Moves ownership of MenuItemView to MenuRunner as well as responbility

views/controls/menu/menu_runner.h

Index: views/controls/menu/menu_runner.h
diff --git a/views/controls/menu/menu_runner.h b/views/controls/menu/menu_runner.h
index 1337d0818d0aef9860d8e2a2e47fe74f4e4f503d..5cdb845f2943b0ab830da33a421794d1944d9779 100644
--- a/views/controls/menu/menu_runner.h
+++ b/views/controls/menu/menu_runner.h
@@ -7,6 +7,7 @@
 #pragma once
 
 #include "base/basictypes.h"
+#include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "views/controls/menu/menu_item_view.h"
 
@@ -15,32 +16,73 @@ namespace views {
 class MenuButton;
 class Widget;
 
-// 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.
+namespace internal {
+class MenuRunnerImpl;
+}
+
+// MenuRunner is responsible for showing (running) the menu and additionally
+// owning the MenuItemView. RunMenuAt() runs a nested message loop. It is safe
+// to delete MenuRunner at any point, but MenuRunner internally only deletes the
+// MenuItemView *after* the nested message loop completes. If MenuRunner is
+// deleted while the menu is showing the delegate of the menu is reset. This is
+// done to ensure delegates aren't notified after they may have been deleted.
 //
-// TODO: this is a work around for 57890. If we fix it this class shouldn't be
-// needed.
+// NOTE: while you can delete a MenuRunner at any point, the nested message loop
+// won't return immediately. This means if you delete the object that owns
+// the MenuRunner while the menu is running, your object is effectively still
+// on the stack. A return value of MENU_DELETED indicated this. In most cases
+// if RunMenuAt() returns MENU_DELETED, you should return immediately.
 class VIEWS_EXPORT MenuRunner {
  public:
+  enum RunTypes {
+    // The menu has mnemonics.
+    HAS_MNEMONICS = 1 << 0,
+
+    // The menu is a nested context menu. For example, click a folder on the
+    // bookmark bar, then right click an entry to get its context menu.
+    IS_NESTED     = 1 << 1,
+
+    // Used for showing a menu during a drop operation. This does NOT block the
+    // caller, instead the delegate is notified when the menu closes via the
+    // DropMenuClosed method.
+    FOR_DROP      = 1 << 2,
+  };
+
+  enum RunResult {
+    // Indicates RunMenuAt is returning because the MenuRunner was deleted.
+    MENU_DELETED,
+
+    // Indicates RunMenuAt returned and MenuRunner was not deleted.
+    NORMAL_EXIT
+  };
+
+  // Creates a new MenuRunner. MenuRunner owns the supplied menu.
   explicit MenuRunner(MenuItemView* menu);
   ~MenuRunner();
 
-  // Runs the menu.
-  void RunMenuAt(Widget* parent,
-                 MenuButton* button,
-                 const gfx::Rect& bounds,
-                 MenuItemView::AnchorPosition anchor,
-                 bool has_mnemonics);
+  // Returns the menu.
+  MenuItemView* GetMenu();
 
- private:
-  class Holder;
+  // Takes ownership of |menu|, deleting it when MenuRunner is deleted. You
+  // only need call this if you create additional menus from
+  // MenuDelegate::GetSiblingMenu.
+  void OwnMenu(MenuItemView* menu);
+
+  // Runs the menu. |types| is a bitmask of RunTypes. If this returns
+  // MENU_DELETED the method is returning because the MenuRunner was deleted.
+  // Typically callers should NOT do any processing if this returns
+  // MENU_DELETED.
+  RunResult RunMenuAt(Widget* parent,
+                      MenuButton* button,
+                      const gfx::Rect& bounds,
+                      MenuItemView::AnchorPosition anchor,
+                      int32 types) WARN_UNUSED_RESULT;
 
-  Holder* holder_;
+  // Hides and cancels the menu. This does nothing if the menu is not open.
+  void Cancel();
+
+ private:
+  internal::MenuRunnerImpl* holder_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuRunner);
 };