summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorrdevlin.cronin@chromium.org <rdevlin.cronin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-18 19:24:47 +0000
committerrdevlin.cronin@chromium.org <rdevlin.cronin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-18 19:26:03 +0000
commit8a188eee9d8bccc5345fe52254437412b94a3ea9 (patch)
treea812681b3bbe9c86eaf2ee7f36d7bb91dfa3d0f0
parentee7a5fd630f228f0b4e818d8184c354f20edbb75 (diff)
downloadchromium_src-8a188eee9d8bccc5345fe52254437412b94a3ea9.zip
chromium_src-8a188eee9d8bccc5345fe52254437412b94a3ea9.tar.gz
chromium_src-8a188eee9d8bccc5345fe52254437412b94a3ea9.tar.bz2
Have ExtensionToolbarModel include page action extensions
BUG=397259 Review URL: https://codereview.chromium.org/464623002 Cr-Commit-Position: refs/heads/master@{#290325} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290325 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--chrome/browser/extensions/api/extension_action/browser_action_apitest.cc4
-rw-r--r--chrome/browser/extensions/extension_toolbar_model.cc79
-rw-r--r--chrome/browser/extensions/extension_toolbar_model.h45
-rw-r--r--chrome/browser/extensions/extension_toolbar_model_browsertest.cc35
-rw-r--r--chrome/browser/extensions/extension_toolbar_model_unittest.cc139
-rw-r--r--chrome/browser/extensions/test_extension_system.h7
-rw-r--r--chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm18
-rw-r--r--chrome/browser/ui/views/extensions/extension_action_view_controller.cc2
-rw-r--r--chrome/browser/ui/views/toolbar/browser_action_view.cc7
-rw-r--r--chrome/browser/ui/views/toolbar/browser_action_view.h5
-rw-r--r--chrome/browser/ui/views/toolbar/browser_actions_container.cc44
-rw-r--r--chrome/browser/ui/views/toolbar/browser_actions_container.h16
-rw-r--r--chrome/chrome_tests_unit.gypi1
-rw-r--r--chrome/common/extensions/manifest_handlers/synthesize_browser_action_handler.cc7
14 files changed, 305 insertions, 104 deletions
diff --git a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
index 61e647c..ba86982 100644
--- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
@@ -552,7 +552,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, IncognitoDragging) {
// ABC -> CAB
ExtensionToolbarModel* toolbar_model = ExtensionToolbarModel::Get(
browser()->profile());
- toolbar_model->MoveBrowserAction(extension_c, 0);
+ toolbar_model->MoveExtensionIcon(extension_c, 0);
EXPECT_EQ(kTooltipC, GetBrowserActionsBar().GetTooltip(0));
EXPECT_EQ(kTooltipA, GetBrowserActionsBar().GetTooltip(1));
@@ -562,7 +562,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, IncognitoDragging) {
EXPECT_EQ(kTooltipA, incognito_bar.GetTooltip(1));
// CAB -> CBA
- toolbar_model->MoveBrowserAction(extension_b, 1);
+ toolbar_model->MoveExtensionIcon(extension_b, 1);
EXPECT_EQ(kTooltipC, GetBrowserActionsBar().GetTooltip(0));
EXPECT_EQ(kTooltipB, GetBrowserActionsBar().GetTooltip(1));
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index 7dea3ac..a87656b 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -34,17 +34,14 @@
namespace extensions {
-bool ExtensionToolbarModel::Observer::BrowserActionShowPopup(
- const Extension* extension) {
- return false;
-}
-
ExtensionToolbarModel::ExtensionToolbarModel(Profile* profile,
ExtensionPrefs* extension_prefs)
: profile_(profile),
extension_prefs_(extension_prefs),
prefs_(profile_->GetPrefs()),
extensions_initialized_(false),
+ include_all_extensions_(
+ FeatureSwitch::extension_action_redesign()->IsEnabled()),
is_highlighting_(false),
extension_registry_observer_(this),
weak_ptr_factory_(this) {
@@ -76,7 +73,7 @@ void ExtensionToolbarModel::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
-void ExtensionToolbarModel::MoveBrowserAction(const Extension* extension,
+void ExtensionToolbarModel::MoveExtensionIcon(const Extension* extension,
int index) {
ExtensionList::iterator pos = std::find(toolbar_items_.begin(),
toolbar_items_.end(), extension);
@@ -116,7 +113,8 @@ void ExtensionToolbarModel::MoveBrowserAction(const Extension* extension,
last_known_positions_.push_back(extension->id());
}
- FOR_EACH_OBSERVER(Observer, observers_, BrowserActionMoved(extension, index));
+ FOR_EACH_OBSERVER(
+ Observer, observers_, ToolbarExtensionMoved(extension, index));
UpdatePrefs();
}
@@ -175,10 +173,8 @@ void ExtensionToolbarModel::OnExtensionLoaded(
if (toolbar_items_[i].get() == extension)
return;
}
- if (ExtensionActionAPI::GetBrowserActionVisibility(extension_prefs_,
- extension->id())) {
- AddExtension(extension);
- }
+
+ AddExtension(extension);
}
void ExtensionToolbarModel::OnExtensionUnloaded(
@@ -240,7 +236,7 @@ size_t ExtensionToolbarModel::FindNewPositionFromLastKnownGood(
// 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
- // browser action icons preceding |extension|.
+ // 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())
@@ -260,9 +256,25 @@ size_t ExtensionToolbarModel::FindNewPositionFromLastKnownGood(
return toolbar_items_.size();
}
+bool ExtensionToolbarModel::ShouldAddExtension(const Extension* extension) {
+ ExtensionActionManager* action_manager =
+ 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->GetBrowserAction(*extension) ||
+ action_manager->GetPageAction(*extension);
+ }
+
+ return action_manager->GetBrowserAction(*extension) &&
+ ExtensionActionAPI::GetBrowserActionVisibility(
+ extension_prefs_, extension->id());
+}
+
void ExtensionToolbarModel::AddExtension(const Extension* extension) {
- // We only care about extensions with browser actions.
- if (!ExtensionActionManager::Get(profile_)->GetBrowserAction(*extension))
+ if (!ShouldAddExtension(extension))
return;
size_t new_index = toolbar_items_.size();
@@ -291,8 +303,8 @@ void ExtensionToolbarModel::AddExtension(const Extension* extension) {
// 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_,
- BrowserActionAdded(extension, new_index));
+ FOR_EACH_OBSERVER(
+ Observer, observers_, ToolbarExtensionAdded(extension, new_index));
}
}
@@ -312,13 +324,14 @@ void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
extension);
if (pos != highlighted_items_.end()) {
highlighted_items_.erase(pos);
- FOR_EACH_OBSERVER(Observer, observers_, BrowserActionRemoved(extension));
+ FOR_EACH_OBSERVER(
+ Observer, observers_, ToolbarExtensionRemoved(extension));
// If the highlighted list is now empty, we stop highlighting.
if (highlighted_items_.empty())
StopHighlighting();
}
} else {
- FOR_EACH_OBSERVER(Observer, observers_, BrowserActionRemoved(extension));
+ FOR_EACH_OBSERVER(Observer, observers_, ToolbarExtensionRemoved(extension));
}
UpdatePrefs();
@@ -337,7 +350,7 @@ void ExtensionToolbarModel::InitializeExtensionList(
Populate(last_known_positions_, extensions);
extensions_initialized_ = true;
- FOR_EACH_OBSERVER(Observer, observers_, VisibleCountChanged());
+ FOR_EACH_OBSERVER(Observer, observers_, ToolbarVisibleCountChanged());
}
void ExtensionToolbarModel::Populate(const ExtensionIdList& positions,
@@ -357,11 +370,9 @@ void ExtensionToolbarModel::Populate(const ExtensionIdList& positions,
it != extensions.end();
++it) {
const Extension* extension = it->get();
- if (!extension_action_manager->GetBrowserAction(*extension))
- continue;
- if (!ExtensionActionAPI::GetBrowserActionVisibility(
- extension_prefs_, extension->id())) {
- ++hidden;
+ if (!ShouldAddExtension(extension)) {
+ if (extension_action_manager->GetBrowserAction(*extension))
+ ++hidden;
continue;
}
@@ -381,7 +392,7 @@ void ExtensionToolbarModel::Populate(const ExtensionIdList& positions,
// calls SetVisibleCount.
toolbar_items_.pop_back();
FOR_EACH_OBSERVER(
- Observer, observers_, BrowserActionRemoved(extension));
+ Observer, observers_, ToolbarExtensionRemoved(extension));
}
DCHECK(toolbar_items_.empty());
@@ -399,7 +410,7 @@ void ExtensionToolbarModel::Populate(const ExtensionIdList& positions,
if (iter->get() != NULL) {
toolbar_items_.push_back(*iter);
FOR_EACH_OBSERVER(
- Observer, observers_, BrowserActionAdded(
+ Observer, observers_, ToolbarExtensionAdded(
*iter, toolbar_items_.size() - 1));
}
}
@@ -408,7 +419,7 @@ void ExtensionToolbarModel::Populate(const ExtensionIdList& positions,
if (iter->get() != NULL) {
toolbar_items_.push_back(*iter);
FOR_EACH_OBSERVER(
- Observer, observers_, BrowserActionAdded(
+ Observer, observers_, ToolbarExtensionAdded(
*iter, toolbar_items_.size() - 1));
}
}
@@ -502,7 +513,7 @@ bool ExtensionToolbarModel::ShowBrowserActionPopup(const Extension* extension) {
Observer* obs = NULL;
while ((obs = it.GetNext()) != NULL) {
// Stop after first popup since it should only show in the active window.
- if (obs->BrowserActionShowPopup(extension))
+ if (obs->ShowExtensionActionPopup(extension))
return true;
}
return false;
@@ -519,7 +530,7 @@ void ExtensionToolbarModel::EnsureVisibility(
SetVisibleIconCount(extension_ids.size());
// Inform observers.
- FOR_EACH_OBSERVER(Observer, observers_, VisibleCountChanged());
+ FOR_EACH_OBSERVER(Observer, observers_, ToolbarVisibleCountChanged());
}
if (visible_icon_count_ == -1)
@@ -532,7 +543,7 @@ void ExtensionToolbarModel::EnsureVisibility(
extension != toolbar_items_.end(); ++extension) {
if ((*extension)->id() == (*it)) {
if (extension - toolbar_items_.begin() >= visible_icon_count_)
- MoveBrowserAction(*extension, 0);
+ MoveExtensionIcon(*extension, 0);
break;
}
}
@@ -562,10 +573,10 @@ bool ExtensionToolbarModel::HighlightExtensions(
if (visible_icon_count_ != -1 &&
visible_icon_count_ < static_cast<int>(extension_ids.size())) {
SetVisibleIconCount(extension_ids.size());
- FOR_EACH_OBSERVER(Observer, observers_, VisibleCountChanged());
+ FOR_EACH_OBSERVER(Observer, observers_, ToolbarVisibleCountChanged());
}
- FOR_EACH_OBSERVER(Observer, observers_, HighlightModeChanged(true));
+ FOR_EACH_OBSERVER(Observer, observers_, ToolbarHighlightModeChanged(true));
return true;
}
@@ -582,9 +593,9 @@ void ExtensionToolbarModel::StopHighlighting() {
is_highlighting_ = false;
if (old_visible_icon_count_ != visible_icon_count_) {
SetVisibleIconCount(old_visible_icon_count_);
- FOR_EACH_OBSERVER(Observer, observers_, VisibleCountChanged());
+ FOR_EACH_OBSERVER(Observer, observers_, ToolbarVisibleCountChanged());
}
- FOR_EACH_OBSERVER(Observer, observers_, HighlightModeChanged(false));
+ FOR_EACH_OBSERVER(Observer, observers_, ToolbarHighlightModeChanged(false));
}
}
diff --git a/chrome/browser/extensions/extension_toolbar_model.h b/chrome/browser/extensions/extension_toolbar_model.h
index 0068b8b..457311d 100644
--- a/chrome/browser/extensions/extension_toolbar_model.h
+++ b/chrome/browser/extensions/extension_toolbar_model.h
@@ -35,25 +35,28 @@ class ExtensionToolbarModel : public content::NotificationObserver,
// A class which is informed of changes to the model; represents the view of
// MVC. Also used for signaling view changes such as showing extension popups.
+ // TODO(devlin): Should this really be an observer? There should probably be
+ // only one (aka a Delegate)...
class Observer {
public:
- // An extension with a browser action button has been added, and should go
- // in the toolbar at |index|.
- virtual void BrowserActionAdded(const Extension* extension, int index) {}
+ // An extension has been added to the toolbar and should go at |index|.
+ virtual void ToolbarExtensionAdded(const Extension* extension,
+ int index) = 0;
- // The browser action button for |extension| should no longer show.
- virtual void BrowserActionRemoved(const Extension* extension) {}
+ // The given |extension| should be removed from the toolbar.
+ virtual void ToolbarExtensionRemoved(const Extension* extension) = 0;
- // The browser action button for |extension| has been moved to |index|.
- virtual void BrowserActionMoved(const Extension* extension, int index) {}
+ // The given |extension| has been moved to |index|.
+ virtual void ToolbarExtensionMoved(const Extension* extension,
+ int index) = 0;
// Signal the |extension| to show the popup now in the active window.
// Returns true if a popup was slated to be shown.
- virtual bool BrowserActionShowPopup(const Extension* extension);
+ virtual bool ShowExtensionActionPopup(const Extension* extension) = 0;
// Signal when the container needs to be redrawn because of a size change,
// and when the model has finished loading.
- virtual void VisibleCountChanged() {}
+ virtual void ToolbarVisibleCountChanged() = 0;
// Signal that the model has entered or exited highlighting mode, or that
// the extensions being highlighted have (probably*) changed. Highlighting
@@ -62,7 +65,7 @@ class ExtensionToolbarModel : public content::NotificationObserver,
// * probably, because if we are in highlight mode and receive a call to
// highlight a new set of extensions, we do not compare the current set
// with the new set (and just assume the new set is different).
- virtual void HighlightModeChanged(bool is_highlighting) {}
+ virtual void ToolbarHighlightModeChanged(bool is_highlighting) = 0;
protected:
virtual ~Observer() {}
@@ -71,10 +74,13 @@ class ExtensionToolbarModel : public content::NotificationObserver,
// Convenience function to get the ExtensionToolbarModel for a Profile.
static ExtensionToolbarModel* Get(Profile* profile);
- // Functions called by the view.
+ // Add or remove an observer.
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
- void MoveBrowserAction(const Extension* extension, int index);
+
+ // Moves the given |extension|'s icon to the given |index|.
+ void MoveExtensionIcon(const Extension* extension, int index);
+
// Executes the browser action for an extension and returns the action that
// the UI should perform in response.
// |popup_url_out| will be set if the extension should show a popup, with
@@ -86,9 +92,12 @@ class ExtensionToolbarModel : public content::NotificationObserver,
Browser* browser,
GURL* popup_url_out,
bool should_grant);
+
+ // Sets the number of extension icons that should be visible.
// If count == size(), this will set the visible icon count to -1, meaning
// "show all actions".
void SetVisibleIconCount(int count);
+
// As above, a return value of -1 represents "show all actions".
int GetVisibleIconCount() const { return visible_icon_count_; }
@@ -162,12 +171,16 @@ class ExtensionToolbarModel : public content::NotificationObserver,
// value returned is a zero-based index into the vector of visible items.
size_t FindNewPositionFromLastKnownGood(const Extension* extension);
- // Our observers.
- ObserverList<Observer> observers_;
+ // Returns true if the given |extension| should be added to the toolbar.
+ bool ShouldAddExtension(const Extension* extension);
+ // Adds or removes the given |extension| from the toolbar model.
void AddExtension(const Extension* extension);
void RemoveExtension(const Extension* extension);
+ // Our observers.
+ ObserverList<Observer> observers_;
+
// The Profile this toolbar model is for.
Profile* profile_;
@@ -177,6 +190,10 @@ class ExtensionToolbarModel : public content::NotificationObserver,
// True if we've handled the initial EXTENSIONS_READY notification.
bool extensions_initialized_;
+ // If true, we include all extensions in the toolbar model. If false, we only
+ // include browser actions.
+ bool include_all_extensions_;
+
// Ordered list of browser action buttons.
ExtensionList toolbar_items_;
diff --git a/chrome/browser/extensions/extension_toolbar_model_browsertest.cc b/chrome/browser/extensions/extension_toolbar_model_browsertest.cc
index a34b89e..b5bac5b 100644
--- a/chrome/browser/extensions/extension_toolbar_model_browsertest.cc
+++ b/chrome/browser/extensions/extension_toolbar_model_browsertest.cc
@@ -36,21 +36,28 @@ class ExtensionToolbarModelTest : public ExtensionBrowserTest,
model_->RemoveObserver(this);
}
- virtual void BrowserActionAdded(const Extension* extension,
- int index) OVERRIDE {
+ virtual void ToolbarExtensionAdded(const Extension* extension,
+ int index) OVERRIDE {
inserted_count_++;
}
- virtual void BrowserActionRemoved(const Extension* extension) OVERRIDE {
+ virtual void ToolbarExtensionRemoved(const Extension* extension) OVERRIDE {
removed_count_++;
}
- virtual void BrowserActionMoved(const Extension* extension,
- int index) OVERRIDE {
+ virtual void ToolbarExtensionMoved(const Extension* extension,
+ int index) OVERRIDE {
moved_count_++;
}
- virtual void HighlightModeChanged(bool is_highlighting) OVERRIDE {
+ virtual bool ShowExtensionActionPopup(const Extension* extension) OVERRIDE {
+ return false;
+ }
+
+ virtual void ToolbarVisibleCountChanged() OVERRIDE {
+ }
+
+ virtual void ToolbarHighlightModeChanged(bool is_highlighting) OVERRIDE {
// Add one if highlighting, subtract one if not.
highlight_mode_count_ += is_highlighting ? 1 : -1;
}
@@ -99,7 +106,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, Basic) {
extension->name().c_str());
// Should be a no-op, but still fires the events.
- model_->MoveBrowserAction(extension, 0);
+ model_->MoveExtensionIcon(extension, 0);
EXPECT_EQ(1, moved_count_);
EXPECT_EQ(1u, model_->toolbar_items().size());
const Extension* extension2 = ExtensionAt(0);
@@ -160,7 +167,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, MAYBE_ReorderAndReinsert) {
extensionC->name().c_str());
// Order is now A, B, C. Let's put C first.
- model_->MoveBrowserAction(extensionC, 0);
+ model_->MoveExtensionIcon(extensionC, 0);
EXPECT_EQ(1, moved_count_);
EXPECT_EQ(3u, model_->toolbar_items().size());
EXPECT_EQ(extensionC, ExtensionAt(0));
@@ -169,7 +176,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, MAYBE_ReorderAndReinsert) {
EXPECT_EQ(NULL, ExtensionAt(3));
// Order is now C, A, B. Let's put A last.
- model_->MoveBrowserAction(extensionA, 2);
+ model_->MoveExtensionIcon(extensionA, 2);
EXPECT_EQ(2, moved_count_);
EXPECT_EQ(3u, model_->toolbar_items().size());
EXPECT_EQ(extensionC, ExtensionAt(0));
@@ -205,7 +212,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, MAYBE_ReorderAndReinsert) {
EXPECT_EQ(NULL, ExtensionAt(2));
// Order is now C, A. Flip it.
- model_->MoveBrowserAction(extensionA, 0);
+ model_->MoveExtensionIcon(extensionA, 0);
EXPECT_EQ(3, moved_count_);
EXPECT_EQ(2u, model_->toolbar_items().size());
EXPECT_EQ(extensionA, ExtensionAt(0));
@@ -213,7 +220,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, MAYBE_ReorderAndReinsert) {
EXPECT_EQ(NULL, ExtensionAt(2));
// Move A to the location it already occupies.
- model_->MoveBrowserAction(extensionA, 0);
+ model_->MoveExtensionIcon(extensionA, 0);
EXPECT_EQ(4, moved_count_);
EXPECT_EQ(2u, model_->toolbar_items().size());
EXPECT_EQ(extensionA, ExtensionAt(0));
@@ -287,8 +294,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, UnloadAndDisableMultiple) {
EXPECT_STREQ(idC.c_str(), extensionC->id().c_str());
// Put C in the middle and A to the end.
- model_->MoveBrowserAction(extensionC, 1);
- model_->MoveBrowserAction(extensionA, 2);
+ model_->MoveExtensionIcon(extensionC, 1);
+ model_->MoveExtensionIcon(extensionA, 2);
// Make sure we get this order (C, B, A).
EXPECT_STREQ(idC.c_str(), ExtensionAt(0)->id().c_str());
@@ -332,7 +339,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, Uninstall) {
EXPECT_STREQ("Popup tester", extensionB->name().c_str());
// Order is now A, B. Make B first.
- model_->MoveBrowserAction(extensionB, 0);
+ model_->MoveExtensionIcon(extensionB, 0);
// Order is now B, A. Uninstall Extension B.
UninstallExtension(idB);
diff --git a/chrome/browser/extensions/extension_toolbar_model_unittest.cc b/chrome/browser/extensions/extension_toolbar_model_unittest.cc
new file mode 100644
index 0000000..50d0446
--- /dev/null
+++ b/chrome/browser/extensions/extension_toolbar_model_unittest.cc
@@ -0,0 +1,139 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_service_test_base.h"
+#include "chrome/browser/extensions/extension_toolbar_model.h"
+#include "chrome/browser/extensions/test_extension_system.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/extensions/api/extension_action/action_info.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_builder.h"
+#include "extensions/common/feature_switch.h"
+#include "extensions/common/id_util.h"
+#include "extensions/common/manifest_constants.h"
+#include "extensions/common/value_builder.h"
+
+namespace extensions {
+
+namespace {
+
+// Create an extension. If |action_key| is non-NULL, it should point to either
+// kBrowserAction or kPageAction, and the extension will have the associated
+// action.
+scoped_refptr<const Extension> GetActionExtension(const std::string& name,
+ const char* action_key) {
+ DictionaryBuilder manifest;
+ manifest.Set("name", name)
+ .Set("description", "An extension")
+ .Set("manifest_version", 2)
+ .Set("version", "1.0.0");
+ if (action_key)
+ manifest.Set(action_key, DictionaryBuilder().Pass());
+
+ return ExtensionBuilder().SetManifest(manifest.Pass())
+ .SetID(id_util::GenerateId(name))
+ .Build();
+}
+
+} // namespace
+
+// TODO(devlin): Experiment with moving (some of the)
+// ExtensionToolbarModelBrowserTests into here?
+class ExtensionToolbarModelUnitTest : public ExtensionServiceTestBase {
+ protected:
+ // Initialize the ExtensionService, ExtensionToolbarModel, and
+ // ExtensionSystem.
+ void Init();
+
+ // Add three extensions, one each for browser action, page action, and no
+ // action.
+ testing::AssertionResult AddActionExtensions() WARN_UNUSED_RESULT;
+
+ ExtensionToolbarModel* toolbar_model() { return toolbar_model_.get(); }
+ const Extension* browser_action() { return browser_action_extension_.get(); }
+ const Extension* page_action() { return page_action_extension_.get(); }
+ const Extension* no_action() { return no_action_extension_.get(); }
+
+ private:
+ // The associated (and owned) toolbar model.
+ scoped_ptr<ExtensionToolbarModel> toolbar_model_;
+
+ // Three sample extensions.
+ scoped_refptr<const Extension> browser_action_extension_;
+ scoped_refptr<const Extension> page_action_extension_;
+ scoped_refptr<const Extension> no_action_extension_;
+};
+
+void ExtensionToolbarModelUnitTest::Init() {
+ InitializeEmptyExtensionService();
+ toolbar_model_.reset(
+ new ExtensionToolbarModel(profile(), ExtensionPrefs::Get(profile())));
+ static_cast<TestExtensionSystem*>(ExtensionSystem::Get(profile()))->
+ SetReady();
+ // Run tasks posted to TestExtensionSystem.
+ base::RunLoop().RunUntilIdle();
+}
+
+testing::AssertionResult ExtensionToolbarModelUnitTest::AddActionExtensions() {
+ browser_action_extension_ =
+ GetActionExtension("browser_action", manifest_keys::kBrowserAction);
+ page_action_extension_ =
+ GetActionExtension("page_action", manifest_keys::kPageAction);
+ no_action_extension_ = GetActionExtension("no_action", NULL);
+
+ ExtensionList extensions;
+ extensions.push_back(browser_action_extension_);
+ extensions.push_back(page_action_extension_);
+ extensions.push_back(no_action_extension_);
+
+ // Add and verify each extension.
+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
+ for (ExtensionList::const_iterator iter = extensions.begin();
+ iter != extensions.end(); ++iter) {
+ service()->AddExtension(iter->get());
+ if (!registry->enabled_extensions().GetByID((*iter)->id())) {
+ return testing::AssertionFailure() << "Failed to install extension: " <<
+ (*iter)->name();
+ }
+ }
+
+ return testing::AssertionSuccess();
+}
+
+// Test that, in the absence of the extension-action-redesign switch, the
+// model only contains extensions with browser actions.
+TEST_F(ExtensionToolbarModelUnitTest, TestToolbarExtensionTypesNoSwitch) {
+ Init();
+ ASSERT_TRUE(AddActionExtensions());
+
+ const ExtensionList& extensions = toolbar_model()->toolbar_items();
+ ASSERT_EQ(1u, extensions.size());
+ EXPECT_EQ(browser_action(), extensions[0]);
+}
+
+// Test that, with the extension-action-redesign switch, the model contains
+// all types of extensions, except those which should not be displayed on the
+// toolbar (like component extensions).
+TEST_F(ExtensionToolbarModelUnitTest, TestToolbarExtensionTypesSwitch) {
+ FeatureSwitch::ScopedOverride enable_redesign(
+ FeatureSwitch::extension_action_redesign(), true);
+ Init();
+ ASSERT_TRUE(AddActionExtensions());
+
+ // With the switch on, extensions with page actions and no action should also
+ // be displayed in the toolbar.
+ const ExtensionList& extensions = toolbar_model()->toolbar_items();
+ ASSERT_EQ(3u, extensions.size());
+ EXPECT_EQ(browser_action(), extensions[0]);
+ EXPECT_EQ(page_action(), extensions[1]);
+ EXPECT_EQ(no_action(), extensions[2]);
+}
+
+} // namespace extensions
diff --git a/chrome/browser/extensions/test_extension_system.h b/chrome/browser/extensions/test_extension_system.h
index f436c9f..fb88ef3 100644
--- a/chrome/browser/extensions/test_extension_system.h
+++ b/chrome/browser/extensions/test_extension_system.h
@@ -86,10 +86,9 @@ class TestExtensionSystem : public ExtensionSystem {
GetDeclarativeUserScriptMasterByExtension(
const ExtensionId& extension_id) OVERRIDE;
- void SetReady() {
- LOG(INFO) << "SetReady()";
- ready_.Signal();
- }
+ // Note that you probably want to use base::RunLoop().RunUntilIdle() right
+ // after this to run all the accumulated tasks.
+ void SetReady() { ready_.Signal(); }
// Factory method for tests to use with SetTestingProfile.
static KeyedService* Build(content::BrowserContext* profile);
diff --git a/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm b/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm
index 504809d..04da4d0 100644
--- a/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm
+++ b/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm
@@ -220,19 +220,23 @@ class ExtensionServiceObserverBridge
}
// extensions::ExtensionToolbarModel::Observer implementation.
- virtual void BrowserActionAdded(
+ virtual void ToolbarExtensionAdded(
const Extension* extension,
int index) OVERRIDE {
[owner_ createActionButtonForExtension:extension withIndex:index];
[owner_ resizeContainerAndAnimate:NO];
}
- virtual void BrowserActionRemoved(const Extension* extension) OVERRIDE {
+ virtual void ToolbarExtensionRemoved(const Extension* extension) OVERRIDE {
[owner_ removeActionButtonForExtension:extension];
[owner_ resizeContainerAndAnimate:NO];
}
- virtual bool BrowserActionShowPopup(const Extension* extension) OVERRIDE {
+ virtual void ToolbarExtensionMoved(const Extension* extension,
+ int index) OVERRIDE {
+ }
+
+ virtual bool ShowExtensionActionPopup(const Extension* extension) OVERRIDE {
// Do not override other popups and only show in active window.
ExtensionPopupController* popup = [ExtensionPopupController popup];
if (popup || !browser_->window()->IsActive())
@@ -243,6 +247,12 @@ class ExtensionServiceObserverBridge
shouldGrant:NO];
}
+ virtual void ToolbarVisibleCountChanged() OVERRIDE {
+ }
+
+ virtual void ToolbarHighlightModeChanged(bool is_highlighting) OVERRIDE {
+ }
+
private:
// The object we need to inform when we get a notification. Weak. Owns us.
BrowserActionsController* owner_;
@@ -704,7 +714,7 @@ class ExtensionServiceObserverBridge
if (intersectionWidth > dragThreshold && button != draggedButton &&
![button isAnimating] && index < [self visibleButtonCount]) {
- toolbarModel_->MoveBrowserAction([draggedButton extension], index);
+ toolbarModel_->MoveExtensionIcon([draggedButton extension], index);
[self positionActionButtonsAndAnimate:YES];
return;
}
diff --git a/chrome/browser/ui/views/extensions/extension_action_view_controller.cc b/chrome/browser/ui/views/extensions/extension_action_view_controller.cc
index c5bbe63..56bf76f 100644
--- a/chrome/browser/ui/views/extensions/extension_action_view_controller.cc
+++ b/chrome/browser/ui/views/extensions/extension_action_view_controller.cc
@@ -48,10 +48,10 @@ ExtensionActionViewController::ExtensionActionViewController(
icon_factory_(browser->profile(), extension, extension_action, this),
popup_(NULL),
weak_factory_(this) {
+ DCHECK(extension_action);
DCHECK(extension_action->action_type() == ActionInfo::TYPE_PAGE ||
extension_action->action_type() == ActionInfo::TYPE_BROWSER);
DCHECK(extension);
-
}
ExtensionActionViewController::~ExtensionActionViewController() {
diff --git a/chrome/browser/ui/views/toolbar/browser_action_view.cc b/chrome/browser/ui/views/toolbar/browser_action_view.cc
index 1519518..6ddc7ed 100644
--- a/chrome/browser/ui/views/toolbar/browser_action_view.cc
+++ b/chrome/browser/ui/views/toolbar/browser_action_view.cc
@@ -9,7 +9,6 @@
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/extension_action.h"
-#include "chrome/browser/extensions/extension_action_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/themes/theme_service.h"
#include "chrome/browser/themes/theme_service_factory.h"
@@ -46,14 +45,14 @@ const int kBorderInset = 4;
// BrowserActionView
BrowserActionView::BrowserActionView(const Extension* extension,
+ ExtensionAction* extension_action,
Browser* browser,
BrowserActionView::Delegate* delegate)
: MenuButton(this, base::string16(), NULL, false),
view_controller_(new ExtensionActionViewController(
extension,
browser,
- extensions::ExtensionActionManager::Get(browser->profile())->
- GetBrowserAction(*extension),
+ extension_action,
this)),
delegate_(delegate),
called_registered_extension_command_(false),
@@ -67,7 +66,7 @@ BrowserActionView::BrowserActionView(const Extension* extension,
content::Source<Profile>(browser->profile()->GetOriginalProfile());
registrar_.Add(this,
extensions::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED,
- content::Source<ExtensionAction>(extension_action()));
+ content::Source<ExtensionAction>(extension_action));
registrar_.Add(this,
extensions::NOTIFICATION_EXTENSION_COMMAND_ADDED,
notification_source);
diff --git a/chrome/browser/ui/views/toolbar/browser_action_view.h b/chrome/browser/ui/views/toolbar/browser_action_view.h
index 4d0af0c..6d3c26a 100644
--- a/chrome/browser/ui/views/toolbar/browser_action_view.h
+++ b/chrome/browser/ui/views/toolbar/browser_action_view.h
@@ -82,8 +82,9 @@ class BrowserActionView : public views::MenuButton,
};
BrowserActionView(const extensions::Extension* extension,
- Browser* browser,
- BrowserActionView::Delegate* delegate);
+ ExtensionAction* extension_action,
+ Browser* browser,
+ BrowserActionView::Delegate* delegate);
virtual ~BrowserActionView();
const extensions::Extension* extension() const {
diff --git a/chrome/browser/ui/views/toolbar/browser_actions_container.cc b/chrome/browser/ui/views/toolbar/browser_actions_container.cc
index 491d85c..fce979e 100644
--- a/chrome/browser/ui/views/toolbar/browser_actions_container.cc
+++ b/chrome/browser/ui/views/toolbar/browser_actions_container.cc
@@ -6,6 +6,7 @@
#include "base/compiler_specific.h"
#include "base/stl_util.h"
+#include "chrome/browser/extensions/extension_action_manager.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/extensions/extension_view_host.h"
#include "chrome/browser/extensions/tab_helper.h"
@@ -65,6 +66,15 @@ const int kIconsPerMenuRow = 8; // The menu on Linux is wider.
const int kIconsPerMenuRow = 7;
#endif
+// Returns the ExtensionAction for the given |extension|.
+ExtensionAction* GetExtensionAction(const Extension& extension,
+ Profile* profile) {
+ extensions::ExtensionActionManager* action_manager =
+ extensions::ExtensionActionManager::Get(profile);
+ ExtensionAction* action = action_manager->GetBrowserAction(extension);
+ return action ? action : action_manager->GetPageAction(extension);
+}
+
// A version of MenuButton with almost empty insets to fit properly on the
// toolbar.
class ChevronMenuButton : public views::MenuButton {
@@ -223,7 +233,11 @@ void BrowserActionsContainer::CreateBrowserActionViews() {
if (!ShouldDisplayBrowserAction(i->get()))
continue;
- BrowserActionView* view = new BrowserActionView(i->get(), browser_, this);
+ BrowserActionView* view =
+ new BrowserActionView(i->get(),
+ GetExtensionAction(*i->get(), profile_),
+ browser_,
+ this);
browser_action_views_.push_back(view);
AddChildView(view);
}
@@ -532,7 +546,7 @@ int BrowserActionsContainer::OnPerformDrop(
if (profile_->IsOffTheRecord())
i = model_->IncognitoIndexToOriginal(i);
- model_->MoveBrowserAction(
+ model_->MoveExtensionIcon(
browser_action_views_[data.index()]->extension(), i);
OnDragExited(); // Perform clean up after dragging.
@@ -666,7 +680,7 @@ void BrowserActionsContainer::MoveBrowserAction(const std::string& extension_id,
size_t new_index) {
const Extension* extension = extensions::ExtensionRegistry::Get(profile_)->
enabled_extensions().GetByID(extension_id);
- model_->MoveBrowserAction(extension, new_index);
+ model_->MoveExtensionIcon(extension, new_index);
SchedulePaint();
}
@@ -776,8 +790,8 @@ int BrowserActionsContainer::IconHeight() {
return icon_height;
}
-void BrowserActionsContainer::BrowserActionAdded(const Extension* extension,
- int index) {
+void BrowserActionsContainer::ToolbarExtensionAdded(const Extension* extension,
+ int index) {
#if defined(DEBUG)
for (size_t i = 0; i < browser_action_views_.size(); ++i) {
DCHECK(browser_action_views_[i]->extension() != extension) <<
@@ -795,7 +809,11 @@ void BrowserActionsContainer::BrowserActionAdded(const Extension* extension,
// Add the new browser action to the vector and the view hierarchy.
if (profile_->IsOffTheRecord())
index = model_->OriginalIndexToIncognito(index);
- BrowserActionView* view = new BrowserActionView(extension, browser_, this);
+ BrowserActionView* view =
+ new BrowserActionView(extension,
+ GetExtensionAction(*extension, profile_),
+ browser_,
+ this);
browser_action_views_.insert(browser_action_views_.begin() + index, view);
AddChildViewAt(view, index);
@@ -816,7 +834,8 @@ void BrowserActionsContainer::BrowserActionAdded(const Extension* extension,
}
}
-void BrowserActionsContainer::BrowserActionRemoved(const Extension* extension) {
+void BrowserActionsContainer::ToolbarExtensionRemoved(
+ const Extension* extension) {
CloseOverflowMenu();
size_t visible_actions = VisibleBrowserActionsAfterAnimation();
@@ -851,8 +870,8 @@ void BrowserActionsContainer::BrowserActionRemoved(const Extension* extension) {
}
}
-void BrowserActionsContainer::BrowserActionMoved(const Extension* extension,
- int index) {
+void BrowserActionsContainer::ToolbarExtensionMoved(const Extension* extension,
+ int index) {
if (!ShouldDisplayBrowserAction(extension))
return;
@@ -867,16 +886,17 @@ void BrowserActionsContainer::BrowserActionMoved(const Extension* extension,
SchedulePaint();
}
-bool BrowserActionsContainer::BrowserActionShowPopup(
+bool BrowserActionsContainer::ShowExtensionActionPopup(
const Extension* extension) {
return ShowPopupForExtension(extension, false, false);
}
-void BrowserActionsContainer::VisibleCountChanged() {
+void BrowserActionsContainer::ToolbarVisibleCountChanged() {
SetContainerWidth();
}
-void BrowserActionsContainer::HighlightModeChanged(bool is_highlighting) {
+void BrowserActionsContainer::ToolbarHighlightModeChanged(
+ bool is_highlighting) {
// The visual highlighting is done in OnPaint(). It's a bit of a pain that
// we delete and recreate everything here, but that's how it's done in
// BrowserActionMoved(), too. If we want to optimize it, we could move the
diff --git a/chrome/browser/ui/views/toolbar/browser_actions_container.h b/chrome/browser/ui/views/toolbar/browser_actions_container.h
index f704ead..5d789ba7ee 100644
--- a/chrome/browser/ui/views/toolbar/browser_actions_container.h
+++ b/chrome/browser/ui/views/toolbar/browser_actions_container.h
@@ -290,16 +290,16 @@ class BrowserActionsContainer
static int IconHeight();
// extensions::ExtensionToolbarModel::Observer implementation.
- virtual void BrowserActionAdded(const extensions::Extension* extension,
- int index) OVERRIDE;
- virtual void BrowserActionRemoved(
+ virtual void ToolbarExtensionAdded(const extensions::Extension* extension,
+ int index) OVERRIDE;
+ virtual void ToolbarExtensionRemoved(
const extensions::Extension* extension) OVERRIDE;
- virtual void BrowserActionMoved(const extensions::Extension* extension,
- int index) OVERRIDE;
- virtual bool BrowserActionShowPopup(
+ virtual void ToolbarExtensionMoved(const extensions::Extension* extension,
+ int index) OVERRIDE;
+ virtual bool ShowExtensionActionPopup(
const extensions::Extension* extension) OVERRIDE;
- virtual void VisibleCountChanged() OVERRIDE;
- virtual void HighlightModeChanged(bool is_highlighting) OVERRIDE;
+ virtual void ToolbarVisibleCountChanged() OVERRIDE;
+ virtual void ToolbarHighlightModeChanged(bool is_highlighting) OVERRIDE;
void LoadImages();
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 000d680..8c2728d 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -965,6 +965,7 @@
'browser/extensions/extension_special_storage_policy_unittest.cc',
'browser/extensions/extension_sync_data_unittest.cc',
'browser/extensions/extension_test_message_listener_unittest.cc',
+ 'browser/extensions/extension_toolbar_model_unittest.cc',
'browser/extensions/extension_ui_unittest.cc',
'browser/extensions/extension_warning_badge_service_unittest.cc',
'browser/extensions/extension_warning_service_unittest.cc',
diff --git a/chrome/common/extensions/manifest_handlers/synthesize_browser_action_handler.cc b/chrome/common/extensions/manifest_handlers/synthesize_browser_action_handler.cc
index c824c9f..d56a383 100644
--- a/chrome/common/extensions/manifest_handlers/synthesize_browser_action_handler.cc
+++ b/chrome/common/extensions/manifest_handlers/synthesize_browser_action_handler.cc
@@ -28,12 +28,9 @@ bool SynthesizeBrowserActionHandler::Parse(Extension* extension,
if (extension->manifest()->HasKey(manifest_keys::kSynthesizeBrowserAction))
return false; // This key is reserved, no extension should be using it.
- // TODO(devlin): Make sure we don't have two icons showing for page action
- // extensions if we show page action icons in the browser action
- // toolbar.
- if (!extension->manifest()->HasKey(manifest_keys::kBrowserAction))
+ if (!extension->manifest()->HasKey(manifest_keys::kBrowserAction) &&
+ !extension->manifest()->HasKey(manifest_keys::kPageAction))
ActionInfo::SetBrowserActionInfo(extension, new ActionInfo());
-
return true;
}
generated by cgit v1.1 at 2025-04-01 15:14:20 +0000