Expand usage of platform-apps flag and permission features.
Reapplication of http://codereview.chromium.org/9837045/.
BUG=119758
TEST=added
[email protected],[email protected]
Review URL: http://codereview.chromium.org/9839105
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128838 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/api/dns/dns_apitest.cc b/chrome/browser/extensions/api/dns/dns_apitest.cc
index 508a9aa..3c97302 100644
--- a/chrome/browser/extensions/api/dns/dns_apitest.cc
+++ b/chrome/browser/extensions/api/dns/dns_apitest.cc
@@ -2,13 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "base/command_line.h"
#include "base/memory/scoped_ptr.h"
#include "base/synchronization/waitable_event.h"
#include "chrome/browser/extensions/api/dns/dns_api.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_function_test_utils.h"
-#include "chrome/common/chrome_switches.h"
#include "content/public/browser/browser_thread.h"
#include "net/base/host_resolver.h"
#include "net/base/mock_host_resolver.h"
@@ -21,7 +19,7 @@
namespace {
-class DnsApiTest : public ExtensionApiTest {
+class DnsApiTest : public PlatformAppApiTest {
public:
static const std::string kHostname;
static const std::string kAddress;
@@ -30,12 +28,6 @@
mock_host_resolver_(NULL) {
}
- virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
- ExtensionApiTest::SetUpCommandLine(command_line);
- command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
- command_line->AppendSwitch(switches::kEnablePlatformApps);
- }
-
virtual void SetUpOnMainThread() OVERRIDE {
CreateMockHostResolverOnIOThread();
extensions::DnsResolveFunction::set_host_resolver_for_testing(
@@ -105,6 +97,10 @@
} // namespace
+IN_PROC_BROWSER_TEST_F(DnsApiTest, VerifyPermissions) {
+ VerifyPermissions(test_data_dir_.AppendASCII("dns/api"));
+}
+
IN_PROC_BROWSER_TEST_F(DnsApiTest, DnsResolveIPLiteral) {
scoped_refptr<extensions::DnsResolveFunction> resolve_function(
new extensions::DnsResolveFunction());
diff --git a/chrome/browser/extensions/api/identity/identity_apitest.cc b/chrome/browser/extensions/api/identity/identity_apitest.cc
index 334c8848..3c745f6c 100644
--- a/chrome/browser/extensions/api/identity/identity_apitest.cc
+++ b/chrome/browser/extensions/api/identity/identity_apitest.cc
@@ -28,15 +28,13 @@
} // namespace
-class ExperimentalApiTest : public ExtensionApiTest {
- public:
- void SetUpCommandLine(CommandLine* command_line) {
- ExtensionApiTest::SetUpCommandLine(command_line);
- command_line->AppendSwitch(switches::kEnablePlatformApps);
- command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
- }
+class ExperimentalApiTest : public PlatformAppApiTest {
};
+IN_PROC_BROWSER_TEST_F(ExperimentalApiTest, VerifyPermissions) {
+ VerifyPermissions(test_data_dir_.AppendASCII("identity"));
+}
+
IN_PROC_BROWSER_TEST_F(ExperimentalApiTest, Identity) {
IdentityInterceptor interceptor;
OAuth2MintTokenFlow::SetInterceptorForTests(&interceptor);
diff --git a/chrome/browser/extensions/api/serial/serial_apitest.cc b/chrome/browser/extensions/api/serial/serial_apitest.cc
index e146cc9..1d71ab6 100644
--- a/chrome/browser/extensions/api/serial/serial_apitest.cc
+++ b/chrome/browser/extensions/api/serial/serial_apitest.cc
@@ -2,33 +2,29 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "base/command_line.h"
#include "base/memory/scoped_ptr.h"
#include "chrome/browser/extensions/api/serial/serial_api.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_function_test_utils.h"
#include "chrome/browser/extensions/extension_test_message_listener.h"
#include "chrome/browser/ui/browser.h"
-#include "chrome/common/chrome_switches.h"
#include "content/public/browser/browser_thread.h"
using content::BrowserThread;
namespace {
-class SerialApiTest : public ExtensionApiTest {
+class SerialApiTest : public PlatformAppApiTest {
public:
SerialApiTest() {}
-
- virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
- ExtensionApiTest::SetUpCommandLine(command_line);
- command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
- command_line->AppendSwitch(switches::kEnablePlatformApps);
- }
};
} // namespace
+IN_PROC_BROWSER_TEST_F(SerialApiTest, VerifyPermissions) {
+ VerifyPermissions(test_data_dir_.AppendASCII("serial/api"));
+}
+
IN_PROC_BROWSER_TEST_F(SerialApiTest, SerialExtension) {
ResultCatcher catcher;
catcher.RestrictToProfile(browser()->profile());
diff --git a/chrome/browser/extensions/api/socket/socket_apitest.cc b/chrome/browser/extensions/api/socket/socket_apitest.cc
index 7d8ec3d..d105325 100644
--- a/chrome/browser/extensions/api/socket/socket_apitest.cc
+++ b/chrome/browser/extensions/api/socket/socket_apitest.cc
@@ -2,15 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "base/command_line.h"
#include "base/memory/ref_counted.h"
#include "base/stringprintf.h"
#include "chrome/browser/extensions/api/socket/socket_api.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_function_test_utils.h"
+#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_test_message_listener.h"
#include "chrome/browser/ui/browser.h"
-#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "net/test/test_server.h"
@@ -22,14 +21,8 @@
const std::string kHostname = "127.0.0.1";
const int kPort = 8888;
-class SocketApiTest : public ExtensionApiTest {
+class SocketApiTest : public PlatformAppApiTest {
public:
- virtual void SetUpCommandLine(CommandLine* command_line) {
- ExtensionApiTest::SetUpCommandLine(command_line);
- command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
- command_line->AppendSwitch(switches::kEnablePlatformApps);
- }
-
static std::string GenerateCreateFunctionArgs(const std::string& protocol,
const std::string& address,
int port) {
@@ -38,6 +31,10 @@
}
};
+} // namespace
+
+IN_PROC_BROWSER_TEST_F(SocketApiTest, VerifyPermissions) {
+ VerifyPermissions(test_data_dir_.AppendASCII("socket/api"));
}
IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketUDPCreateGood) {
diff --git a/chrome/browser/extensions/extension_apitest.cc b/chrome/browser/extensions/extension_apitest.cc
index 0f2a93e..6cc6681e 100644
--- a/chrome/browser/extensions/extension_apitest.cc
+++ b/chrome/browser/extensions/extension_apitest.cc
@@ -8,9 +8,12 @@
#include "base/stringprintf.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_test_api.h"
+#include "chrome/browser/extensions/unpacked_installer.h"
+#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h"
@@ -274,3 +277,60 @@
ExtensionBrowserTest::SetUpCommandLine(command_line);
test_data_dir_ = test_data_dir_.AppendASCII("api_test");
}
+
+PlatformAppApiTest::PlatformAppApiTest()
+ : previous_command_line_(CommandLine::NO_PROGRAM) {}
+
+PlatformAppApiTest::~PlatformAppApiTest() {}
+
+void PlatformAppApiTest::SetUpCommandLine(CommandLine* command_line) {
+ ExtensionApiTest::SetUpCommandLine(command_line);
+
+ // If someone is using this class, we're going to insist on management of the
+ // relevant flags. If these flags are already set, die.
+ DCHECK(!command_line->HasSwitch(switches::kEnablePlatformApps));
+ DCHECK(!command_line->HasSwitch(switches::kEnableExperimentalExtensionApis));
+
+ // Squirrel away for potential use in VerifyPermissions.
+ //
+ // TODO(miket): I _could_ just call VerifyPermissions here instead of
+ // requiring everyone who inherits from PlatformAppApiTest to explicitly call
+ // it within a test, but that feels overbearing.
+ previous_command_line_ = *command_line;
+
+ command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
+ command_line->AppendSwitch(switches::kEnablePlatformApps);
+}
+
+void PlatformAppApiTest::VerifyPermissions(const FilePath& extension_path) {
+#if defined(OS_WIN)
+ // See http://code.google.com/p/chromium/issues/detail?id=119758.
+ //
+ // TODO(miket): investigate why WaitForExtensionLoadError() doesn't receive
+ // the expected notification on XP/Vista, but succeeds on other platforms.
+#else
+ CommandLine old_command_line(*CommandLine::ForCurrentProcess());
+ ExtensionService* service = browser()->profile()->GetExtensionService();
+
+ // Neither experimental nor platform-app flag.
+ *CommandLine::ForCurrentProcess() = previous_command_line_;
+ extensions::UnpackedInstaller::Create(service)->Load(extension_path);
+ ASSERT_TRUE(WaitForExtensionLoadError());
+
+ // Only experimental flag.
+ *CommandLine::ForCurrentProcess() = previous_command_line_;
+ CommandLine::ForCurrentProcess()->AppendSwitch(
+ switches::kEnableExperimentalExtensionApis);
+ extensions::UnpackedInstaller::Create(service)->Load(extension_path);
+ ASSERT_TRUE(WaitForExtensionLoadError());
+
+ // Only platform-app flag.
+ *CommandLine::ForCurrentProcess() = previous_command_line_;
+ CommandLine::ForCurrentProcess()->AppendSwitch(
+ switches::kEnablePlatformApps);
+ extensions::UnpackedInstaller::Create(service)->Load(extension_path);
+ ASSERT_TRUE(WaitForExtensionLoadError());
+
+ *CommandLine::ForCurrentProcess() = old_command_line;
+#endif
+}
diff --git a/chrome/browser/extensions/extension_apitest.h b/chrome/browser/extensions/extension_apitest.h
index 94393c7..5c83f89 100644
--- a/chrome/browser/extensions/extension_apitest.h
+++ b/chrome/browser/extensions/extension_apitest.h
@@ -168,4 +168,21 @@
scoped_ptr<ui_test_utils::TestWebSocketServer> websocket_server_;
};
+// PlatformAppApiTest sets up the command-line flags necessary for platform
+// apps (if any), and provides a convenience method for confirming that your
+// API requires those flags.
+class PlatformAppApiTest : public ExtensionApiTest {
+ public:
+ PlatformAppApiTest();
+ virtual ~PlatformAppApiTest();
+
+ virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE;
+
+ protected:
+ void VerifyPermissions(const FilePath& extension_path);
+
+ private:
+ CommandLine previous_command_line_;
+};
+
#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_APITEST_H_
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index c35ccb5..42e2247 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -36,6 +36,7 @@
: loaded_(false),
installed_(false),
extension_installs_observed_(0),
+ extension_load_errors_observed_(0),
target_page_action_count_(-1),
target_visible_page_action_count_(-1) {
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
@@ -425,6 +426,14 @@
WaitForExtensionHostsToLoad();
}
+bool ExtensionBrowserTest::WaitForExtensionLoadError() {
+ int before = extension_load_errors_observed_;
+ ui_test_utils::RegisterAndWait(this,
+ chrome::NOTIFICATION_EXTENSION_LOAD_ERROR,
+ content::NotificationService::AllSources());
+ return extension_load_errors_observed_ != before;
+}
+
bool ExtensionBrowserTest::WaitForExtensionCrash(
const std::string& extension_id) {
ExtensionService* service = browser()->profile()->GetExtensionService();
@@ -485,6 +494,12 @@
MessageLoopForUI::current()->Quit();
break;
+ case chrome::NOTIFICATION_EXTENSION_LOAD_ERROR:
+ VLOG(1) << "Got EXTENSION_LOAD_ERROR notification.";
+ ++extension_load_errors_observed_;
+ MessageLoopForUI::current()->Quit();
+ break;
+
case chrome::NOTIFICATION_EXTENSION_PAGE_ACTION_COUNT_CHANGED: {
LocationBarTesting* location_bar =
browser()->window()->GetLocationBar()->GetLocationBarForTesting();
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
index dd59a8b..2ae201f 100644
--- a/chrome/browser/extensions/extension_browsertest.h
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -121,6 +121,10 @@
// Waits until an extension is loaded.
void WaitForExtensionLoad();
+ // Waits for an extension load error. Returns true if the error really
+ // happened.
+ bool WaitForExtensionLoadError();
+
// Wait for the specified extension to crash. Returns true if it really
// crashed.
bool WaitForExtensionCrash(const std::string& extension_id);
@@ -137,6 +141,7 @@
FilePath test_data_dir_;
std::string last_loaded_extension_id_;
int extension_installs_observed_;
+ int extension_load_errors_observed_;
private:
// Temporary directory for testing.
diff --git a/chrome/common/extensions/api/_permission_features.json b/chrome/common/extensions/api/_permission_features.json
index a27c14c..9c83a15 100644
--- a/chrome/common/extensions/api/_permission_features.json
+++ b/chrome/common/extensions/api/_permission_features.json
@@ -59,6 +59,9 @@
"devtools": {
"extension_types": ["extension", "packaged_app", "platform_app"]
},
+ "dns": {
+ "extension_types": ["platform_app"]
+ },
"experimental": {
"extension_types": [
"extension", "packaged_app", "hosted_app", "platform_app"
@@ -79,6 +82,9 @@
"history": {
"extension_types": ["extension", "packaged_app", "platform_app"]
},
+ "identity": {
+ "extension_types": ["platform_app"]
+ },
"idle": {
"extension_types": ["extension", "packaged_app", "platform_app"]
},
@@ -134,6 +140,9 @@
"proxy": {
"extension_types": ["extension", "packaged_app", "platform_app"]
},
+ "serial": {
+ "extension_types": ["platform_app"]
+ },
"socket": {
"extension_types": ["platform_app"]
},
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index a57b8e1..7f78f81 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -23,7 +23,6 @@
#include "base/utf_string_conversions.h"
#include "base/values.h"
#include "base/version.h"
-#include "crypto/sha2.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/chrome_version_info.h"
@@ -38,6 +37,7 @@
#include "chrome/common/extensions/simple_feature_provider.h"
#include "chrome/common/extensions/user_script.h"
#include "chrome/common/url_constants.h"
+#include "crypto/sha2.h"
#include "googleurl/src/url_util.h"
#include "grit/chromium_strings.h"
#include "grit/generated_resources.h"
@@ -380,6 +380,13 @@
return NULL;
}
+ if (extension->is_platform_app() &&
+ !CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnablePlatformApps)) {
+ *utf8_error = errors::kPlatformAppFlagRequired;
+ return NULL;
+ }
+
return extension;
}
diff --git a/chrome/common/extensions/extension_manifest_constants.cc b/chrome/common/extensions/extension_manifest_constants.cc
index d1fa89e..67bef53 100644
--- a/chrome/common/extensions/extension_manifest_constants.cc
+++ b/chrome/common/extensions/extension_manifest_constants.cc
@@ -455,6 +455,10 @@
"Only one of 'browser_action', 'page_action', and 'app' can be specified.";
const char kPermissionNotAllowed[] =
"Access to permission '*' denied.";
+const char kPlatformAppFlagRequired[] =
+ "Loading platform_app extension type is turned off by default. "
+ "You can enable this type with the --enable-platform-apps "
+ "command-line flag.";
const char kReservedMessageFound[] =
"Reserved key * found in message catalog.";
#if defined(OS_CHROMEOS)
diff --git a/chrome/common/extensions/extension_manifest_constants.h b/chrome/common/extensions/extension_manifest_constants.h
index 8392858..764f901d 100644
--- a/chrome/common/extensions/extension_manifest_constants.h
+++ b/chrome/common/extensions/extension_manifest_constants.h
@@ -302,6 +302,7 @@
extern const char kMultipleOverrides[];
extern const char kNoWildCardsInPaths[];
extern const char kPermissionNotAllowed[];
+ extern const char kPlatformAppFlagRequired[];
extern const char kOneUISurfaceOnly[];
extern const char kReservedMessageFound[];
extern const char kWebContentMustBeEnabled[];
