Linux: Only close the wrench menu when the bookmark model changes if …

…the bookmark submenu is actually showing.

Also, fix a case where we could get into a state where we could not close the menu at all, even if the model changed.
Review URL: https://chromiumcodereview.appspot.com/9748003

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

chrome/browser/ui/gtk/bookmarks/bookmark_sub_menu_model_gtk.cc

@@ -121,7 +121,8 @@ BookmarkSubMenuModel::BookmarkSubMenuModel(
       browser_(browser),
       fixed_items_(0),
       bookmark_end_(0),
-      menu_(NULL) {
+      menu_(NULL),
+      menu_showing_(false) {
 }
 
 BookmarkSubMenuModel::~BookmarkSubMenuModel() {
@@ -136,7 +137,7 @@ void BookmarkSubMenuModel::Loaded(BookmarkModel* model, bool ids_reassigned) {
 }
 
 void BookmarkSubMenuModel::BookmarkModelChanged() {
-  if (menu_)
+  if (menu_showing_ && menu_)
     menu_->Cancel();
 }
 
@@ -149,6 +150,7 @@ void BookmarkSubMenuModel::BookmarkModelBeingDeleted(
 }
 
 void BookmarkSubMenuModel::MenuWillShow() {
+  menu_showing_ = true;
   Clear();
   AddCheckItemWithStringId(IDC_SHOW_BOOKMARK_BAR, IDS_SHOW_BOOKMARK_BAR);
   AddItemWithStringId(IDC_SHOW_BOOKMARK_MANAGER, IDS_BOOKMARK_MANAGER);
@@ -188,6 +190,11 @@ void BookmarkSubMenuModel::MenuWillShow() {
   }
 }
 
+void BookmarkSubMenuModel::MenuClosed() {
+  menu_showing_ = false;
+  BookmarkNodeMenuModel::MenuClosed();
+}
+
 void BookmarkSubMenuModel::ActivatedAt(int index) {
   // Because this is also overridden in BookmarkNodeMenuModel which doesn't know
   // we might be prepending items, we have to adjust the index for it.

chrome/browser/ui/gtk/bookmarks/bookmark_sub_menu_model_gtk.h

@@ -97,6 +97,7 @@ class BookmarkSubMenuModel : public BookmarkNodeMenuModel,
 
   // From MenuModel via BookmarkNodeMenuModel, SimpleMenuModel.
   virtual void MenuWillShow() OVERRIDE;
+  virtual void MenuClosed() OVERRIDE;
   virtual void ActivatedAt(int index) OVERRIDE;
   virtual void ActivatedAt(int index, int event_flags) OVERRIDE;
   virtual bool IsEnabledAt(int index) const OVERRIDE;
@@ -113,11 +114,16 @@ class BookmarkSubMenuModel : public BookmarkNodeMenuModel,
   // The index of the first non-bookmark item after the bookmarks.
   int bookmark_end_;
 
-  // We need to be able to call Cancel() on the menu when bookmarks change. This
-  // is a bit of an abstraction violation but it could be converted to an
-  // interface with just a Cancel() method if necessary.
+  // We need to be able to call Cancel() on the wrench menu when bookmarks
+  // change. This is a bit of an abstraction violation but it could be converted
+  // to an interface with just a Cancel() method if necessary.
   MenuGtk* menu_;
 
+  // We keep track of whether the bookmark submenu is currently showing. If it's
+  // not showing, then we don't need to forcibly close the entire wrench menu
+  // when the bookmark model loads.
+  bool menu_showing_;
+
   DISALLOW_COPY_AND_ASSIGN(BookmarkSubMenuModel);
 };
 

chrome/browser/ui/gtk/browser_toolbar_gtk.cc

@@ -658,6 +658,8 @@ bool BrowserToolbarGtk::ShouldOnlyShowLocation() const {
 void BrowserToolbarGtk::RebuildWrenchMenu() {
   wrench_menu_model_.reset(new WrenchMenuModel(this, browser_));
   wrench_menu_.reset(new MenuGtk(this, wrench_menu_model_.get()));
+  // The bookmark menu model needs to be able to force the wrench menu to close.
+  wrench_menu_model_->bookmark_sub_menu_model()->SetMenuGtk(wrench_menu_.get());
   is_wrench_menu_model_valid_ = true;
 }