diff options
8 files changed, 36 insertions, 31 deletions
diff --git a/chrome/browser/extensions/api/declarative_content/content_action_unittest.cc b/chrome/browser/extensions/api/declarative_content/content_action_unittest.cc index a61a301..3484ec3 100644 --- a/chrome/browser/extensions/api/declarative_content/content_action_unittest.cc +++ b/chrome/browser/extensions/api/declarative_content/content_action_unittest.cc @@ -21,6 +21,7 @@ #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/value_builder.h" #include "ipc/ipc_message_utils.h" #include "testing/gmock/include/gmock/gmock.h" @@ -98,6 +99,9 @@ TEST(DeclarativeContentActionTest, InvalidCreation) { } TEST(DeclarativeContentActionTest, ShowPageActionWithoutPageAction) { + // Tests legacy behavior. + FeatureSwitch::ScopedOverride action_redesign_override( + FeatureSwitch::extension_action_redesign(), false); TestExtensionEnvironment env; const Extension* extension = env.MakeExtension(base::DictionaryValue()); diff --git a/chrome/browser/extensions/extension_action_icon_factory_unittest.cc b/chrome/browser/extensions/extension_action_icon_factory_unittest.cc index ae27486..5a1eb3e 100644 --- a/chrome/browser/extensions/extension_action_icon_factory_unittest.cc +++ b/chrome/browser/extensions/extension_action_icon_factory_unittest.cc @@ -211,15 +211,14 @@ TEST_P(ExtensionActionIconFactoryTest, NoIcons) { ASSERT_FALSE(browser_action->default_icon()); ASSERT_TRUE(browser_action->GetExplicitlySetIcon(0 /*tab id*/).IsEmpty()); - gfx::ImageSkia favicon = GetFavicon(); - ExtensionActionIconFactory icon_factory( profile(), extension.get(), browser_action, this); gfx::Image icon = icon_factory.GetIcon(0); EXPECT_TRUE(ImageRepsAreEqual( - favicon.GetRepresentation(1.0f), + browser_action->GetDefaultIconImage().ToImageSkia()->GetRepresentation( + 1.0f), icon.ToImageSkia()->GetRepresentation(1.0f))); } @@ -253,11 +252,12 @@ TEST_P(ExtensionActionIconFactoryTest, AfterSetIcon) { set_icon.ToImageSkia()->GetRepresentation(1.0f), icon.ToImageSkia()->GetRepresentation(1.0f))); - // It should still return favicon for another tabs. + // It should still return the default icon for another tab. icon = icon_factory.GetIcon(1); EXPECT_TRUE(ImageRepsAreEqual( - GetFavicon().GetRepresentation(1.0f), + browser_action->GetDefaultIconImage().ToImageSkia()->GetRepresentation( + 1.0f), icon.ToImageSkia()->GetRepresentation(1.0f))); } diff --git a/chrome/browser/extensions/location_bar_controller_unittest.cc b/chrome/browser/extensions/location_bar_controller_unittest.cc index d34f821..09c12d28 100644 --- a/chrome/browser/extensions/location_bar_controller_unittest.cc +++ b/chrome/browser/extensions/location_bar_controller_unittest.cc @@ -40,6 +40,8 @@ class LocationBarControllerUnitTest : public ChromeRenderViewHostTestHarness { void SetUp() override { active_script_override_.reset(new FeatureSwitch::ScopedOverride( FeatureSwitch::scripts_require_action(), true)); + extension_action_override_.reset(new FeatureSwitch::ScopedOverride( + FeatureSwitch::extension_action_redesign(), false)); ChromeRenderViewHostTestHarness::SetUp(); #if defined OS_CHROMEOS @@ -99,6 +101,9 @@ class LocationBarControllerUnitTest : public ChromeRenderViewHostTestHarness { // Since we also test that we show page actions for pending script requests, // we need to enable that feature. scoped_ptr<FeatureSwitch::ScopedOverride> active_script_override_; + + // This tests legacy page actions. + scoped_ptr<FeatureSwitch::ScopedOverride> extension_action_override_; }; // Test that the location bar gets the proper current actions. diff --git a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm index 12c0018..6ce3866 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm @@ -29,6 +29,7 @@ #include "content/public/browser/web_contents.h" #include "extensions/common/extension.h" #include "extensions/common/extension_builder.h" +#include "extensions/common/feature_switch.h" #include "extensions/common/manifest_constants.h" #include "extensions/common/value_builder.h" #import "third_party/ocmock/OCMock/OCMock.h" @@ -288,6 +289,9 @@ TEST_F(ExtensionInstalledBubbleControllerTest, // Test the layout of a bubble for an unpacked extension (which is not syncable) // and verify that the page action preview is enabled. TEST_F(ExtensionInstalledBubbleControllerTest, BubbleLayoutPageActionUnpacked) { + // Tests legacy behavior. + extensions::FeatureSwitch::ScopedOverride extension_action_override( + extensions::FeatureSwitch::extension_action_redesign(), false); // Page actions need a web contents (for the location bar to not break). AddWebContents(); diff --git a/chrome/browser/ui/toolbar/app_menu_model.cc b/chrome/browser/ui/toolbar/app_menu_model.cc index 62db93d..0703811 100644 --- a/chrome/browser/ui/toolbar/app_menu_model.cc +++ b/chrome/browser/ui/toolbar/app_menu_model.cc @@ -880,8 +880,9 @@ bool AppMenuModel::AddGlobalErrorMenuItems() { void AppMenuModel::CreateActionToolbarOverflowMenu() { // We only add the extensions overflow container if there are any icons that // aren't shown in the main container. - // browser_->window() can return null during startup. - if (browser_->window() && + // browser_->window() can return null during startup, and + // GetToolbarActionsBar() can be null in testing. + if (browser_->window() && browser_->window()->GetToolbarActionsBar() && browser_->window()->GetToolbarActionsBar()->NeedsOverflow()) { #if defined(OS_MACOSX) // There's a bug in AppKit menus, where if a menu item with a custom view diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc index 64ef450..a84530b 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc +++ b/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc @@ -96,10 +96,8 @@ ToolbarActionsBarUnitTest::ToolbarActionsBarUnitTest(bool use_redesign) ToolbarActionsBarUnitTest::~ToolbarActionsBarUnitTest() {} void ToolbarActionsBarUnitTest::SetUp() { - if (use_redesign_) { - redesign_switch_.reset(new extensions::FeatureSwitch::ScopedOverride( - extensions::FeatureSwitch::extension_action_redesign(), true)); - } + redesign_switch_.reset(new extensions::FeatureSwitch::ScopedOverride( + extensions::FeatureSwitch::extension_action_redesign(), use_redesign_)); BrowserWithTestWindowTest::SetUp(); // The toolbar typically displays extension icons, so create some extension diff --git a/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc b/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc index ad31804..0523060 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc +++ b/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc @@ -419,37 +419,26 @@ void ToolbarActionsModelUnitTest::SetMockActionsFactory( TEST_F(ToolbarActionsModelUnitTest, BasicToolbarActionsModelTest) { Init(); - // Load an extension with no browser action. - scoped_refptr<const extensions::Extension> extension1 = - extensions::extension_action_test_util::CreateActionExtension( - "no_action", extensions::extension_action_test_util::NO_ACTION); - ASSERT_TRUE(AddExtension(extension1)); - - // This extension should not be in the model (has no browser action). - EXPECT_EQ(0u, observer()->inserted_count()); - EXPECT_EQ(0u, num_toolbar_items()); - EXPECT_EQ(std::string(), GetActionIdAtIndex(0u)); - // Load an extension with a browser action. - scoped_refptr<const extensions::Extension> extension2 = + scoped_refptr<const extensions::Extension> extension = extensions::extension_action_test_util::CreateActionExtension( "browser_action", extensions::extension_action_test_util::BROWSER_ACTION); - ASSERT_TRUE(AddExtension(extension2)); + ASSERT_TRUE(AddExtension(extension)); // We should now find our extension in the model. EXPECT_EQ(1u, observer()->inserted_count()); EXPECT_EQ(1u, num_toolbar_items()); - EXPECT_EQ(extension2->id(), GetActionIdAtIndex(0u)); + EXPECT_EQ(extension->id(), GetActionIdAtIndex(0u)); // Should be a no-op, but still fires the events. - toolbar_model()->MoveActionIcon(extension2->id(), 0); + toolbar_model()->MoveActionIcon(extension->id(), 0); EXPECT_EQ(1u, observer()->moved_count()); EXPECT_EQ(1u, num_toolbar_items()); - EXPECT_EQ(extension2->id(), GetActionIdAtIndex(0u)); + EXPECT_EQ(extension->id(), GetActionIdAtIndex(0u)); // Remove the extension and verify. - ASSERT_TRUE(RemoveExtension(extension2)); + ASSERT_TRUE(RemoveExtension(extension)); EXPECT_EQ(1u, observer()->removed_count()); EXPECT_EQ(0u, num_toolbar_items()); EXPECT_EQ(std::string(), GetActionIdAtIndex(0u)); @@ -910,7 +899,9 @@ TEST_F(ToolbarActionsModelUnitTest, ActionsToolbarSizeAfterPrefChange) { // Test that, in the absence of the extension-action-redesign switch, the // model only contains extensions with browser actions and component actions. -TEST_F(ToolbarActionsModelUnitTest, TestToolbarExtensionTypesNoSwitch) { +TEST_F(ToolbarActionsModelUnitTest, TestToolbarExtensionTypesDisabledSwitch) { + extensions::FeatureSwitch::ScopedOverride enable_redesign( + extensions::FeatureSwitch::extension_action_redesign(), false); Init(); ASSERT_TRUE(AddActionExtensions()); @@ -921,7 +912,7 @@ TEST_F(ToolbarActionsModelUnitTest, TestToolbarExtensionTypesNoSwitch) { // 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(ToolbarActionsModelUnitTest, TestToolbarExtensionTypesSwitch) { +TEST_F(ToolbarActionsModelUnitTest, TestToolbarExtensionTypesEnabledSwitch) { extensions::FeatureSwitch::ScopedOverride enable_redesign( extensions::FeatureSwitch::extension_action_redesign(), true); Init(); @@ -977,6 +968,8 @@ TEST_F(ToolbarActionsModelUnitTest, TestToolbarExtensionTypesSwitch) { // Test that hiding actions on the toolbar results in their removal from the // model when the redesign switch is not enabled. TEST_F(ToolbarActionsModelUnitTest, ActionsToolbarActionsVisibilityNoSwitch) { + extensions::FeatureSwitch::ScopedOverride enable_redesign( + extensions::FeatureSwitch::extension_action_redesign(), false); Init(); extensions::ExtensionActionAPI* action_api = diff --git a/extensions/common/feature_switch.cc b/extensions/common/feature_switch.cc index 88301f1..36fb3857 100644 --- a/extensions/common/feature_switch.cc +++ b/extensions/common/feature_switch.cc @@ -52,7 +52,7 @@ class CommonSwitches { FeatureSwitch::DEFAULT_DISABLED), extension_action_redesign(switches::kExtensionActionRedesign, kExtensionActionRedesignExperiment, - FeatureSwitch::DEFAULT_DISABLED), + FeatureSwitch::DEFAULT_ENABLED), extension_action_redesign_override(switches::kExtensionActionRedesign, FeatureSwitch::DEFAULT_ENABLED), scripts_require_action(switches::kScriptsRequireAction, |