[Extensions Toolbar] Add a finch config for the redesign

chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc

Index: chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc
diff --git a/chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc b/chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc
index 74eebe0fa81281e1a0306990c92b8302d6addbe9..8077a4bcf2518b5a06b710e2a13e52673004418c 100644
--- a/chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc
+++ b/chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc
@@ -21,6 +21,7 @@
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -643,32 +644,70 @@ IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest,
 
 IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest,
                        ShowPageActionWithoutPageAction) {
+  // Load an extension without a page action.
   std::string manifest_without_page_action = kDeclarativeContentManifest;
   base::ReplaceSubstringsAfterOffset(
       &manifest_without_page_action, 0, "\"page_action\": {},", "");
   ASSERT_NE(kDeclarativeContentManifest, manifest_without_page_action);
   ext_dir_.WriteManifest(manifest_without_page_action);
   ext_dir_.WriteFile(FILE_PATH_LITERAL("background.js"), kBackgroundHelpers);
-  const Extension* extension = LoadExtension(ext_dir_.unpacked_path());
-  ASSERT_TRUE(extension);
-
-  EXPECT_THAT(ExecuteScriptInBackgroundPage(
-                  extension->id(),
-                  "setRules([{\n"
-                  "  conditions: [new PageStateMatcher({\n"
-                  "                   pageUrl: {hostPrefix: \"test\"}})],\n"
-                  "  actions: [new ShowPageAction()]\n"
-                  "}], 'test_rule');\n"),
-              testing::HasSubstr("without a page action"));
-
-  content::WebContents* const tab =
-      browser()->tab_strip_model()->GetWebContentsAt(0);
-  NavigateInRenderer(tab, GURL("http://test/"));
 
-  EXPECT_EQ(NULL,
-            ExtensionActionManager::Get(browser()->profile())->
-                GetPageAction(*extension));
-  EXPECT_EQ(0u, extension_action_test_util::GetVisiblePageActionCount(tab));
+  std::string extension_id;
+
+  {
+    // Without the extension action redesign, the extension should get an error.
+    FeatureSwitch::ScopedOverride override_toolbar_redesign(

[Devlin, 2015/09/21 23:28:15]

This first block is identical to the one of the left, except for the feature
switch here.

The second block is also identical, except for the feature switch and different
"expects".

[Finnur, 2015/09/22 14:18:32]

I probably would have created a helper function instead, but since this clause
goes away when the switch goes away, this is probably fine for now.

[Devlin, 2015/09/23 00:28:35]

I was thinking about it, and there were enough differences (the switch,
reloading vs loading the extension, the test asserts, etc) that it really
wouldn't have saved much, and, like you said, it'll go away once the switch is
default.

But I can clean up one of the really ugly repeated bits, which is the hard-coded
script.  Pulled that out into a variable to make it a little nicer.

+        FeatureSwitch::extension_action_redesign(), false);
+    const Extension* extension = LoadExtension(ext_dir_.unpacked_path());
+    ASSERT_TRUE(extension);
+    extension_id = extension->id();
+
+    EXPECT_THAT(ExecuteScriptInBackgroundPage(
+                    extension->id(),
+                    "setRules([{\n"
+                    "  conditions: [new PageStateMatcher({\n"
+                    "                   pageUrl: {hostPrefix: \"test\"}})],\n"
+                    "  actions: [new ShowPageAction()]\n"
+                    "}], 'test_rule');\n"),
+                testing::HasSubstr("without a page action"));
+
+    content::WebContents* const tab =
+        browser()->tab_strip_model()->GetWebContentsAt(0);
+    NavigateInRenderer(tab, GURL("http://test/"));
+
+    // There should be no page action.
+    EXPECT_EQ(nullptr, ExtensionActionManager::Get(browser()->profile())
+                           ->GetPageAction(*extension));
+    EXPECT_EQ(0u, extension_action_test_util::GetVisiblePageActionCount(tab));
+  }
+  {
+    // With the extension action redesign, it should work normally.
+    FeatureSwitch::ScopedOverride override_toolbar_redesign(
+        FeatureSwitch::extension_action_redesign(), true);
+    ReloadExtension(extension_id);
+    const Extension* extension =
+        ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(
+            extension_id);
+    ASSERT_TRUE(extension);
+
+    EXPECT_THAT(ExecuteScriptInBackgroundPage(
+                    extension->id(),
+                    "setRules([{\n"
+                    "  conditions: [new PageStateMatcher({\n"
+                    "                   pageUrl: {hostPrefix: \"test\"}})],\n"
+                    "  actions: [new ShowPageAction()]\n"
+                    "}], 'test_rule');\n"),
+                testing::Not(testing::HasSubstr("without a page action")));
+
+    content::WebContents* const tab =
+        browser()->tab_strip_model()->GetWebContentsAt(0);
+    NavigateInRenderer(tab, GURL("http://test/"));
+
+    // There should be a page action.
+    EXPECT_TRUE(ExtensionActionManager::Get(browser()->profile())
+                    ->GetPageAction(*extension));
+    EXPECT_EQ(1u, extension_action_test_util::GetVisiblePageActionCount(tab));
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest,