Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6394)

Unified Diff: chrome/browser/ui/toolbar/toolbar_actions_model.cc

Issue 1241063003: Support Component Actions in the toolbar. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Changes per mfoltz@'s comments. Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/toolbar/toolbar_actions_model.cc
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/ui/toolbar/toolbar_actions_model.cc
similarity index 53%
rename from chrome/browser/extensions/extension_toolbar_model.cc
rename to chrome/browser/ui/toolbar/toolbar_actions_model.cc
index a50998db3f9c2af102082cddaf041375903a7f05..c7c862b22a10b52708a964d136509182e8661e16 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/ui/toolbar/toolbar_actions_model.cc
@@ -2,7 +2,7 @@
// 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>
@@ -14,124 +14,126 @@
#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
@@ -140,42 +142,85 @@ void ExtensionToolbarModel::SetVisibleIconCount(size_t count) {
// 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()));
+ }
+}
+
+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
apacible 2015/08/10 22:38:10 Added above in the loop that adds the component ac
+
+ return action_list.Pass();
}
-void ExtensionToolbarModel::OnExtensionActionVisibilityChanged(
+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;
@@ -183,20 +228,23 @@ void ExtensionToolbarModel::OnExtensionActionVisibilityChanged(
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
apacible 2015/08/10 22:38:09 Done.
+ extension_id, extensions::ExtensionRegistry::EVERYTHING);
if (is_now_visible)
AddExtension(extension);
else
@@ -204,34 +252,34 @@ void ExtensionToolbarModel::OnExtensionActionVisibilityChanged(
}
}
-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());
@@ -241,41 +289,41 @@ void ExtensionToolbarModel::OnExtensionUninstalled(
}
}
-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;
@@ -287,19 +335,18 @@ size_t ExtensionToolbarModel::FindNewPositionFromLastKnownGood(
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;
}
@@ -307,9 +354,9 @@ bool ExtensionToolbarModel::ShouldAddExtension(const Extension* 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;
@@ -323,8 +370,8 @@ void ExtensionToolbarModel::AddExtension(const Extension* extension) {
// 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
@@ -333,7 +380,7 @@ void ExtensionToolbarModel::AddExtension(const Extension* extension) {
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,
@@ -345,17 +392,19 @@ void ExtensionToolbarModel::AddExtension(const Extension* extension) {
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()) {
@@ -365,13 +414,13 @@ void ExtensionToolbarModel::AddExtension(const Extension* extension) {
} 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
@@ -386,12 +435,14 @@ void ExtensionToolbarModel::AddExtension(const Extension* extension) {
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;
@@ -406,34 +457,36 @@ void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
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
@@ -442,89 +495,148 @@ void ExtensionToolbarModel::InitializeExtensionList() {
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 gen
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 ha
apacible 2015/08/10 22:38:10 I add to the struct of string, type after this che
Devlin 2015/08/13 21:21:10 That's not quite what I was suggesting. I was won
apacible 2015/08/15 08:46:40 Talked offline: - Created ToolbarItem struct. - Pa
+ 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 extensi
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
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
apacible 2015/08/10 22:38:10 Good point. How would I go about finding how to hi
Devlin 2015/08/13 21:21:10 The decision is set by whether or not it's visible
apacible 2015/08/15 08:46:40 Talked offline; updated comment. As a start, we'l
toolbar_items_.push_back(*iter);
if (iter - original_model->toolbar_items_.begin() < original_visible)
++visible_icon_count_;
@@ -532,51 +644,54 @@ void ExtensionToolbarModel::IncognitoPopulate() {
}
}
-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
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(),
@@ -594,13 +709,14 @@ void ExtensionToolbarModel::OnExtensionToolbarPrefChange() {
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;
}
@@ -609,23 +725,22 @@ void ExtensionToolbarModel::OnExtensionToolbarPrefChange() {
// 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 diff
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.
@@ -633,52 +748,46 @@ bool ExtensionToolbarModel::ShowExtensionActionPopup(
// (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);
}
}
@@ -694,8 +803,8 @@ bool ExtensionToolbarModel::HighlightExtensions(
// 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;
}
@@ -707,7 +816,7 @@ bool ExtensionToolbarModel::HighlightExtensions(
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().
@@ -722,21 +831,34 @@ void ExtensionToolbarModel::StopHighlighting() {
// 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);
+}

Powered by Google App Engine
This is Rietveld 408576698