Fix use-after-free in BrowserActionOverflowMenuController

chrome/browser/ui/views/toolbar/browser_actions_container.cc

 // Copyright 2013 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 "chrome/browser/ui/views/toolbar/browser_actions_container.h"
 
 #include "base/compiler_specific.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ skipping 218 matching lines @@
       continue;
 
     BrowserActionView* view = new BrowserActionView(i->get(), browser_, this);
     browser_action_views_.push_back(view);
     AddChildView(view);
   }
 }
 
 void BrowserActionsContainer::DeleteBrowserActionViews() {
   HidePopup();
+  if (overflow_menu_)

[msw, 2014/07/09 16:56:27]

Can we simply close the overflow menu on DeleteBrowserActionViews?

[Devlin, 2014/07/09 19:23:27]

I'm not sure that we can safely - DeleteBrowserActionViews() can be called as a
result of moving browser actions, which can be triggered by the overflow menu. 
So it the flow would become:

OverflowMenu::OnDrop
  MoveBrowserAction()
  Finish()

MoveBrowserAction()
  DeleteBrowserActions()
  ...

DeleteBrowserActions()
  CloseOverflowMenu()
  Delete...

(All pseudocode, obviously).  The problem is that the menu would be canceled via
the call to close, but then would try to finish.

+    overflow_menu_->NotifyBrowserActionViewsDeleting();
   STLDeleteElements(&browser_action_views_);
 }
 
 size_t BrowserActionsContainer::VisibleBrowserActions() const {
   size_t visible_actions = 0;
   for (size_t i = 0; i < browser_action_views_.size(); ++i) {
     if (browser_action_views_[i]->visible())
       ++visible_actions;
   }
   return visible_actions;
@@ skipping 804 matching lines @@
                                      views::BubbleBorder::TOP_RIGHT,
                                      show_action);
   popup_->GetWidget()->AddObserver(this);
   popup_button_ = button;
 
   // Only set button as pushed if it was triggered by a user click.
   if (should_grant)
     popup_button_->SetButtonPushed();
   return true;
 }