Revert "Embedded Extension Options: remove flags and trunk channel restrictions"

This reverts commit f51d7453.

The UI is breaking with certain extensions and users have no
ability to work around this. Putting back behind a flag and
trunk-only restriction.

BUG=414497,474476
CC=jleichtling@chromium.org,amineer@chromium.org
TBR=kalman@chromium.org,fsamuel@chromium.org

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

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

chrome/app/generated_resources.grd

@@ -6500,6 +6500,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
          If enabled, responses to certain types of queries will appear directly in the Omnibox suggestion list.
         </message>
       </if>
+      <message name="IDS_FLAGS_ENABLE_EMBEDDED_EXTENSION_OPTIONS_NAME" desc="Name of the flag that enables embedding extension options in chrome://extensions.">
+        Enable embedded extension options.
+      </message>
+      <message name="IDS_FLAGS_ENABLE_EMBEDDED_EXTENSION_OPTIONS_DESCRIPTION" desc="Description of the flag that enables embedding extension options in chrome://extensions.">
+        Display extension options as an embedded element in chrome://extensions rather than opening a new tab.
+      </message>
       <message name="IDS_FLAGS_ENABLE_EXTENSION_ACTION_REDESIGN_NAME" desc="Name of the flag to enable the extension toolbar redesign">
         Enable extension toolbar redesign.
       </message>

chrome/browser/about_flags.cc

@@ -1853,6 +1853,13 @@ const Experiment kExperiments[] = {
     kOsDesktop,
     SINGLE_VALUE_TYPE(switches::kEnableExperimentalHotwording)
   },
+  {
+    "enable-embedded-extension-options",
+    IDS_FLAGS_ENABLE_EMBEDDED_EXTENSION_OPTIONS_NAME,
+    IDS_FLAGS_ENABLE_EMBEDDED_EXTENSION_OPTIONS_DESCRIPTION,
+    kOsDesktop,
+    SINGLE_VALUE_TYPE(extensions::switches::kEnableEmbeddedExtensionOptions)
+  },
   {
     "enable-website-settings-manager",
     IDS_FLAGS_ENABLE_WEBSITE_SETTINGS_NAME,

chrome/browser/chrome_content_browser_client.cc

@@ -1333,6 +1333,7 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches(
       extensions::switches::kAllowLegacyExtensionManifests,
       extensions::switches::kEnableAppView,
       extensions::switches::kEnableAppWindowControls,
+      extensions::switches::kEnableEmbeddedExtensionOptions,
       extensions::switches::kEnableExperimentalExtensionApis,
       extensions::switches::kEnableScriptsRequireAction,
       extensions::switches::kExtensionsOnChromeURLs,

chrome/browser/extensions/extension_webui_apitest.cc

@@ -127,6 +127,16 @@ class ExtensionWebUITest : public ExtensionApiTest {
     return frame_host;
   }
 
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    FeatureSwitch::ScopedOverride enable_options(
+        FeatureSwitch::embedded_extension_options(), true);
+    // Need to add a command line flag as well as a FeatureSwitch because the
+    // FeatureSwitch is not copied over to the renderer process from the
+    // browser process.
+    command_line->AppendSwitch(switches::kEnableEmbeddedExtensionOptions);
+    ExtensionApiTest::SetUpCommandLine(command_line);
+  }
+
   scoped_ptr<FeatureSwitch::ScopedOverride> enable_options_;
 };
 

chrome/common/extensions/api/_api_features.json

@@ -370,7 +370,7 @@
     "dependencies": ["permission:embeddedExtensionOptions"]
   }, {
     "internal": true,
-    "channel": "dev",
+    "channel": "trunk",
     "contexts": ["webui"],
     "matches": ["chrome://extensions-frame/*", "chrome://extensions/*"]
   }],

chrome/common/extensions/api/_permission_features.json

