Support Component Actions in the toolbar.

chrome/browser/ui/toolbar/toolbar_actions_model.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/extensions/extension_toolbar_model.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/location.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_base.h"
 #include "base/prefs/pref_service.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
-#include "chrome/browser/extensions/extension_toolbar_model_factory.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/extensions/extension_action_view_controller.h"
 #include "chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
+#include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_model_factory.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
-#include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/pref_names.h"
-#include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/one_shot_event.h"
 
-namespace extensions {
-
-ExtensionToolbarModel::ExtensionToolbarModel(Profile* profile,
-                                             ExtensionPrefs* extension_prefs)
+ToolbarActionsModel::ToolbarActionsModel(Profile* profile,
+    extensions::ExtensionPrefs* extension_prefs)
     : profile_(profile),
-      extension_prefs_(extension_prefs),
+      prefs_service_(extension_prefs),
       prefs_(profile_->GetPrefs()),
-      extension_action_api_(ExtensionActionAPI::Get(profile_)),
-      extensions_initialized_(false),
-      include_all_extensions_(FeatureSwitch::extension_action_redesign()
-                                  ->IsEnabled()),
+      extension_action_api_(extensions::ExtensionActionAPI::Get(profile_)),
+      extension_registry_(extensions::ExtensionRegistry::Get(profile_)),
+      actions_initialized_(false),
+      include_all_extensions_(

[Devlin, 2015/08/06 22:04:56]

Let's just rename this to, like, use_redesign_.

[apacible, 2015/08/10 22:38:10]

Done.

+          extensions::FeatureSwitch::extension_action_redesign()->IsEnabled()),
       highlight_type_(HIGHLIGHT_NONE),
       extension_action_observer_(this),
       extension_registry_observer_(this),
       weak_ptr_factory_(this) {
-  ExtensionSystem::Get(profile_)->ready().Post(
+  extensions::ExtensionSystem::Get(profile_)->ready().Post(
       FROM_HERE,
-      base::Bind(&ExtensionToolbarModel::OnReady,
+      base::Bind(&ToolbarActionsModel::OnReady,
                  weak_ptr_factory_.GetWeakPtr()));
-  visible_icon_count_ = prefs_->GetInteger(pref_names::kToolbarSize);
+  visible_icon_count_ = prefs_->GetInteger(
+      extensions::pref_names::kToolbarSize);
 
   // We only care about watching the prefs if not in incognito mode.
   if (!profile_->IsOffTheRecord()) {
     pref_change_registrar_.Init(prefs_);
     pref_change_callback_ =
-        base::Bind(&ExtensionToolbarModel::OnExtensionToolbarPrefChange,
+        base::Bind(&ToolbarActionsModel::OnActionToolbarPrefChange,
                    base::Unretained(this));
-    pref_change_registrar_.Add(pref_names::kToolbar, pref_change_callback_);
+    pref_change_registrar_.Add(
+        extensions::pref_names::kToolbar, pref_change_callback_);
   }
 }
 
-ExtensionToolbarModel::~ExtensionToolbarModel() {
+ToolbarActionsModel::~ToolbarActionsModel() {
 }
 
 // static
-ExtensionToolbarModel* ExtensionToolbarModel::Get(Profile* profile) {
-  return ExtensionToolbarModelFactory::GetForProfile(profile);
+ToolbarActionsModel* ToolbarActionsModel::Get(Profile* profile) {
+  return ToolbarActionsModelFactory::GetForProfile(profile);
 }
 
-void ExtensionToolbarModel::AddObserver(Observer* observer) {
+void ToolbarActionsModel::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
 
-void ExtensionToolbarModel::RemoveObserver(Observer* observer) {
+void ToolbarActionsModel::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void ExtensionToolbarModel::MoveExtensionIcon(const std::string& id,
-                                              size_t index) {
-  ExtensionList::iterator pos = toolbar_items_.begin();
-  while (pos != toolbar_items_.end() && (*pos)->id() != id)
+void ToolbarActionsModel::MoveActionIcon(const std::string& id, size_t index) {
+  ActionIdList::iterator pos = toolbar_items_.begin();
+  while (pos != toolbar_items_.end() && (*pos) != id)
     ++pos;
   if (pos == toolbar_items_.end()) {
     NOTREACHED();
     return;
   }
-  scoped_refptr<const Extension> extension = *pos;
+
   toolbar_items_.erase(pos);
 
-  ExtensionIdList::iterator pos_id = std::find(last_known_positions_.begin(),
-                                               last_known_positions_.end(),
-                                               id);
+  ActionIdList::iterator pos_id =
+      std::find(last_known_positions_.begin(),
+                last_known_positions_.end(),
+                id);
   if (pos_id != last_known_positions_.end())
     last_known_positions_.erase(pos_id);
 
   if (index < toolbar_items_.size()) {
     // If the index is not at the end, find the item currently at |index|, and
-    // insert |extension| before it in both |toolbar_items_| and
+    // insert the action with id |id| before it in both |toolbar_items_| and

[Devlin, 2015/08/06 22:04:56]

nitty nit: just |id|, not id |id|.

[apacible, 2015/08/10 22:38:10]

Done.

     // |last_known_positions_|.
-    ExtensionList::iterator iter = toolbar_items_.begin() + index;
+    ActionIdList::iterator iter = toolbar_items_.begin() + index;
     last_known_positions_.insert(std::find(last_known_positions_.begin(),
                                            last_known_positions_.end(),
-                                           (*iter)->id()),
+                                           *iter),
                                  id);
-    toolbar_items_.insert(iter, extension);
+    toolbar_items_.insert(iter, id);
   } else {
-    // Otherwise, put |extension| at the end.
+    // Otherwise, put the action at the end.
     DCHECK_EQ(toolbar_items_.size(), index);
     index = toolbar_items_.size();
-    toolbar_items_.push_back(extension);
+    toolbar_items_.push_back(id);
     last_known_positions_.push_back(id);
   }
 
   FOR_EACH_OBSERVER(Observer, observers_,
-                    OnToolbarExtensionMoved(extension.get(), index));
-  MaybeUpdateVisibilityPref(extension.get(), index);
+                    OnToolbarActionMoved(id, index));
+  MaybeUpdateVisibilityPref(id, index);
   UpdatePrefs();
 }
 
-void ExtensionToolbarModel::SetVisibleIconCount(size_t count) {
+void ToolbarActionsModel::SetVisibleIconCount(size_t count) {
   visible_icon_count_ = (count >= toolbar_items_.size()) ? -1 : count;
 
   // Only set the prefs if we're not in highlight mode and the profile is not
   // incognito. Highlight mode is designed to be a transitory state, and should
-  //  not persist across browser restarts (though it may be re-entered), and we
+  // not persist across browser restarts (though it may be re-entered), and we
   // don't store anything in incognito.
   if (!is_highlighting() && !profile_->IsOffTheRecord()) {
     // Additionally, if we are using the new toolbar, any icons which are in the
     // overflow menu are considered "hidden". But it so happens that the times
     // we are likely to call SetVisibleIconCount() are also those when we are
     // in flux. So wait for things to cool down before setting the prefs.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
-        base::Bind(&ExtensionToolbarModel::MaybeUpdateVisibilityPrefs,
+        base::Bind(&ToolbarActionsModel::MaybeUpdateVisibilityPrefs,
                    weak_ptr_factory_.GetWeakPtr()));
-    prefs_->SetInteger(pref_names::kToolbarSize, visible_icon_count_);
+    prefs_->SetInteger(extensions::pref_names::kToolbarSize,
+                       visible_icon_count_);
   }
 
   FOR_EACH_OBSERVER(Observer, observers_, OnToolbarVisibleCountChanged());
 }
 
-void ExtensionToolbarModel::OnExtensionActionUpdated(
+void ToolbarActionsModel::OnExtensionActionUpdated(
     ExtensionAction* extension_action,
     content::WebContents* web_contents,
     content::BrowserContext* browser_context) {
-  const Extension* extension =
-      ExtensionRegistry::Get(profile_)->enabled_extensions().GetByID(
-          extension_action->extension_id());
   // Notify observers if the extension exists and is in the model.
-  if (std::find(toolbar_items_.begin(), toolbar_items_.end(), extension) !=
-          toolbar_items_.end()) {
+  if (std::find(toolbar_items_.begin(), toolbar_items_.end(),
+      extension_action->extension_id()) != toolbar_items_.end()) {
     FOR_EACH_OBSERVER(Observer, observers_,
-                      OnToolbarExtensionUpdated(extension));
+                      OnToolbarActionUpdated(
+                          extension_action->extension_id()));
   }
 }
 
-void ExtensionToolbarModel::OnExtensionActionVisibilityChanged(
+ActionList ToolbarActionsModel::GetActions(Browser* browser,
+                                           ToolbarActionsBar* bar) {
+  DCHECK(browser);
+  DCHECK(bar);
+  ActionList action_list;
+
+  // Get the component action list.
+  ActionList component_actions =
+      ComponentToolbarActionsFactory::GetInstance()->
+          GetComponentToolbarActions(browser);
+
+  extensions::ExtensionActionManager* action_manager =
+      extensions::ExtensionActionManager::Get(profile_);
+
+  ActionIdList items = toolbar_items();
+  // Some actions are suppressed when others are highlighted. The list of ids
+  // is kept track of in |model_|.

[Devlin, 2015/08/06 22:04:56]

This is the model. ;)

[apacible, 2015/08/10 22:38:10]

Done.

+  for (size_t i = 0; i < items.size(); i++) {

[Devlin, 2015/08/06 22:04:56]

range-based for loop.

[apacible, 2015/08/10 22:38:10]

Done.

+    // Check if current action_id is for an extension.
+    if (IsKnownActionWithType(items[i], ACTION_EXTENSION)) {
+      // Get the extension.
+      const extensions::Extension* extension = GetExtensionById(items[i]);
+      DCHECK(extension);
+
+      // Create and add an ExtensionActionViewController for the extension.
+      action_list.push_back(new ExtensionActionViewController(
+          extension,
+          browser,
+          action_manager->GetExtensionAction(*extension),
+          bar));
+    } else if (IsKnownActionWithType(items[i], ACTION_COMPONENT)) {
+      DCHECK(extensions::FeatureSwitch::extension_action_redesign()->

[Devlin, 2015/08/06 22:04:56]

This is cached as a variable.

[apacible, 2015/08/10 22:38:09]

Done.

+             IsEnabled());
+      // Find the corresponding action to action_id.
+      for (size_t j = 0; j < component_actions.size(); j++) {

[Devlin, 2015/08/06 22:04:56]

range-based

[apacible, 2015/08/10 22:38:09]

Done.

+        if (component_actions[j]->GetId() == items[i]) {
+          action_list.push_back(component_actions[j]);
+          continue;

[Devlin, 2015/08/06 22:04:55]

this looks like it should be a break.

[apacible, 2015/08/10 22:38:10]

Done.

+        }
+      }
+    }
+  }
+
+  component_actions.weak_clear();

[Devlin, 2015/08/06 22:04:56]

have a comment about why this is safe (i.e., doesn't leak).

[apacible, 2015/08/10 22:38:10]

Added above in the loop that adds the component actions into action_list.

+
+  return action_list.Pass();
+}
+
+void ToolbarActionsModel::OnExtensionActionVisibilityChanged(
     const std::string& extension_id,
     bool is_now_visible) {
-  const Extension* extension =
-      ExtensionRegistry::Get(profile_)->GetExtensionById(
-          extension_id, ExtensionRegistry::EVERYTHING);
-
   // Hiding works differently with the new and old toolbars.
   if (include_all_extensions_) {
-    // It's possible that we haven't added this extension yet, if its
+    // It's possible that we haven't added this action yet, if its
     // visibility was adjusted in the course of its initialization.
-    if (std::find(toolbar_items_.begin(), toolbar_items_.end(), extension) ==
-            toolbar_items_.end())
+    if (std::find(toolbar_items_.begin(), toolbar_items_.end(),
+        extension_id) == toolbar_items_.end())
       return;
 
     int new_size = 0;
     int new_index = 0;
     if (is_now_visible) {
       // If this action used to be hidden, we can't possibly be showing all.
       DCHECK_LT(visible_icon_count(), toolbar_items_.size());
-      // Grow the bar by one and move the extension to the end of the visibles.
+      // Grow the bar by one and move the action to the end of the visibles.
       new_size = visible_icon_count() + 1;
       new_index = new_size - 1;
     } else {
       // If we're hiding one, we must be showing at least one.
       DCHECK_GE(visible_icon_count(), 0u);
-      // Shrink the bar by one and move the extension to the beginning of the
+      // Shrink the bar by one and move the action to the beginning of the
       // overflow menu.
       new_size = visible_icon_count() - 1;
       new_index = new_size;
     }
     SetVisibleIconCount(new_size);
-    MoveExtensionIcon(extension->id(), new_index);
+    MoveActionIcon(extension_id, new_index);
   } else {  // Don't include all extensions.
+    const extensions::Extension* extension =
+        extension_registry_->GetExtensionById(

[Devlin, 2015/08/06 22:04:56]

We have a method for this now, right?  (It shouldn't be possible for this to be
anything but an enabled extension, I think.)

[apacible, 2015/08/10 22:38:09]

Done.

+            extension_id, extensions::ExtensionRegistry::EVERYTHING);
     if (is_now_visible)
       AddExtension(extension);
     else
       RemoveExtension(extension);
   }
 }
 
-void ExtensionToolbarModel::OnExtensionLoaded(
+void ToolbarActionsModel::OnExtensionLoaded(
     content::BrowserContext* browser_context,
-    const Extension* extension) {
+    const extensions::Extension* extension) {
   // We don't want to add the same extension twice. It may have already been
   // added by EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED below, if the user
   // hides the browser action and then disables and enables the extension.
   for (size_t i = 0; i < toolbar_items_.size(); i++) {

[Devlin, 2015/08/06 22:04:56]

this can be std::find now.

[apacible, 2015/08/10 22:38:10]

Done.

-    if (toolbar_items_[i].get() == extension)
+    if (toolbar_items_[i] == extension->id())
       return;
   }
 
   AddExtension(extension);
 }
 
-void ExtensionToolbarModel::OnExtensionUnloaded(
+void ToolbarActionsModel::OnExtensionUnloaded(
     content::BrowserContext* browser_context,
-    const Extension* extension,
-    UnloadedExtensionInfo::Reason reason) {
+    const extensions::Extension* extension,
+    extensions::UnloadedExtensionInfo::Reason reason) {
   RemoveExtension(extension);
 }
 
-void ExtensionToolbarModel::OnExtensionUninstalled(
+void ToolbarActionsModel::OnExtensionUninstalled(
     content::BrowserContext* browser_context,
-    const Extension* extension,
+    const extensions::Extension* extension,
     extensions::UninstallReason reason) {
   // Remove the extension id from the ordered list, if it exists (the extension
   // might not be represented in the list because it might not have an icon).
-  ExtensionIdList::iterator pos =
+  ActionIdList::iterator pos =
       std::find(last_known_positions_.begin(),
                 last_known_positions_.end(), extension->id());
 
   if (pos != last_known_positions_.end()) {
     last_known_positions_.erase(pos);
     UpdatePrefs();
   }
 }
 
-void ExtensionToolbarModel::OnReady() {
-  ExtensionRegistry* registry = ExtensionRegistry::Get(profile_);
-  InitializeExtensionList();
+void ToolbarActionsModel::OnReady() {
+  InitializeActionList();
   // Wait until the extension system is ready before observing any further
   // changes so that the toolbar buttons can be shown in their stable ordering
   // taken from prefs.
-  extension_registry_observer_.Add(registry);
+  extension_registry_observer_.Add(extension_registry_);
   extension_action_observer_.Add(extension_action_api_);
 
   if (ExtensionToolbarIconSurfacingBubbleDelegate::ShouldShowForProfile(
-          profile_)) {
-    ExtensionIdList ids;
-    for (const auto& extension : toolbar_items_)
-      ids.push_back(extension->id());
-    HighlightExtensions(ids, HIGHLIGHT_INFO);
+      profile_)) {
+    ActionIdList ids;
+    for (const std::string& id : toolbar_items_)
+      ids.push_back(id);
+    HighlightActions(ids, HIGHLIGHT_INFO);
   }
 
-  extensions_initialized_ = true;
+  actions_initialized_ = true;
   FOR_EACH_OBSERVER(Observer, observers_, OnToolbarModelInitialized());
 }
 
-size_t ExtensionToolbarModel::FindNewPositionFromLastKnownGood(
-    const Extension* extension) {
+size_t ToolbarActionsModel::FindNewPositionFromLastKnownGood(
+    const std::string& id) {
   // See if we have last known good position for this extension.
   size_t new_index = 0;
   // Loop through the ID list of known positions, to count the number of visible
   // extension icons preceding |extension|.
-  for (ExtensionIdList::const_iterator iter_id = last_known_positions_.begin();
-       iter_id < last_known_positions_.end(); ++iter_id) {
-    if ((*iter_id) == extension->id())
+  for (ActionIdList::const_iterator iter_id =

[Devlin, 2015/08/06 22:04:56]

range-based.

[apacible, 2015/08/10 22:38:10]

Done.

+       last_known_positions_.begin(); iter_id < last_known_positions_.end();
+       ++iter_id) {
+    if ((*iter_id) == id)
       return new_index;  // We've found the right position.
     // Found an id, need to see if it is visible.
-    for (ExtensionList::const_iterator iter_ext = toolbar_items_.begin();
-         iter_ext < toolbar_items_.end(); ++iter_ext) {
-      if ((*iter_ext)->id() == (*iter_id)) {
+    for (ActionIdList::const_iterator iter_ext =
+         toolbar_items_.begin(); iter_ext < toolbar_items_.end(); ++iter_ext) {
+      if ((*iter_ext) == (*iter_id)) {
         // This extension is visible, update the index value.
         ++new_index;
         break;
       }
     }
   }
 
   // Position not found.
   return toolbar_items_.size();
 }
 
-bool ExtensionToolbarModel::ShouldAddExtension(const Extension* extension) {
+bool ToolbarActionsModel::ShouldAddExtension(
+    const extensions::Extension* extension) {
   // In incognito mode, don't add any extensions that aren't incognito-enabled.
   if (profile_->IsOffTheRecord() &&
-      !util::IsIncognitoEnabled(extension->id(), profile_))
+      !extensions::util::IsIncognitoEnabled(extension->id(), profile_))
     return false;
 
-  ExtensionActionManager* action_manager =
-      ExtensionActionManager::Get(profile_);
+  extensions::ExtensionActionManager* action_manager =
+      extensions::ExtensionActionManager::Get(profile_);
   if (include_all_extensions_) {
     // In this case, we don't care about the browser action visibility, because
     // we want to show each extension regardless.
-    // TODO(devlin): Extension actions which are not visible should be moved to
-    // the overflow menu by default.
     return action_manager->GetExtensionAction(*extension) != NULL;
   }
 
   return action_manager->GetBrowserAction(*extension) &&
          extension_action_api_->GetBrowserActionVisibility(extension->id());
 }
 
-void ExtensionToolbarModel::AddExtension(const Extension* extension) {
+void ToolbarActionsModel::AddExtension(const extensions::Extension* extension) {
   // We only use AddExtension() once the system is initialized.
-  DCHECK(extensions_initialized_);
+  DCHECK(actions_initialized_);
   if (!ShouldAddExtension(extension))
     return;
 
   // See if we have a last known good position for this extension.
   bool is_new_extension =
       std::find(last_known_positions_.begin(),
                 last_known_positions_.end(),
                 extension->id()) == last_known_positions_.end();
 
   // New extensions go at the right (end) of the visible extensions. Other
   // extensions go at their previous position.
   size_t new_index = 0;
   if (is_new_extension) {
-    new_index = Manifest::IsComponentLocation(extension->location()) ?
-        0 : visible_icon_count();
+    new_index = extensions::Manifest::IsComponentLocation(
+        extension->location()) ? 0 : visible_icon_count();
     // For the last-known position, we use the index of the extension that is
     // just before this extension, plus one. (Note that this isn't the same
     // as new_index + 1, because last_known_positions_ can include disabled
     // extensions.)
     int new_last_known_index =
         new_index == 0 ? 0 :
         std::find(last_known_positions_.begin(),
                   last_known_positions_.end(),
-                  toolbar_items_[new_index - 1]->id()) -
+                  toolbar_items_[new_index - 1]) -
             last_known_positions_.begin() + 1;
     // In theory, the extension before this one should always
     // be in last known positions, but if something funny happened with prefs,
     // make sure we handle it.
     // TODO(devlin): Track down these cases so we can CHECK this.
     new_last_known_index =
         std::min<int>(new_last_known_index, last_known_positions_.size());
     last_known_positions_.insert(
         last_known_positions_.begin() + new_last_known_index, extension->id());
     UpdatePrefs();
   } else {
-    new_index = FindNewPositionFromLastKnownGood(extension);
+    new_index = FindNewPositionFromLastKnownGood(extension->id());
   }
 
-  toolbar_items_.insert(toolbar_items_.begin() + new_index, extension);
+  action_id_to_type_.insert(
+      std::pair<std::string, ActionType>(extension->id(), ACTION_EXTENSION));
+  toolbar_items_.insert(toolbar_items_.begin() + new_index, extension->id());
 
   // If we're currently highlighting, then even though we add a browser action
   // to the full list (|toolbar_items_|, there won't be another *visible*
   // browser action, which was what the observers care about.
   if (!is_highlighting()) {
     FOR_EACH_OBSERVER(Observer, observers_,
-                      OnToolbarExtensionAdded(extension, new_index));
+                      OnToolbarActionAdded(extension->id(), new_index));
 
     int visible_count_delta = 0;
     if (is_new_extension && !all_icons_visible()) {
       // If this is a new extension (and not all extensions are visible), we
       // expand the toolbar out so that the new one can be seen.
       visible_count_delta = 1;
     } else if (profile_->IsOffTheRecord()) {
       // If this is an incognito profile, we also have to check to make sure the
       // overflow matches the main bar's status.
-      ExtensionToolbarModel* main_model =
-          ExtensionToolbarModel::Get(profile_->GetOriginalProfile());
+      ToolbarActionsModel* main_model =
+          ToolbarActionsModel::Get(profile_->GetOriginalProfile());
       // Find what the index will be in the main bar. Because Observer calls are
       // nondeterministic, we can't just assume the main bar will have the
       // extension and look it up.
       size_t main_index =
-          main_model->FindNewPositionFromLastKnownGood(extension);
+          main_model->FindNewPositionFromLastKnownGood(extension->id());
       bool visible = main_index < main_model->visible_icon_count();
       // We may need to adjust the visible count if the incognito bar isn't
       // showing all icons and this one is visible, or if it is showing all
       // icons and this is hidden.
       if (visible && !all_icons_visible())
         visible_count_delta = 1;
       else if (!visible && all_icons_visible())
         visible_count_delta = -1;
     }
 
     if (visible_count_delta)
       SetVisibleIconCount(visible_icon_count() + visible_count_delta);
   }
 
-  MaybeUpdateVisibilityPref(extension, new_index);
+  MaybeUpdateVisibilityPref(extension->id(), new_index);
 }
 
-void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
-  ExtensionList::iterator pos =
-      std::find(toolbar_items_.begin(), toolbar_items_.end(), extension);
+void ToolbarActionsModel::RemoveExtension(
+    const extensions::Extension* extension) {
+  ActionIdList::iterator pos =
+      std::find(toolbar_items_.begin(), toolbar_items_.end(),
+          extension->id());
   if (pos == toolbar_items_.end())
     return;
 
   // If our visible count is set to the current size, we need to decrement it.
   if (visible_icon_count_ == static_cast<int>(toolbar_items_.size()))
     SetVisibleIconCount(toolbar_items_.size() - 1);
 
   toolbar_items_.erase(pos);
 
   // If we're in highlight mode, we also have to remove the extension from
   // the highlighted list.
   if (is_highlighting()) {
     pos = std::find(highlighted_items_.begin(),
                     highlighted_items_.end(),
-                    extension);
+                    extension->id());
     if (pos != highlighted_items_.end()) {
       highlighted_items_.erase(pos);
       FOR_EACH_OBSERVER(Observer, observers_,
-                        OnToolbarExtensionRemoved(extension));
+                        OnToolbarActionRemoved(extension->id()));
       // If the highlighted list is now empty, we stop highlighting.
       if (highlighted_items_.empty())
         StopHighlighting();
     }
   } else {
     FOR_EACH_OBSERVER(Observer, observers_,
-                      OnToolbarExtensionRemoved(extension));
+                      OnToolbarActionRemoved(extension->id()));
   }
 
   UpdatePrefs();
 }
 
 // Combine the currently enabled extensions that have browser actions (which
-// we get from the ExtensionRegistry) with the ordering we get from the
-// pref service. For robustness we use a somewhat inefficient process:
-// 1. Create a vector of extensions sorted by their pref values. This vector may
+// we get from the ExtensionRegistry) and component actions (which we get from
+// ComponentToolbarActionsFactory ) with the ordering we get from the pref
+// service. For robustness we use a somewhat inefficient process:
+// 1. Create a vector of actions sorted by their pref values. This vector may
 // have holes.
-// 2. Create a vector of extensions that did not have a pref value.
+// 2. Create a vector of actions that did not have a pref value.
 // 3. Remove holes from the sorted vector and append the unsorted vector.
-void ExtensionToolbarModel::InitializeExtensionList() {
+void ToolbarActionsModel::InitializeActionList() {
   DCHECK(toolbar_items_.empty());  // We shouldn't have any items yet.
+  DCHECK(action_id_to_type_.empty());
 
-  last_known_positions_ = extension_prefs_->GetToolbarOrder();
+  last_known_positions_ = prefs_service_->GetToolbarOrder();
   if (profile_->IsOffTheRecord())
     IncognitoPopulate();
   else
     Populate(&last_known_positions_);
 
   MaybeUpdateVisibilityPrefs();
 }
 
-void ExtensionToolbarModel::Populate(ExtensionIdList* positions) {
+void ToolbarActionsModel::Populate(ActionIdList* positions) {
   DCHECK(!profile_->IsOffTheRecord());
-  const ExtensionSet& extensions =
-      ExtensionRegistry::Get(profile_)->enabled_extensions();
+
   // Items that have explicit positions.
-  ExtensionList sorted(positions->size(), NULL);
+  ActionIdList sorted(positions->size());
   // The items that don't have explicit positions.
-  ExtensionList unsorted;
+  ActionIdList unsorted;
 
-  // Create the lists.
+  // Populate the lists.
   int hidden = 0;
-  for (const scoped_refptr<const Extension>& extension : extensions) {
+  int browser_actions_count = 0;
+  int component_actions_count = 0;
+
+  // First, add the extension action ids to the lists.
+  const extensions::ExtensionSet& extensions =
+      extension_registry_->enabled_extensions();
+  for (const scoped_refptr<const extensions::Extension>& extension :
+       extensions) {
     if (!ShouldAddExtension(extension.get())) {
       if (!extension_action_api_->GetBrowserActionVisibility(extension->id()))
         ++hidden;
       continue;
     }
 
-    ExtensionIdList::const_iterator pos =
+    ActionIdList::const_iterator pos =
         std::find(positions->begin(), positions->end(), extension->id());
     if (pos != positions->end()) {
-      sorted[pos - positions->begin()] = extension;
+      sorted[pos - positions->begin()] = extension->id();
     } else {
       // Unknown extension - push it to the back of unsorted, and add it to the
       // list of ids at the end.
-      unsorted.push_back(extension);
+      unsorted.push_back(extension->id());
       positions->push_back(extension->id());
     }
   }
 
+  // Next, add the component action ids to the lists.
+  ActionIdList component_ids =
+      ComponentToolbarActionsFactory::GetComponentIds();
+  for (const std::string& id : component_ids) {
+    ActionIdList::const_iterator pos =
+        std::find(positions->begin(), positions->end(), id);
+    if (pos != positions->end()) {

[Devlin, 2015/08/06 22:04:56]

Instead of duplicating this logic, can we just generate a set of all ids
(extension and component), and then iterate over it to sort it?

[apacible, 2015/08/10 22:38:10]

Done.

+      sorted[pos - positions->begin()] = id;
+    } else {
+      // New component - push it to the back of unsorted, and add it to the
+      // list of ids at the end. This should only be the case when there is
+      // a browser feature using a new component action.
+      unsorted.push_back(id);
+      positions->push_back(id);
+    }
+  }
+
   // Merge the lists.
   sorted.insert(sorted.end(), unsorted.begin(), unsorted.end());
   toolbar_items_.reserve(sorted.size());
 
-  for (const scoped_refptr<const Extension>& extension : sorted) {
-    // It's possible for the extension order to contain items that aren't
-    // actually loaded on this machine.  For example, when extension sync is on,
-    // we sync the extension order as-is but double-check with the user before
-    // syncing NPAPI-containing extensions, so if one of those is not actually
-    // synced, we'll get a NULL in the list.  This sort of case can also happen
-    // if some error prevents an extension from loading.
-    if (extension.get()) {
-      // We don't notify observers of the added extension yet. Rather, observers
-      // should wait for the "OnToolbarModelInitialized" notification, and then
-      // bulk-update. (This saves a lot of bouncing-back-and-forth here, and
-      // allows observers to ensure that the extension system is always
-      // initialized before using the extensions).
-      toolbar_items_.push_back(extension);
+  for (const std::string& action_id : sorted) {
+    // Check the type of action.
+    if (std::find(component_ids.begin(), component_ids.end(), action_id) ==

[Devlin, 2015/08/06 22:04:55]

These lookups still make me wonder if we should have a struct of string, type. 
Was there a reason not to do that?

[apacible, 2015/08/10 22:38:10]

I add to the struct of string, type after this check, right before pushing the
id into toolbar_items_. See L572, L579. This is the only time we do this
particular lookup.

[Devlin, 2015/08/13 21:21:10]

That's not quite what I was suggesting.  I was wondering if we should have
toolbar_items be a std::vector<ToolbarAction>, where ToolbarItem is a struct {
std::string id, ActionType type}.  Then, this loop becomes
for (item : sorted) {
  if (item.type == EXTENSION)
    ...
  else
    ...
}

This would also simplify other methods like RedesignIsShowingNewIcons and
CreateActions, as well as being more performant (no lookup).  And it gets rid of
some cruft in the header file, which is really nice.

WDYT?

[apacible, 2015/08/15 08:46:40]

Talked offline:
- Created ToolbarItem struct.
- Pass around ToolbarItem rather than ID (string) in private functions; use ID
(string) in public functions.

This definitely helps simplify things in general.

+        component_ids.end()) {
+      // It's possible for the extension order to contain items that aren't
+      // actually loaded on this machine.  For example, when extension sync is
+      // on, we sync the extension order as-is but double-check with the user
+      // before syncing NPAPI-containing extensions, so if one of those is not
+      // actually synced, we'll get a NULL in the list.  This sort of case can
+      // also happen if some error prevents an extension from loading.
+      if (GetExtensionById(action_id)) {
+        // We don't notify observers of the added extension yet. Rather,

[Devlin, 2015/08/06 22:04:55]

This comment applies to both component and extension actions.  Probably just put
it above the for loop.

[apacible, 2015/08/10 22:38:09]

Done.

+        // observers should wait for the "OnToolbarModelInitialized"
+        // notification, and then bulk-update. (This saves a lot of
+        // bouncing-back-and-forth here, and allows observers to ensure that
+        // the extension system is always initialized before using the
+        // extensions).
+        action_id_to_type_.insert(
+            std::pair<std::string, ActionType>(action_id, ACTION_EXTENSION));
+        toolbar_items_.push_back(action_id);
+        browser_actions_count++;

[Devlin, 2015/08/06 22:04:56]

nitty nit: prefer pre-increment.

[apacible, 2015/08/10 22:38:10]

Done.

+      }
+    } else {
+      // Always add component actions.
+      action_id_to_type_.insert(
+          std::pair<std::string, ActionType>(action_id, ACTION_COMPONENT));
+      toolbar_items_.push_back(action_id);
+      component_actions_count++;
     }
   }
 
+  // Histogram names are prefixed with "ExtensionToolbarModel" rather than
+  // "ToolbarActionsModel" for historical reasons.
   UMA_HISTOGRAM_COUNTS_100(
       "ExtensionToolbarModel.BrowserActionsPermanentlyHidden", hidden);
   UMA_HISTOGRAM_COUNTS_100("ExtensionToolbarModel.BrowserActionsCount",
+                           browser_actions_count);
+  UMA_HISTOGRAM_COUNTS_100("ToolbarActionsModel.ComponentActionsCount",
+                           component_actions_count);
+  UMA_HISTOGRAM_COUNTS_100("ToolbarActionsModel.OverallActionsCount",
                            toolbar_items_.size());
 
   if (!toolbar_items_.empty()) {
     // Visible count can be -1, meaning: 'show all'. Since UMA converts negative
     // values to 0, this would be counted as 'show none' unless we convert it to
     // max.
+    int visible_browser_icon_size = visible_icon_count_ -
+        component_actions_count;
     UMA_HISTOGRAM_COUNTS_100("ExtensionToolbarModel.BrowserActionsVisible",
                              visible_icon_count_ == -1 ?
                                  base::HistogramBase::kSampleType_MAX :
-                                 visible_icon_count_);
+                                 visible_browser_icon_size);

[Devlin, 2015/08/06 22:04:56]

UMA for visible toolbar actions would be good, too.  (When everything launches
and stabilizes, we might get rid of the plain-old browser action one.)

[apacible, 2015/08/10 22:38:09]

Done.

   }
 }
 
-void ExtensionToolbarModel::IncognitoPopulate() {
+void ToolbarActionsModel::IncognitoPopulate() {
   DCHECK(profile_->IsOffTheRecord());
-  const ExtensionToolbarModel* original_model =
-      ExtensionToolbarModel::Get(profile_->GetOriginalProfile());
+  const ToolbarActionsModel* original_model =
+      ToolbarActionsModel::Get(profile_->GetOriginalProfile());
 
   // Find the absolute value of the original model's count.
   int original_visible = original_model->visible_icon_count();
 
-  // In incognito mode, we show only those extensions that are
-  // incognito-enabled. Further, any actions that were overflowed in regular
-  // mode are still overflowed. Order is the same as in regular mode.
+  std::map<std::string, ActionType> original_action_id_to_type =
+      original_model->action_id_to_type();
+
+  // In incognito mode, we show only those actions that are incognito-enabled
+  // Further, any actions that were overflowed in regular mode are still
+  // overflowed. Order is the same as in regular mode.
   visible_icon_count_ = 0;
-  for (ExtensionList::const_iterator iter =
+
+  for (ActionIdList::const_iterator iter =
            original_model->toolbar_items_.begin();
        iter != original_model->toolbar_items_.end(); ++iter) {
-    if (ShouldAddExtension(iter->get())) {
+    if (original_action_id_to_type.count(*iter) == 0)
+      continue;
+
+    if (original_action_id_to_type.find(*iter)->second == ACTION_EXTENSION) {
+      if (ShouldAddExtension(GetExtensionById(*iter))) {
+        toolbar_items_.push_back(*iter);
+        if (iter - original_model->toolbar_items_.begin() < original_visible)
+          ++visible_icon_count_;
+      }
+    } else {
+      // Always show component actions.

[Devlin, 2015/08/06 22:04:56]

Why not respect the user's choice to hide them?  I might not want the cast icon
always appearing in an incognito window.

[apacible, 2015/08/10 22:38:10]

Good point. How would I go about finding how to hide/show an action? It
currently doesn't seem like there's a way to set/get that decision for component
actions.

[Devlin, 2015/08/13 21:21:10]

The decision is set by whether or not it's visible on the main bar.  Visibility
is indicated by its index being less than the visible icon count.  So you can
just check the main bar, just as we do for extensions.

[apacible, 2015/08/15 08:46:40]

Talked offline; updated comment.

As a start, we'll always have component actions be present. Visibility is
determined by the main bar.

       toolbar_items_.push_back(*iter);
       if (iter - original_model->toolbar_items_.begin() < original_visible)
         ++visible_icon_count_;
     }
   }
 }
 
-void ExtensionToolbarModel::UpdatePrefs() {
-  if (!extension_prefs_ || profile_->IsOffTheRecord())
+void ToolbarActionsModel::UpdatePrefs() {
+  if (!prefs_service_ || profile_->IsOffTheRecord())
     return;
 
   // Don't observe change caused by self.
-  pref_change_registrar_.Remove(pref_names::kToolbar);
-  extension_prefs_->SetToolbarOrder(last_known_positions_);
-  pref_change_registrar_.Add(pref_names::kToolbar, pref_change_callback_);
+  pref_change_registrar_.Remove(extensions::pref_names::kToolbar);
+  prefs_service_->SetToolbarOrder(last_known_positions_);
+  pref_change_registrar_.Add(extensions::pref_names::kToolbar,
+                             pref_change_callback_);
 }
 
-void ExtensionToolbarModel::MaybeUpdateVisibilityPref(
-    const Extension* extension, size_t index) {
+void ToolbarActionsModel::MaybeUpdateVisibilityPref(
+    const std::string& id, size_t index) {
+  // Component Actions are always shown; visibility should not change.

[Devlin, 2015/08/06 22:04:56]

This isn't necessarily true.  That said, this pref has a lot wrong with it (and
is really just there for historical purposes).  I rephrase this to just say
"Component actions don't have prefs to update."

[apacible, 2015/08/10 22:38:10]

Done.

+  if (IsKnownActionWithType(id, ACTION_COMPONENT))
+    return;
+
   // We only update the visibility pref for hidden/not hidden based on the
   // overflow menu with the new toolbar design.
   if (include_all_extensions_ && !profile_->IsOffTheRecord()) {
     bool visible = index < visible_icon_count();
-    if (visible != extension_action_api_->GetBrowserActionVisibility(
-                       extension->id())) {
+    if (visible != extension_action_api_->GetBrowserActionVisibility(id)) {
       // Don't observe changes caused by ourselves.
       bool was_registered = false;
       if (extension_action_observer_.IsObserving(extension_action_api_)) {
         was_registered = true;
         extension_action_observer_.RemoveAll();
       }
-      extension_action_api_->SetBrowserActionVisibility(extension->id(),
-                                                       visible);
+      extension_action_api_->SetBrowserActionVisibility(id, visible);
       if (was_registered)
         extension_action_observer_.Add(extension_action_api_);
     }
   }
 }
 
-void ExtensionToolbarModel::MaybeUpdateVisibilityPrefs() {
+void ToolbarActionsModel::MaybeUpdateVisibilityPrefs() {
   for (size_t i = 0u; i < toolbar_items_.size(); ++i)
-    MaybeUpdateVisibilityPref(toolbar_items_[i].get(), i);
+    MaybeUpdateVisibilityPref(toolbar_items_[i], i);
 }
 
-void ExtensionToolbarModel::OnExtensionToolbarPrefChange() {
+void ToolbarActionsModel::OnActionToolbarPrefChange() {
   // If extensions are not ready, defer to later Populate() call.
-  if (!extensions_initialized_)
+  if (!actions_initialized_)
     return;
 
   // Recalculate |last_known_positions_| to be |pref_positions| followed by
   // ones that are only in |last_known_positions_|.
-  ExtensionIdList pref_positions = extension_prefs_->GetToolbarOrder();
+  ActionIdList pref_positions = prefs_service_->GetToolbarOrder();
   size_t pref_position_size = pref_positions.size();
   for (size_t i = 0; i < last_known_positions_.size(); ++i) {
     if (std::find(pref_positions.begin(), pref_positions.end(),
                   last_known_positions_[i]) == pref_positions.end()) {
       pref_positions.push_back(last_known_positions_[i]);
     }
   }
   last_known_positions_.swap(pref_positions);
 
   int desired_index = 0;
   // Loop over the updated list of last known positions, moving any extensions
   // that are in the wrong place.
   for (const std::string& id : last_known_positions_) {
     int current_index = GetIndexForId(id);
     if (current_index == -1)
       continue;
     if (current_index != desired_index) {
-      scoped_refptr<const Extension> extension = toolbar_items_[current_index];
+      std::string extension_id = toolbar_items_[current_index];
       toolbar_items_.erase(toolbar_items_.begin() + current_index);
-      toolbar_items_.insert(toolbar_items_.begin() + desired_index, extension);
+      toolbar_items_.insert(toolbar_items_.begin() + desired_index,
+          extension_id);
       // Notify the observers to keep them up-to-date.
       FOR_EACH_OBSERVER(
           Observer, observers_,
-          OnToolbarExtensionMoved(extension.get(), desired_index));
+          OnToolbarActionMoved(extension_id, desired_index));
     }
     ++desired_index;
   }
 
   if (last_known_positions_.size() > pref_position_size) {
     // Need to update pref because we have extra icons. But can't call
     // UpdatePrefs() directly within observation closure.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&ExtensionToolbarModel::UpdatePrefs,
+        FROM_HERE, base::Bind(&ToolbarActionsModel::UpdatePrefs,
                               weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
-int ExtensionToolbarModel::GetIndexForId(const std::string& id) const {
-  for (size_t i = 0; i < toolbar_items().size(); ++i) {
-    if (toolbar_items()[i]->id() == id)
+int ToolbarActionsModel::GetIndexForId(const std::string& id) const {
+  for (size_t i = 0; i < toolbar_items_.size(); ++i) {

[Devlin, 2015/08/06 22:04:56]

Again, toolbar_items() can potentially return different a different set than
toolbar_items_. Should probably stick with toolbar_items().

[apacible, 2015/08/10 22:38:10]

Done.

+    if (toolbar_items_[i] == id)
       return i;
   }
   return -1;
 }
 
-bool ExtensionToolbarModel::ShowExtensionActionPopup(
-    const Extension* extension,
-    Browser* browser,
-    bool grant_active_tab) {
+bool ToolbarActionsModel::ShowToolbarActionPopup(const std::string& id,
+                                                 Browser* browser,
+                                                 bool grant_active_tab) {
   base::ObserverListBase<Observer>::Iterator it(&observers_);
   Observer* obs = NULL;
   // Look for the Observer associated with the browser.
   // This would be cleaner if we had an abstract class for the Toolbar UI
   // (like we do for LocationBar), but sadly, we don't.
   while ((obs = it.GetNext()) != NULL) {
     if (obs->GetBrowser() == browser)
-      return obs->ShowExtensionActionPopup(extension, grant_active_tab);
+      return obs->ShowToolbarActionPopup(id, grant_active_tab);
   }
   return false;
 }
 
-void ExtensionToolbarModel::EnsureVisibility(
-    const ExtensionIdList& extension_ids) {
+void ToolbarActionsModel::EnsureVisibility(const ActionIdList& ids) {
   if (all_icons_visible())
     return;  // Already showing all.
 
   // Otherwise, make sure we have enough room to show all the extensions
   // requested.
-  if (visible_icon_count() < extension_ids.size())
-    SetVisibleIconCount(extension_ids.size());
+  if (visible_icon_count() < ids.size())
+    SetVisibleIconCount(ids.size());
 
   if (all_icons_visible())
     return;  // May have been set to max by SetVisibleIconCount.
 
   // Guillotine's Delight: Move an orange noble to the front of the line.
-  for (ExtensionIdList::const_iterator it = extension_ids.begin();
-       it != extension_ids.end(); ++it) {
-    for (ExtensionList::const_iterator extension = toolbar_items_.begin();
-         extension != toolbar_items_.end(); ++extension) {
-      if ((*extension)->id() == (*it)) {
-        if (extension - toolbar_items_.begin() >=
-                static_cast<int>(visible_icon_count()))
-          MoveExtensionIcon((*extension)->id(), 0);
+  for (ActionIdList::const_iterator it = ids.begin();
+       it != ids.end(); ++it) {
+    for (ActionIdList::const_iterator id = toolbar_items_.begin();
+          id != toolbar_items_.end(); ++id) {
+      if ((*id) == (*it)) {
+        if (id - toolbar_items_.begin() >=
+            static_cast<int>(visible_icon_count()))
+          MoveActionIcon((*id), 0);
         break;
       }
     }
   }
 }
 
-bool ExtensionToolbarModel::HighlightExtensions(
-    const ExtensionIdList& extension_ids,
-    HighlightType highlight_type) {
+bool ToolbarActionsModel::HighlightActions(const ActionIdList& ids,
+                                           HighlightType highlight_type) {
   highlighted_items_.clear();
 
-  for (ExtensionIdList::const_iterator id = extension_ids.begin();
-       id != extension_ids.end();
-       ++id) {
-    for (ExtensionList::const_iterator extension = toolbar_items_.begin();
-         extension != toolbar_items_.end();
-         ++extension) {
-      if (*id == (*extension)->id())
-        highlighted_items_.push_back(*extension);
+  for (std::string action_id : ids) {
+    for (std::string toolbar_item_id : toolbar_items_) {
+      if (action_id == toolbar_item_id)
+        highlighted_items_.push_back(toolbar_item_id);
     }
   }
 
   // If we have any items in |highlighted_items_|, then we entered highlighting
   // mode.
   if (highlighted_items_.size()) {
     // It's important that is_highlighting_ is changed immediately before the
     // observers are notified since it changes the result of toolbar_items().
     highlight_type_ = highlight_type;
     FOR_EACH_OBSERVER(Observer, observers_,
                       OnToolbarHighlightModeChanged(true));
 
     // We set the visible icon count after the highlight mode change because
     // the UI actions are created/destroyed during highlight, and doing that
     // prior to changing the size allows us to still have smooth animations.
-    if (visible_icon_count() < extension_ids.size())
-      SetVisibleIconCount(extension_ids.size());
+    if (visible_icon_count() < ids.size())
+      SetVisibleIconCount(ids.size());
 
     return true;
   }
 
   // Otherwise, we didn't enter highlighting mode (and, in fact, exited it if
   // we were otherwise in it).
   if (is_highlighting())
     StopHighlighting();
   return false;
 }
 
-void ExtensionToolbarModel::StopHighlighting() {
+void ToolbarActionsModel::StopHighlighting() {
   if (is_highlighting()) {
     // It's important that is_highlighting_ is changed immediately before the
     // observers are notified since it changes the result of toolbar_items().
     highlight_type_ = HIGHLIGHT_NONE;
     FOR_EACH_OBSERVER(Observer, observers_,
                       OnToolbarHighlightModeChanged(false));
 
     // For the same reason, we don't clear highlighted_items_ until after the
     // mode changed.
     highlighted_items_.clear();
 
     // We set the visible icon count after the highlight mode change because
     // the UI actions are created/destroyed during highlight, and doing that
     // prior to changing the size allows us to still have smooth animations.
-    int saved_icon_count = prefs_->GetInteger(pref_names::kToolbarSize);
+    int saved_icon_count = prefs_->GetInteger(
+        extensions::pref_names::kToolbarSize);
     if (saved_icon_count != visible_icon_count_)
       SetVisibleIconCount(saved_icon_count);
   }
 }
 
-bool ExtensionToolbarModel::RedesignIsShowingNewIcons() const {
-  for (const scoped_refptr<const Extension>& extension : toolbar_items_) {
-    // Without the redesign, we only show extensions with browser actions.
-    // Any extension without a browser action is an indication that we're
-    // showing something new.
-    if (!extension->manifest()->HasKey(manifest_keys::kBrowserAction))
-      return true;
+bool ToolbarActionsModel::RedesignIsShowingNewIcons() const {
+  for (std::string id : toolbar_items_) {

[Devlin, 2015/08/06 22:04:55]

const &

[apacible, 2015/08/10 22:38:10]

Done.

+    if (IsKnownActionWithType(id, ACTION_EXTENSION)) {
+      // Without the redesign, we only show extensions with browser actions.
+      // Any extension without a browser action is an indication that we're
+      // showing something new.
+      if (!GetExtensionById(id)->manifest()->HasKey(
+          extensions::manifest_keys::kBrowserAction))
+        return true;
+    }
   }
   return false;
 }
 
-}  // namespace extensions
+bool ToolbarActionsModel::IsKnownActionWithType(const std::string& id,
+    ActionType type) const {
+  return action_id_to_type_.count(id) > 0 &&
+      action_id_to_type_.find(id)->second == type;
+}
+
+const extensions::Extension* ToolbarActionsModel::GetExtensionById(
+    const std::string& id) const {
+  return extension_registry_->enabled_extensions().GetByID(id);
+}