Show media router toolbar icon ephemerally for active local routes and issues

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/ui/toolbar/toolbar_actions_model.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/location.h"
@@ skipping 258 matching lines @@
   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(extension_registry_);
   extension_action_observer_.Add(extension_action_api_);
 
   actions_initialized_ = true;
   FOR_EACH_OBSERVER(Observer, observers_, OnToolbarModelInitialized());
 
-  // Handle component action migrations.  We must make sure that observers are
-  // notified of initialization first, so that the associated widgets are
-  // created.
-  ComponentToolbarActionsFactory::GetInstance()->HandleComponentMigrations(
-      component_migration_helper_.get(), profile_);
+  if (use_redesign_) {

[takumif, 2016/09/20 18:56:34]

There were some browsertest failures due to DCHECK(use_redesign_) failing in
ToolbarActionsModel. This is caused by MRActionController calling
ToolbarActionsModel::HasComponentAction() as a result of a pref change by
ComponentToolbarActionsFactory::HandleComponentMigrations().

I think putting this here is better than setting
extensions::FeatureSwitch::extension_action_redesign() in all the failing tests.

[Devlin, 2016/09/20 21:55:48]

Overall, this seems reasonable, but which tests were failing? 
extension_action_redesign should default to true, unless the test is overriding
it...

[takumif, 2016/09/20 23:38:11]

This shows the tests that were failing. They seem to be legacy tests:

https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel...

+    // Handle component action migrations.  We must make sure that observers are
+    // notified of initialization first, so that the associated widgets are
+    // created.
+    ComponentToolbarActionsFactory::GetInstance()->HandleComponentMigrations(
+        component_migration_helper_.get(), profile_);
+  }
 }
 
 size_t ToolbarActionsModel::FindNewPositionFromLastKnownGood(
     const ToolbarItem& action) {
   // See if we have last known good position for this action.
   size_t new_index = 0;
   // Loop through the ID list of known positions, to count the number of
   // visible action icons preceding |action|'s id.
   for (const std::string& last_pos_id : last_known_positions_) {
     if (last_pos_id == action.id)
@@ skipping 26 matching lines @@
   }
 
   return extension_action_manager_->GetBrowserAction(*extension) &&
          extension_action_api_->GetBrowserActionVisibility(extension->id());
 }
 
 void ToolbarActionsModel::AddExtension(const extensions::Extension* extension) {
   if (!ShouldAddExtension(extension))
     return;
 
-  AddItem(ToolbarItem(extension->id(), EXTENSION_ACTION),
-          extensions::Manifest::IsComponentLocation(extension->location()));
+  AddItem(ToolbarItem(extension->id(), EXTENSION_ACTION));
 }
 
-void ToolbarActionsModel::AddItem(const ToolbarItem& item, bool is_component) {
+void ToolbarActionsModel::AddItem(const ToolbarItem& item) {
   // We only use AddItem() once the system is initialized.
   DCHECK(actions_initialized_);
 
   // See if we have a last known good position for this extension.
   bool is_new_extension = !base::ContainsValue(last_known_positions_, item.id);
 
   // 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 = is_component ? 0 : visible_icon_count();
+    new_index = 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) -
                                  last_known_positions_.begin() + 1;
     // In theory, the extension before this one should always
@@ skipping 269 matching lines @@
 bool ToolbarActionsModel::HasComponentAction(
     const std::string& action_id) const {
   DCHECK(use_redesign_);
   return HasItem(ToolbarItem(action_id, COMPONENT_ACTION));
 }
 
 void ToolbarActionsModel::AddComponentAction(const std::string& action_id) {
   DCHECK(use_redesign_);
   ToolbarItem component_item(action_id, COMPONENT_ACTION);
   DCHECK(!HasItem(component_item));
-  AddItem(component_item, true);
+  AddItem(component_item);
 }
 
 void ToolbarActionsModel::RemoveComponentAction(const std::string& action_id) {
   DCHECK(use_redesign_);
   ToolbarItem component_item(action_id, COMPONENT_ACTION);
   DCHECK(HasItem(component_item));
   RemoveItem(component_item);
   RemovePref(component_item);
 }
 
@@ skipping 191 matching lines @@
         prefs_->GetInteger(extensions::pref_names::kToolbarSize);
     if (saved_icon_count != visible_icon_count_)
       SetVisibleIconCount(saved_icon_count);
   }
 }
 
 const extensions::Extension* ToolbarActionsModel::GetExtensionById(
     const std::string& id) const {
   return extension_registry_->enabled_extensions().GetByID(id);
 }