summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorkalman <kalman@chromium.org>2014-10-17 17:50:04 -0700
committerCommit bot <commit-bot@chromium.org>2014-10-18 00:50:25 +0000
commit9807701d4c6789b9df38a9a33ca28ca4de2861c5 (patch)
tree469ea825cfca69b94c717e26bcc5a4ad0f9cf889
parentea319e730a0002e972853c996ec41c454a90e3aa (diff)
downloadchromium_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}
-rw-r--r--chrome/browser/extensions/extension_webui_apitest.cc11
-rw-r--r--chrome/common/extensions/manifest_tests/extension_manifests_options_unittest.cc31
-rw-r--r--chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc3
-rw-r--r--extensions/browser/guest_view/extension_options/extension_options_apitest.cc14
-rw-r--r--extensions/browser/guest_view/extension_options/extension_options_guest.cc4
-rw-r--r--extensions/common/manifest_handlers/options_page_info.cc13
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;
}
}