Fixed Issue 141873. Crash when BrowserActionButton get disabled or hidden.

chrome/browser/ui/views/browser_action_view.cc

Index: chrome/browser/ui/views/browser_action_view.cc
diff --git a/chrome/browser/ui/views/browser_action_view.cc b/chrome/browser/ui/views/browser_action_view.cc
index 01877ec97e7a4cc41555242559ea06057161748d..579e14b68cfde999a1c2b352f9365726eab73ddc 100644
--- a/chrome/browser/ui/views/browser_action_view.cc
+++ b/chrome/browser/ui/views/browser_action_view.cc
@@ -59,9 +59,15 @@ BrowserActionView::BrowserActionView(const Extension* extension,
       delegate_(delegate),
       button_(NULL),
       extension_(extension) {
+  button_ = new BrowserActionButton(extension_, browser_, delegate_);
+  button_->set_drag_controller(delegate_);
+  AddChildView(button_);
+  button_->UpdateState();
 }
 
 BrowserActionView::~BrowserActionView() {
+  RemoveChildView(button_);
+  button_->Destroy();

[sky, 2012/08/10 19:01:27]

Why does BrowserActionButton need a Destroy()? Can't it do the cleanup it needs
in ~BrowserActionButton?

[yefimt, 2012/08/10 19:27:05]

It was the original code before my previous CL, so I just reverting it back to
how it was.

 }
 
 gfx::Canvas* BrowserActionView::GetIconWithBadge() {
@@ -96,18 +102,6 @@ void BrowserActionView::Layout() {
                      BrowserActionsContainer::IconHeight());
 }
 
-void BrowserActionView::ViewHierarchyChanged(bool is_add,
-                                             View* parent,
-                                             View* child) {
-  if (is_add && (child == this)) {
-    button_ = new BrowserActionButton(extension_, browser_, delegate_);
-    button_->set_drag_controller(delegate_);
-
-    AddChildView(button_);
-    button_->UpdateState();
-  }
-}
-
 void BrowserActionView::GetAccessibleState(ui::AccessibleViewState* state) {
   state->name = l10n_util::GetStringUTF16(
       IDS_ACCNAME_EXTENSIONS_BROWSER_ACTION);
@@ -230,8 +224,9 @@ void BrowserActionButton::ShowContextMenuForView(View* source,
           views::MenuRunner::MENU_DELETED) {
     return;
   }
+  // BrowserActionButton could be deleted by this point,
+  // i.e. if user selected Disable from the menu.
 
-  menu_runner_.reset();

[sky, 2012/08/10 19:01:27]

Why are you removing this?

[yefimt, 2012/08/10 19:27:05]

It is crashing here if BrowserActionButton is deleted by this point. I added
this line in my previous CL because it looked reasonable for me, but it causes
crash now.

   SetButtonNotPushed();
   context_menu_ = NULL;
 }