Expand usage of platform-apps flag and permission features.

Reapplication of http://codereview.chromium.org/9834022/ with the
VerifyPermissions test #ifdefed out for Windows.

BUG=119758
TEST=added

Review URL: https://chromiumcodereview.appspot.com/9837045

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128610 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 133 insertions, 28 deletions

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 @@ using extension_function_test_utils::RunFunctionAndReturnResult;
 
 namespace {
 
-class DnsApiTest : public ExtensionApiTest {
+class DnsApiTest : public PlatformAppApiTest {
  public:
   static const std::string kHostname;
   static const std::string kAddress;
@@ -30,12 +28,6 @@ class DnsApiTest : public ExtensionApiTest {
                  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 @@ const std::string DnsApiTest::kAddress = "9.8.7.6";
 
 }  // 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/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 @@ namespace {
 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 @@ class SocketApiTest : public ExtensionApiTest {
   }
 };
 
+}  // 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..6cc6681 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 @@ void ExtensionApiTest::SetUpCommandLine(CommandLine* command_line) {
   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 @@ class ExtensionApiTest : public ExtensionBrowserTest {
   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..46d7acc 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -36,6 +36,7 @@ ExtensionBrowserTest::ExtensionBrowserTest()
     : 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,15 @@ void ExtensionBrowserTest::WaitForExtensionLoad() {
   WaitForExtensionHostsToLoad();
 }
 
+bool ExtensionBrowserTest::WaitForExtensionLoadError() {
+  int before = extension_load_errors_observed_;
+  ui_test_utils::RegisterAndWait(this,
+                                 chrome::NOTIFICATION_EXTENSION_LOAD_ERROR,
+                                 content::NotificationService::AllSources());
+  WaitForExtensionHostsToLoad();
+  return extension_load_errors_observed_ != before;
+}
+
 bool ExtensionBrowserTest::WaitForExtensionCrash(
     const std::string& extension_id) {
   ExtensionService* service = browser()->profile()->GetExtensionService();
@@ -485,6 +495,12 @@ void ExtensionBrowserTest::Observe(
       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 @@ class ExtensionBrowserTest
   // 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 @@ class ExtensionBrowserTest
   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 425f59e..bb52471 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"
@@ -134,6 +137,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 549566d..7bca682 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 @@ scoped_refptr<Extension> Extension::Create(const FilePath& path,
     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 @@ const char kOneUISurfaceOnly[] =
     "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..764f901 100644
--- a/chrome/common/extensions/extension_manifest_constants.h
+++ b/chrome/common/extensions/extension_manifest_constants.h
@@ -302,6 +302,7 @@ namespace extension_manifest_errors {
   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[];