@@ -366,7 +366,7 @@
     "location": "component"
   },
   "embeddedExtensionOptions": {
-    "channel": "dev",
+    "channel": "trunk",
     "extension_types": ["extension"]
   },
   "enterprise.platformKeys": {

chrome/common/extensions/manifest_tests/extension_manifests_options_unittest.cc

@@ -46,6 +46,9 @@ TEST_F(OptionsPageManifestTest, OptionsPageInApps) {
 
 // Tests for the options_ui.page manifest field.
 TEST_F(OptionsPageManifestTest, OptionsUIPage) {
+  FeatureSwitch::ScopedOverride enable_flag(
+      FeatureSwitch::embedded_extension_options(), true);
+
   scoped_refptr<Extension> extension =
       LoadAndExpectSuccess("options_ui_page_basic.json");
   EXPECT_EQ(base::StringPrintf("chrome-extension://%s/options.html",
@@ -64,6 +67,9 @@ TEST_F(OptionsPageManifestTest, OptionsUIPage) {
 
 // Tests for the options_ui.chrome_style and options_ui.open_in_tab fields.
 TEST_F(OptionsPageManifestTest, OptionsUIChromeStyleAndOpenInTab) {
+  FeatureSwitch::ScopedOverride enable_flag(
+      FeatureSwitch::embedded_extension_options(), true);
+
   scoped_refptr<Extension> extension =
       LoadAndExpectSuccess("options_ui_flags_true.json");
   EXPECT_TRUE(OptionsPageInfo::ShouldUseChromeStyle(extension.get()));
@@ -78,3 +84,23 @@ TEST_F(OptionsPageManifestTest, OptionsUIChromeStyleAndOpenInTab) {
   EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get()));
   EXPECT_FALSE(OptionsPageInfo::ShouldOpenInTab(extension.get()));
 }
+
+// Tests for the options_ui.chrome_style and options_ui.open_in_tab fields when
+// the flag for embedded extension options is turned off. chrome_style should
+// always be false, open_in_tab should always be true.
+TEST_F(OptionsPageManifestTest, OptionsUIChromeStyleAndOpenInTabNoFlag) {
+  ASSERT_FALSE(FeatureSwitch::embedded_extension_options()->IsEnabled());
+
+  scoped_refptr<Extension> extension =
+      LoadAndExpectSuccess("options_ui_flags_true.json");
+  EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get()));
+  EXPECT_TRUE(OptionsPageInfo::ShouldOpenInTab(extension.get()));
+
+  extension = LoadAndExpectSuccess("options_ui_flags_false.json");
+  EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get()));
+  EXPECT_TRUE(OptionsPageInfo::ShouldOpenInTab(extension.get()));
+
+  extension = LoadAndExpectSuccess("options_ui_page_basic.json");
+  EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get()));
+  EXPECT_TRUE(OptionsPageInfo::ShouldOpenInTab(extension.get()));
+}

chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc

@@ -285,7 +285,8 @@ void ChromeExtensionsDispatcherDelegate::RequireAdditionalModules(
     module_system->Require("denyAppView");
   }
 
-  if (context->GetAvailability("extensionOptionsInternal").is_available()) {
+  if (extensions::FeatureSwitch::embedded_extension_options()->IsEnabled() &&
+      context->GetAvailability("extensionOptionsInternal").is_available()) {
     module_system->Require("extensionOptions");
   }
 }

extensions/browser/guest_view/extension_options/extension_options_apitest.cc

