[Extensions Toolbar] Make the ExtensionToolbarModel icon count more stable

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

 // Copyright 2014 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/toolbar/toolbar_actions_bar.h"
 
 #include "base/auto_reset.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ skipping 184 matching lines @@
   }
 #endif
 
   // The overflow displays any icons not shown by the main bar.
   return in_overflow_mode_ ?
       model_->toolbar_items().size() - model_visible_size : model_visible_size;
 }
 
 void ToolbarActionsBar::CreateActions() {
   DCHECK(toolbar_actions_.empty());
-  if (!model_)
+  // We wait for the extension system to be initialized before we add any
+  // actions, as they rely on the extension system to function; we also have
+  // to wait for the delegate to be ready to create any added views.
+  if (!model_ || !model_->extensions_initialized() || !delegate_->IsReady())

[Peter Kasting, 2014/11/14 21:31:58]

Hmm.  Is there code somewhere that triggers another call to this function when
each of these conditions changes, so we'll be sure to create actions as soon as
possible?

Is it possible to know for sure that one of these will happen after the others,
so we can just be gated on it?  (Then the change of that conditional could
trigger the call to here, removing the need to check anything here.)

[Devlin, 2014/11/14 22:00:37]

Yep - the delegate (the view for the browser actions container) will call
CreateActions as soon as its ready (for mac, on construction, for views, in
ViewHierarchyChanged()), and this class calls it when the toolbar model is
initialized.
 
Sadly, no - When first starting up the browser, the view will be ready before
the model is initialized; for subsequent windows, the model is already
initialized, so we have to wait on the view.

     return;
 
   {
     // We don't redraw the view while creating actions.
     base::AutoReset<bool> layout_resetter(&suppress_layout_, true);
 
     // Extension actions come first.
     extensions::ExtensionActionManager* action_manager =
         extensions::ExtensionActionManager::Get(browser_->profile());
     const extensions::ExtensionList& toolbar_items = model_->GetItemOrderForTab(
@@ skipping 206 matching lines @@
   // appearing, etc), it's not worth the extra complexity to create and insert
   // only the new actions.
   DeleteActions();
   CreateActions();
   delegate_->ResizeAndAnimate(gfx::Tween::LINEAR,
                               GetPreferredSize().width(),
                               true);  // suppress chevron
 }
 
 void ToolbarActionsBar::OnToolbarModelInitialized() {
-  ToolbarVisibleCountChanged();  // We may not have resized since the start.
-  delegate_->Redraw(false);  // Order didn't change.
+  // We shouldn't have any actions before the model is initialized.
+  DCHECK(toolbar_actions_.empty());
   suppress_animation_ = false;
+  CreateActions();
+  if (!toolbar_actions_.empty()) {
+    delegate_->Redraw(false);
+    ToolbarVisibleCountChanged();
+  }
 }
 
 void ToolbarActionsBar::OnToolbarReorderNecessary(
     content::WebContents* web_contents) {
   if (GetCurrentWebContents() == web_contents)
     ReorderActions();
 }
 
 Browser* ToolbarActionsBar::GetBrowser() {
   return browser_;
@@ skipping 41 matching lines @@
   for (ToolbarActionViewController* action : toolbar_actions_) {
     if (action->GetId() == id)
       return action;
   }
   return nullptr;
 }
 
 content::WebContents* ToolbarActionsBar::GetCurrentWebContents() {
   return browser_->tab_strip_model()->GetActiveWebContents();
 }