[Extensions Toolbar Mac] More cleanup of BrowserActionsController

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/stl_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 61 matching lines @@
     : toolbar_actions_bar_(new ToolbarActionsBar(this,
                                                  browser,
                                                  main_container != nullptr)),
       browser_(browser),
       main_container_(main_container),
       popup_owner_(NULL),
       container_width_(0),
       resize_area_(NULL),
       chevron_(NULL),
       suppress_chevron_(false),
+      added_to_view_(false),
       resize_amount_(0),
       animation_target_size_(0) {
   set_id(VIEW_ID_BROWSER_ACTION_TOOLBAR);
 
   bool overflow_experiment =
       extensions::FeatureSwitch::extension_action_redesign()->IsEnabled();
   DCHECK(!in_overflow_mode() || overflow_experiment);
 
   if (!in_overflow_mode()) {
     resize_animation_.reset(new gfx::SlideAnimation(this));
@@ skipping 152 matching lines @@
     }
   }
 }
 
 void BrowserActionsContainer::RemoveAllViews() {
   HideActivePopup();
   STLDeleteElements(&toolbar_action_views_);
 }
 
 void BrowserActionsContainer::Redraw(bool order_changed) {
+  if (!added_to_view_) {
+    // We don't want to redraw before the view has been fully added to the
+    // hierarchy.
+    return;
+  }
+
   std::vector<ToolbarActionViewController*> actions =
       toolbar_actions_bar_->toolbar_actions();
   if (order_changed) {
     // Run through the views and compare them to the desired order. If something
     // is out of place, find the correct spot for it.
     for (int i = 0; i < static_cast<int>(actions.size()) - 1; ++i) {
       if (actions[i] != toolbar_action_views_[i]->view_controller()) {
         // Find where the correct view is (it's guaranteed to be after our
         // current index, since everything up to this point is correct).
         int j = i + 1;
@@ skipping 445 matching lines @@
     canvas->FillRect(indicator_bounds, kDropIndicatorColor);
   }
 }
 
 void BrowserActionsContainer::OnThemeChanged() {
   LoadImages();
 }
 
 void BrowserActionsContainer::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   if (!toolbar_actions_bar_->enabled())

[sky, 2014/11/24 21:54:43]

Is it possible this is added when toolbar_actions_bar_isn't enabled and then
toolbar_actions_bar_ is enabled?

[Devlin, 2014/11/24 22:00:28]

Nope.  ToolbarActionsBar::enabled() corresponds to the presence of an
ExtensionToolbarModel - which is only null in unittests.  If there is, or should
ever be, anything in the browser actions container, then enabled() will be true
from construction.

     return;
 
   if (details.is_add && details.child == this) {
     if (!in_overflow_mode()) {  // We only need one keybinding registry.
       extension_keybinding_registry_.reset(new ExtensionKeybindingRegistryViews(
           browser_->profile(),
           parent()->GetFocusManager(),
           extensions::ExtensionKeybindingRegistry::ALL_EXTENSIONS,
           this));
     }
 
     // Initial toolbar button creation and placement in the widget hierarchy.
     // We do this here instead of in the constructor because adding views
     // calls Layout on the Toolbar, which needs this object to be constructed
     // before its Layout function is called.
     toolbar_actions_bar_->CreateActions();
+
+    added_to_view_ = true;
   }
 }
 
 void BrowserActionsContainer::LoadImages() {
   if (in_overflow_mode())
     return;  // Overflow mode has neither a chevron nor highlighting.
 
   ui::ThemeProvider* tp = GetThemeProvider();
   if (tp && chevron_) {
     chevron_->SetImage(views::Button::STATE_NORMAL,
                        *tp->GetImageSkiaNamed(IDR_BROWSER_ACTIONS_OVERFLOW));
   }
 
   const int kImages[] = IMAGE_GRID(IDR_DEVELOPER_MODE_HIGHLIGHT);
   highlight_painter_.reset(views::Painter::CreateImageGridPainter(kImages));
 }