@@ -14,6 +14,20 @@ using extensions::Extension;
 using extensions::FeatureSwitch;
 
 class ExtensionOptionsApiTest : public ExtensionApiTest {
+ private:
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    ExtensionApiTest::SetUpCommandLine(command_line);
+
+    enable_options_.reset(new FeatureSwitch::ScopedOverride(
+        FeatureSwitch::embedded_extension_options(), true));
+    // Need to add a command line flag as well as a FeatureSwitch because the
+    // FeatureSwitch is not copied over to the renderer process from the
+    // browser process.
+    command_line->AppendSwitch(
+        extensions::switches::kEnableEmbeddedExtensionOptions);
+  }
+
+  scoped_ptr<FeatureSwitch::ScopedOverride> enable_options_;
 };
 
 IN_PROC_BROWSER_TEST_F(ExtensionOptionsApiTest, ExtensionCanEmbedOwnOptions) {

extensions/browser/guest_view/extension_options/extension_options_guest.cc

@@ -50,6 +50,9 @@ ExtensionOptionsGuest::~ExtensionOptionsGuest() {
 extensions::GuestViewBase* ExtensionOptionsGuest::Create(
     content::BrowserContext* browser_context,
     int guest_instance_id) {
+  if (!extensions::FeatureSwitch::embedded_extension_options()->IsEnabled()) {
+    return NULL;
+  }
   return new ExtensionOptionsGuest(browser_context, guest_instance_id);
 }
 

extensions/common/feature_switch.cc

@@ -41,6 +41,9 @@ class CommonSwitches {
             FeatureSwitch::DEFAULT_DISABLED),
         scripts_require_action(switches::kScriptsRequireAction,
                                FeatureSwitch::DEFAULT_DISABLED),
+        embedded_extension_options(
+            switches::kEmbeddedExtensionOptions,
+            FeatureSwitch::DEFAULT_DISABLED),
         app_view(switches::kAppView,
                  FeatureSwitch::DEFAULT_DISABLED),
         mime_handler_view(switches::kMimeHandlerView,
@@ -60,6 +63,7 @@ class CommonSwitches {
   FeatureSwitch enable_override_bookmarks_ui;
   FeatureSwitch extension_action_redesign;
   FeatureSwitch scripts_require_action;
+  FeatureSwitch embedded_extension_options;
   FeatureSwitch app_view;
   FeatureSwitch mime_handler_view;
 };
@@ -90,6 +94,9 @@ FeatureSwitch* FeatureSwitch::extension_action_redesign() {
 FeatureSwitch* FeatureSwitch::scripts_require_action() {
   return &g_common_switches.Get().scripts_require_action;
 }
+FeatureSwitch* FeatureSwitch::embedded_extension_options() {
+  return &g_common_switches.Get().embedded_extension_options;
+}
 FeatureSwitch* FeatureSwitch::app_view() {
   return &g_common_switches.Get().app_view;
 }

extensions/common/feature_switch.h

@@ -26,6 +26,7 @@ class FeatureSwitch {
   static FeatureSwitch* enable_override_bookmarks_ui();
   static FeatureSwitch* extension_action_redesign();
   static FeatureSwitch* scripts_require_action();
+  static FeatureSwitch* embedded_extension_options();
   static FeatureSwitch* app_view();
   static FeatureSwitch* mime_handler_view();
 

extensions/common/manifest_handlers/options_page_info.cc

@@ -112,10 +112,11 @@ scoped_ptr<OptionsPageInfo> OptionsPageInfo::Create(
     base::string16* error) {
   GURL options_page;
   bool chrome_style = false;
-  bool open_in_tab = false;
+  bool open_in_tab = !FeatureSwitch::embedded_extension_options()->IsEnabled();
 
   // Parse the options_ui object.
-  if (options_ui_value) {
+  if (options_ui_value &&
+      FeatureSwitch::embedded_extension_options()->IsEnabled()) {
     base::string16 options_ui_error;
 
     scoped_ptr<OptionsUI> options_ui =

extensions/common/switches.cc

@@ -19,6 +19,10 @@ const char kAllowLegacyExtensionManifests[] =
 // Enables the <appview> tag in platform apps.
 const char kAppView[] = "app-view";
 
+// Enables extension options to be embedded in chrome://extensions rather than
+// a new tab.
+const char kEmbeddedExtensionOptions[] = "embedded-extension-options";
+
 // Show apps windows after the first paint. Windows will be shown significantly
 // later for heavy apps loading resources synchronously but it will be
 // insignificant for apps that load most of their resources asynchronously.
@@ -31,6 +35,11 @@ const char kEnableAppView[] = "enable-app-view";
 // Enables the <window-controls> tag in platform apps.
 const char kEnableAppWindowControls[] = "enable-app-window-controls";
 
+// Hack so that feature switch can work with about_flags. See
+// kEnableScriptsRequireAction.
+const char kEnableEmbeddedExtensionOptions[] =
+    "enable-embedded-extension-options";
+
 // Enables extension APIs that are in development.
 const char kEnableExperimentalExtensionApis[] =
     "enable-experimental-extension-apis";

extensions/common/switches.h

@@ -14,9 +14,11 @@ namespace switches {
 extern const char kAllowHTTPBackgroundPage[];
 extern const char kAllowLegacyExtensionManifests[];
 extern const char kAppView[];
+extern const char kEmbeddedExtensionOptions[];
 extern const char kEnableAppsShowOnFirstPaint[];
 extern const char kEnableAppView[];
 extern const char kEnableAppWindowControls[];
+extern const char kEnableEmbeddedExtensionOptions[];
 extern const char kEnableExperimentalExtensionApis[];
 extern const char kEnableExtensionActionRedesign[];
 extern const char kEnableMimeHandlerView[];