Fix disappearing page actions bug.

chrome/browser/ui/views/location_bar/location_bar_view.cc

 // Copyright (c) 2012 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/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/i18n/rtl.h"
@@ skipping 1038 matching lines @@
        i != content_setting_views_.end(); ++i) {
     const bool was_visible = (*i)->visible();
     (*i)->Update(GetToolbarModel()->input_in_progress() ?
         NULL : GetWebContents());
     if (was_visible != (*i)->visible())
       visibility_changed = true;
   }
   return visibility_changed;
 }
 
-void LocationBarView::DeletePageActionViews() {
+void LocationBarView::DeletePageActionsAndViews() {
   for (PageActionViews::const_iterator i(page_action_views_.begin());
        i != page_action_views_.end(); ++i)
     RemoveChildView(*i);
   STLDeleteElements(&page_action_views_);
+  STLDeleteElements(&page_actions_);
 }
 
 bool LocationBarView::RefreshPageActionViews() {
   if (is_popup_mode_)
     return false;
 
   bool changed = false;
   PageActions new_page_actions;
 
   WebContents* web_contents = GetWebContents();
   if (web_contents) {
     extensions::TabHelper* extensions_tab_helper =
         extensions::TabHelper::FromWebContents(web_contents);
     extensions::LocationBarController* controller =
         extensions_tab_helper->location_bar_controller();
     new_page_actions = controller->GetCurrentActions();
   }
 
   // On startup we sometimes haven't loaded any extensions. This makes sure
   // we catch up when the extensions (and any page actions) load.
   if (page_actions_ != new_page_actions) {
     changed = true;
 
     page_actions_.swap(new_page_actions);
-    DeletePageActionViews();  // Delete the old views (if any).
+    page_action_views.clear();

[danduong, 2014/12/20 03:09:22]

This should be page_action_views_. Also we need to remove them from the view
hierarchy.

[erikchen, 2014/12/22 20:39:43]

Done.

 
     // Create the page action views.
     for (PageActions::const_iterator i = page_actions_.begin();
          i != page_actions_.end(); ++i) {
       PageActionWithBadgeView* page_action_view = new PageActionWithBadgeView(
           delegate_->CreatePageActionImageView(this, *i));
       page_action_view->SetVisible(false);
       page_action_views_.push_back(page_action_view);
     }
 
@@ skipping 187 matching lines @@
 }
 
 void LocationBarView::UpdatePageActions() {
   if (RefreshPageActionViews()) {  // Changed.
     Layout();
     SchedulePaint();
   }
 }
 
 void LocationBarView::InvalidatePageActions() {
-  DeletePageActionViews();
+  DeletePageActionsAndViews();

[danduong, 2014/12/20 04:28:40]

Hah! I think the bug is actually here.

This probably should be refresh page action views NOT delete.  If you use
Chrome's task manager and kill an extension's background page. This bug is 100%
reproducible.  Also makes the crash bug really easy to reproduce too.

[danduong, 2014/12/20 04:59:04]

I just realized that you commented on this in the bug.  Is there any reason why
we shouldn't just call Refresh at this point?

[erikchen, 2014/12/22 20:39:43]

We should be calling Refresh. This is another bug.

 }
 
 void LocationBarView::UpdateBookmarkStarVisibility() {
   if (star_view_) {
     star_view_->SetVisible(
         browser_defaults::bookmarks_enabled && !is_popup_mode_ &&
         !GetToolbarModel()->input_in_progress() &&
         edit_bookmarks_enabled_.GetValue() &&
         !IsBookmarkStarHiddenByExtension());
   }
@@ skipping 358 matching lines @@
 
 void LocationBarView::ModelChanged(const SearchModel::State& old_state,
                                    const SearchModel::State& new_state) {
   const bool visible = !GetToolbarModel()->input_in_progress() &&
       new_state.voice_search_supported;
   if (mic_search_view_->visible() != visible) {
     mic_search_view_->SetVisible(visible);
     Layout();
   }
 }