diff options
author | kalman <kalman@chromium.org> | 2014-10-17 17:50:04 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-10-18 00:50:25 +0000 |
commit | 9807701d4c6789b9df38a9a33ca28ca4de2861c5 (patch) | |
tree | 469ea825cfca69b94c717e26bcc5a4ad0f9cf889 | |
parent | ea319e730a0002e972853c996ec41c454a90e3aa (diff) | |
download | chromium_src-9807701d4c6789b9df38a9a33ca28ca4de2861c5.zip chromium_src-9807701d4c6789b9df38a9a33ca28ca4de2861c5.tar.gz chromium_src-9807701d4c6789b9df38a9a33ca28ca4de2861c5.tar.bz2 |
First stage of launch for embedded options pages. With this patch, extensions
are allowed to opt-in to having embedded options pages and/or chrome style.
The flag --enable-embedded-extension-options makes the default for embedding
options pages true unless extensions opt-out. This is still useful for
development.
BUG=414920
R=rockot@chromium.org
Review URL: https://codereview.chromium.org/667453003
Cr-Commit-Position: refs/heads/master@{#300190}
6 files changed, 34 insertions, 42 deletions
diff --git a/chrome/browser/extensions/extension_webui_apitest.cc b/chrome/browser/extensions/extension_webui_apitest.cc index 4e70950..da102bc 100644 --- a/chrome/browser/extensions/extension_webui_apitest.cc +++ b/chrome/browser/extensions/extension_webui_apitest.cc @@ -17,7 +17,6 @@ #include "extensions/browser/event_router.h" #include "extensions/common/api/test.h" #include "extensions/common/extension.h" -#include "extensions/common/feature_switch.h" #include "extensions/common/switches.h" #include "extensions/test/extension_test_message_listener.h" #include "testing/gtest/include/gtest/gtest.h" @@ -127,16 +126,6 @@ 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_; }; diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_options_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_options_unittest.cc index 03527cb..941c62e 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_options_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_options_unittest.cc @@ -65,42 +65,61 @@ TEST_F(OptionsPageManifestTest, OptionsUIPage) { RunTestcases(testcases, arraysize(testcases), EXPECT_TYPE_WARNING); } -// Tests for the options_ui.chrome_style and options_ui.open_in_tab fields. +// Tests for the options_ui.chrome_style and options_ui.open_in_tab fields with +// the flag enabled. This makes open_in_tab default to true if unspecified. TEST_F(OptionsPageManifestTest, OptionsUIChromeStyleAndOpenInTab) { FeatureSwitch::ScopedOverride enable_flag( FeatureSwitch::embedded_extension_options(), true); + // Explicitly specifying true in the manifest for options_ui.chrome_style and + // options_ui.open_in_tab sets them both to true. scoped_refptr<Extension> extension = LoadAndExpectSuccess("options_ui_flags_true.json"); EXPECT_TRUE(OptionsPageInfo::ShouldUseChromeStyle(extension.get())); EXPECT_TRUE(OptionsPageInfo::ShouldOpenInTab(extension.get())); + // Explicitly specifying false in the manifest for options_ui.chrome_style + // and options_ui.open_in_tab sets them both to false. extension = LoadAndExpectSuccess("options_ui_flags_false.json"); EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get())); EXPECT_FALSE(OptionsPageInfo::ShouldOpenInTab(extension.get())); - // If chrome_style and open_in_tab are not set, they should be false. + // Specifying an options_ui key but neither options_ui.chrome_style nor + // options_ui.open_in_tab uses the default values: false for open-in-tab, + // false for use-chrome-style. extension = LoadAndExpectSuccess("options_ui_page_basic.json"); EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get())); EXPECT_FALSE(OptionsPageInfo::ShouldOpenInTab(extension.get())); + + // Likewise specifying no options_ui key at all. + extension = LoadAndExpectSuccess("init_valid_options.json"); + 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. +// the flag for embedded extension options is turned off. This makes +// open_in_tab default to false. TEST_F(OptionsPageManifestTest, OptionsUIChromeStyleAndOpenInTabNoFlag) { ASSERT_FALSE(FeatureSwitch::embedded_extension_options()->IsEnabled()); + // These conditions are the same as OptionsUIChromeStyleAndOpenInTab, except + // when the flag is off the default for open-in-tab is true. + scoped_refptr<Extension> extension = LoadAndExpectSuccess("options_ui_flags_true.json"); - EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get())); + EXPECT_TRUE(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())); + EXPECT_FALSE(OptionsPageInfo::ShouldOpenInTab(extension.get())); extension = LoadAndExpectSuccess("options_ui_page_basic.json"); EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get())); EXPECT_TRUE(OptionsPageInfo::ShouldOpenInTab(extension.get())); + + extension = LoadAndExpectSuccess("init_valid_options.json"); + EXPECT_FALSE(OptionsPageInfo::ShouldUseChromeStyle(extension.get())); + EXPECT_TRUE(OptionsPageInfo::ShouldOpenInTab(extension.get())); } diff --git a/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc b/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc index 6d0b1e1..8640d30 100644 --- a/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc +++ b/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc @@ -268,8 +268,7 @@ void ChromeExtensionsDispatcherDelegate::RequireAdditionalModules( } } - if (extensions::FeatureSwitch::embedded_extension_options()->IsEnabled() && - context->GetAvailability("extensionOptionsInternal").is_available()) { + if (context->GetAvailability("extensionOptionsInternal").is_available()) { module_system->Require("extensionOptions"); } } diff --git a/extensions/browser/guest_view/extension_options/extension_options_apitest.cc b/extensions/browser/guest_view/extension_options/extension_options_apitest.cc index e003af9..c74a3ee 100644 --- a/extensions/browser/guest_view/extension_options/extension_options_apitest.cc +++ b/extensions/browser/guest_view/extension_options/extension_options_apitest.cc @@ -14,20 +14,6 @@ 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_; }; // crbug/415949. diff --git a/extensions/browser/guest_view/extension_options/extension_options_guest.cc b/extensions/browser/guest_view/extension_options/extension_options_guest.cc index 66c9aba..8988e1b 100644 --- a/extensions/browser/guest_view/extension_options/extension_options_guest.cc +++ b/extensions/browser/guest_view/extension_options/extension_options_guest.cc @@ -20,7 +20,6 @@ #include "extensions/common/constants.h" #include "extensions/common/extension.h" #include "extensions/common/extension_messages.h" -#include "extensions/common/feature_switch.h" #include "extensions/common/manifest_handlers/options_page_info.h" #include "extensions/common/permissions/permissions_data.h" #include "extensions/strings/grit/extensions_strings.h" @@ -51,9 +50,6 @@ 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); } diff --git a/extensions/common/manifest_handlers/options_page_info.cc b/extensions/common/manifest_handlers/options_page_info.cc index e669f0c..efcff83 100644 --- a/extensions/common/manifest_handlers/options_page_info.cc +++ b/extensions/common/manifest_handlers/options_page_info.cc @@ -111,12 +111,14 @@ scoped_ptr<OptionsPageInfo> OptionsPageInfo::Create( std::vector<InstallWarning>* install_warnings, base::string16* error) { GURL options_page; + // Chrome styling is always opt-in. bool chrome_style = false; + // Extensions can opt in or out to opening in a tab, and users can choose via + // the --embedded-extension-options flag which should be the default. bool open_in_tab = !FeatureSwitch::embedded_extension_options()->IsEnabled(); // Parse the options_ui object. - if (options_ui_value && - FeatureSwitch::embedded_extension_options()->IsEnabled()) { + if (options_ui_value) { base::string16 options_ui_error; scoped_ptr<OptionsUI> options_ui = @@ -140,9 +142,10 @@ scoped_ptr<OptionsPageInfo> OptionsPageInfo::Create( install_warnings->push_back( InstallWarning(base::UTF16ToASCII(options_parse_error))); } - chrome_style = - options_ui->chrome_style.get() && *options_ui->chrome_style; - open_in_tab = options_ui->open_in_tab.get() && *options_ui->open_in_tab; + if (options_ui->chrome_style.get()) + chrome_style = *options_ui->chrome_style; + if (options_ui->open_in_tab.get()) + open_in_tab = *options_ui->open_in_tab; } } |