[Extensions] Default-enable the toolbar redesign on trunk

BUG=514768
TBR=sky@chromium.org (trivial c/b/ui changes)

Review URL: https://codereview.chromium.org/1761913003

Cr-Commit-Position: refs/heads/master@{#379336}

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,