Chromium Code Reviews| Index: chrome/browser/ui/toolbar/toolbar_actions_bar.cc |
| diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc |
| index 2a318efd44470ad6b4f3b3c84f29429997376db1..758f3551f6ade8047a1552a48883d096e539896e 100644 |
| --- a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc |
| +++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc |
| @@ -25,6 +25,7 @@ |
| #include "chrome/common/pref_names.h" |
| #include "components/crx_file/id_util.h" |
| #include "components/pref_registry/pref_registry_syncable.h" |
| +#include "extensions/browser/extension_registry.h" |
| #include "extensions/browser/extension_system.h" |
| #include "extensions/browser/runtime_data.h" |
| #include "extensions/common/extension.h" |
| @@ -119,9 +120,10 @@ ToolbarActionsBar::ToolbarActionsBar(ToolbarActionsBarDelegate* delegate, |
| ToolbarActionsBar* main_bar) |
| : delegate_(delegate), |
| browser_(browser), |
| - model_(extensions::ExtensionToolbarModel::Get(browser_->profile())), |
| + model_(ToolbarActionsModel::Get(browser_->profile())), |
| main_bar_(main_bar), |
| platform_settings_(main_bar != nullptr), |
| + component_ids_(ComponentToolbarActionsFactory::GetComponentIds()), |
|
Devlin
2015/08/03 20:58:30
I don't think the ToolbarActionsBar should need kn
apacible
2015/08/04 22:15:58
Done.
|
| popup_owner_(nullptr), |
| model_observer_(this), |
| suppress_layout_(false), |
| @@ -258,7 +260,7 @@ size_t ToolbarActionsBar::GetIconCount() const { |
| // icons than we have, and we should always have a view per item in the model. |
| // (The only exception is if this is in initialization.) |
| if (!toolbar_actions_.empty() && !suppress_layout_ && |
| - model_->extensions_initialized()) { |
| + model_->actions_initialized()) { |
| size_t num_extension_actions = 0u; |
| for (ToolbarActionViewController* action : toolbar_actions_) { |
| // No component action should ever have a valid extension id, so we can |
| @@ -273,7 +275,7 @@ size_t ToolbarActionsBar::GetIconCount() const { |
| size_t num_total_actions = num_extension_actions + num_component_actions; |
| DCHECK_LE(visible_icons, num_total_actions); |
| - DCHECK_EQ(model_->toolbar_items().size(), num_extension_actions); |
| + DCHECK_EQ(model_->toolbar_items().size(), num_total_actions); |
| } |
| #endif |
| @@ -307,7 +309,7 @@ void ToolbarActionsBar::CreateActions() { |
| DCHECK(toolbar_actions_.empty()); |
| // We wait for the extension system to be initialized before we add any |
| // actions, as they rely on the extension system to function. |
| - if (!model_ || !model_->extensions_initialized()) |
| + if (!model_ || !model_->actions_initialized()) |
| return; |
| { |
| @@ -318,37 +320,15 @@ void ToolbarActionsBar::CreateActions() { |
| // 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_->toolbar_items(); |
| - for (const scoped_refptr<const extensions::Extension>& extension : |
| - toolbar_items) { |
| - toolbar_actions_.push_back(new ExtensionActionViewController( |
| - extension.get(), |
| - browser_, |
| - action_manager->GetExtensionAction(*extension), |
| - this)); |
| - } |
| + // Get the toolbar actions. |
| + toolbar_actions_ = model_->GetActions(browser_, this); |
| - // Component actions come second, and are suppressed if the extension |
| - // actions are being highlighted. |
| if (!model_->is_highlighting()) { |
| // TODO(robliao): Remove ScopedTracker below once https://crbug.com/463337 |
| // is fixed. |
| tracked_objects::ScopedTracker tracking_profile2( |
| FROM_HERE_WITH_EXPLICIT_FUNCTION( |
| "ToolbarActionsBar::CreateActions2")); |
| - |
| - ScopedVector<ToolbarActionViewController> component_actions = |
| - ComponentToolbarActionsFactory::GetInstance()-> |
| - GetComponentToolbarActions(browser_); |
| - DCHECK(component_actions.empty() || |
| - extensions::FeatureSwitch::extension_action_redesign()->IsEnabled()); |
| - toolbar_actions_.insert(toolbar_actions_.end(), |
| - component_actions.begin(), |
| - component_actions.end()); |
| - component_actions.weak_clear(); |
| } |
| if (!toolbar_actions_.empty()) { |
| @@ -438,7 +418,7 @@ void ToolbarActionsBar::OnDragDrop(int dragged_index, |
| delta = -1; |
| else if (drag_type == DRAG_TO_MAIN) |
| delta = 1; |
| - model_->MoveExtensionIcon(toolbar_actions_[dragged_index]->GetId(), |
| + model_->MoveActionIcon(toolbar_actions_[dragged_index]->GetId(), |
| dropped_index); |
| if (delta) |
| model_->SetVisibleIconCount(model_->visible_icon_count() + delta); |
| @@ -523,8 +503,7 @@ void ToolbarActionsBar::MaybeShowExtensionBubble( |
| // so wait until animation stops. |
| pending_extension_bubble_controller_ = controller.Pass(); |
| } else { |
| - const extensions::ExtensionIdList& affected_extensions = |
| - controller->GetExtensionIdList(); |
| + const ActionIdList& affected_extensions = controller->GetExtensionIdList(); |
| ToolbarActionViewController* anchor_action = nullptr; |
| for (const std::string& id : affected_extensions) { |
| anchor_action = GetActionForId(id); |
| @@ -535,12 +514,22 @@ void ToolbarActionsBar::MaybeShowExtensionBubble( |
| } |
| } |
| -void ToolbarActionsBar::OnToolbarExtensionAdded( |
| - const extensions::Extension* extension, |
| - int index) { |
| - DCHECK(GetActionForId(extension->id()) == nullptr) << |
| +void ToolbarActionsBar::OnToolbarActionAdded(const std::string& action_id, |
| + int index) { |
| + DCHECK(GetActionForId(action_id) == nullptr) << |
| "Asked to add a toolbar action view for an extension that already exists"; |
| + // Check that |action_id| is not a component action since |
| + // OnToolbarActionAdded is only called after the toolbar model has been |
| + // initialized. |
| + if (std::find(component_ids_.begin(), component_ids_.end(), action_id) != |
| + component_ids_.end()) |
| + return; |
| + |
| + const extensions::Extension* extension = |
| + extensions::ExtensionRegistry::Get(browser_->profile())-> |
| + enabled_extensions().GetByID(action_id); |
| + |
| toolbar_actions_.insert( |
| toolbar_actions_.begin() + index, |
| new ExtensionActionViewController( |
| @@ -553,7 +542,7 @@ void ToolbarActionsBar::OnToolbarExtensionAdded( |
| delegate_->AddViewForAction(toolbar_actions_[index], index); |
| // If we are still initializing the container, don't bother animating. |
| - if (!model_->extensions_initialized()) |
| + if (!model_->actions_initialized()) |
| return; |
| // We may need to resize (e.g. to show the new icon, or the chevron). We don't |
| @@ -567,15 +556,19 @@ void ToolbarActionsBar::OnToolbarExtensionAdded( |
| ResizeDelegate(gfx::Tween::LINEAR, true); |
| } |
| -void ToolbarActionsBar::OnToolbarExtensionRemoved( |
| - const extensions::Extension* extension) { |
| +void ToolbarActionsBar::OnToolbarActionRemoved(const std::string& action_id) { |
| ToolbarActions::iterator iter = toolbar_actions_.begin(); |
| - while (iter != toolbar_actions_.end() && (*iter)->GetId() != extension->id()) |
| + while (iter != toolbar_actions_.end() && (*iter)->GetId() != action_id) |
| ++iter; |
| if (iter == toolbar_actions_.end()) |
| return; |
| + // Component actions cannot be removed. |
| + if (std::find(component_ids_.begin(), component_ids_.end(), action_id) != |
| + component_ids_.end()) |
| + return; |
| + |
| // The action should outlive the UI element (which is owned by the delegate), |
| // so we can't delete it just yet. But we should remove it from the list of |
| // actions so that any width calculations are correct. |
| @@ -589,9 +582,9 @@ void ToolbarActionsBar::OnToolbarExtensionRemoved( |
| // There is an exception if this is an off-the-record profile, and the |
| // extension is no longer incognito-enabled. |
| if (!extensions::ExtensionSystem::Get(browser_->profile())->runtime_data()-> |
| - IsBeingUpgraded(extension->id()) || |
| + IsBeingUpgraded(action_id) || |
| (browser_->profile()->IsOffTheRecord() && |
| - !extensions::util::IsIncognitoEnabled(extension->id(), |
| + !extensions::util::IsIncognitoEnabled(action_id, |
| browser_->profile()))) { |
| if (toolbar_actions_.size() > model_->visible_icon_count()) { |
| // If we have more icons than we can show, then we must not be changing |
| @@ -610,9 +603,8 @@ void ToolbarActionsBar::OnToolbarExtensionRemoved( |
| SetOverflowedActionWantsToRun(); |
| } |
| -void ToolbarActionsBar::OnToolbarExtensionMoved( |
| - const extensions::Extension* extension, |
| - int index) { |
| +void ToolbarActionsBar::OnToolbarActionMoved(const std::string& action_id, |
| + int index) { |
| DCHECK(index >= 0 && index < static_cast<int>(toolbar_actions_.size())); |
| // Unfortunately, |index| doesn't really mean a lot to us, because this |
| // window's toolbar could be different (if actions are popped out). Just |
| @@ -620,9 +612,8 @@ void ToolbarActionsBar::OnToolbarExtensionMoved( |
| ReorderActions(); |
| } |
| -void ToolbarActionsBar::OnToolbarExtensionUpdated( |
| - const extensions::Extension* extension) { |
| - ToolbarActionViewController* action = GetActionForId(extension->id()); |
| +void ToolbarActionsBar::OnToolbarActionUpdated(const std::string& action_id) { |
| + ToolbarActionViewController* action = GetActionForId(action_id); |
| // There might not be a view in cases where we are highlighting or if we |
| // haven't fully initialized the actions. |
| if (action) { |
| @@ -631,14 +622,13 @@ void ToolbarActionsBar::OnToolbarExtensionUpdated( |
| } |
| } |
| -bool ToolbarActionsBar::ShowExtensionActionPopup( |
| - const extensions::Extension* extension, |
| - bool grant_active_tab) { |
| +bool ToolbarActionsBar::ShowToolbarActionPopup(const std::string& action_id, |
| + bool grant_active_tab) { |
| // Don't override another popup, and only show in the active window. |
| if (popup_owner() || !browser_->window()->IsActive()) |
| return false; |
| - ToolbarActionViewController* action = GetActionForId(extension->id()); |
| + ToolbarActionViewController* action = GetActionForId(action_id); |
| return action && action->ExecuteAction(grant_active_tab); |
| } |
| @@ -668,7 +658,7 @@ void ToolbarActionsBar::ResizeDelegate(gfx::Tween::Type tween_type, |
| } |
| void ToolbarActionsBar::OnToolbarHighlightModeChanged(bool is_highlighting) { |
| - if (!model_->extensions_initialized()) |
| + if (!model_->actions_initialized()) |
| return; |
| // It's a bit of a pain that we delete and recreate everything here, but given |
| // everything else going on (the lack of highlight, [n] more extensions |
| @@ -704,8 +694,8 @@ void ToolbarActionsBar::ReorderActions() { |
| // First, reset the order to that of the model. |
| auto compare = [](ToolbarActionViewController* const& action, |
| - const scoped_refptr<const extensions::Extension>& ext) { |
| - return action->GetId() == ext->id(); |
| + const std::string ext) { |
| + return action->GetId() == ext; |
| }; |
| SortContainer(&toolbar_actions_.get(), model_->toolbar_items(), compare); |
| @@ -740,9 +730,9 @@ void ToolbarActionsBar::SetOverflowedActionWantsToRun() { |
| } |
| ToolbarActionViewController* ToolbarActionsBar::GetActionForId( |
| - const std::string& id) { |
| + const std::string& action_id) { |
| for (ToolbarActionViewController* action : toolbar_actions_) { |
| - if (action->GetId() == id) |
| + if (action->GetId() == action_id) |
| return action; |
| } |
| return nullptr; |