Reland r105978: Report errors when extensions attempt to use

private APIs.

Revert was: http://codereview.chromium.org/8329012

Problem with Customization test was that the introduction of an error caused a
modal dialog which caused the test to time out. Removed the test because we
don't need to test this since we have general tests that ensure private APIs
are private elsewhere.

Problem with WebSocket test was that the manifest was requesting
customization permission even though it didn't use it. Introduction of error
message caused timeout same as above. In this case, just removing the
unneeded permission fixes the problem.

BUG=100489
TBR=jstritar@chromium.org


Review URL: http://codereview.chromium.org/8349017

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

4 files changed, 87 insertions, 36 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 00b4d40..384d07e 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1491,6 +1491,20 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags,
   base::i18n::AdjustStringForLocaleDirection(&localized_name);
   name_ = UTF16ToUTF8(localized_name);
 
+  // Load App settings. LoadExtent at least has to be done before
+  // ParsePermissions(), because the valid permissions depend on what type of
+  // package this is.
+  if (!LoadIsApp(manifest_value_.get(), error) ||
+      !LoadExtent(manifest_value_.get(), keys::kWebURLs,
+                  &extent_,
+                  errors::kInvalidWebURLs, errors::kInvalidWebURL,
+                  parse_strictness, error) ||
+      !EnsureNotHybridApp(manifest_value_.get(), error) ||
+      !LoadLaunchURL(manifest_value_.get(), error) ||
+      !LoadLaunchContainer(manifest_value_.get(), error)) {
+    return false;
+  }
+
   // Initialize the permissions (optional).
   ExtensionAPIPermissionSet api_permissions;
   URLPatternSet host_permissions;
@@ -1898,17 +1912,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags,
       return false;  // Failed to parse file browser actions definition.
   }
 
-  // Load App settings.
-  if (!LoadIsApp(manifest_value_.get(), error) ||
-      !LoadExtent(manifest_value_.get(), keys::kWebURLs,
-                  &extent_,
-                  errors::kInvalidWebURLs, errors::kInvalidWebURL,
-                  parse_strictness, error) ||
-      !EnsureNotHybridApp(manifest_value_.get(), error) ||
-      !LoadLaunchURL(manifest_value_.get(), error) ||
-      !LoadLaunchContainer(manifest_value_.get(), error))
-    return false;
-
+  // App isolation.
   if (api_permissions.count(ExtensionAPIPermission::kExperimental)) {
     if (!LoadAppIsolation(manifest_value_.get(), error))
       return false;
@@ -2560,14 +2564,9 @@ bool Extension::ParsePermissions(const DictionaryValue* source,
           ExtensionPermissionsInfo::GetInstance()->GetByName(permission_str);
 
       if (permission != NULL) {
-        if (CanSpecifyAPIPermission(permission, error))
-          api_permissions->insert(permission->id());
-
-        // Sometimes when you can't specify an API permission we ignore it
-        // silently. This seems like a bug. Specifically, crbug.com/100489.
-        if (!error->empty())
+        if (!CanSpecifyAPIPermission(permission, error))
           return false;
-
+        api_permissions->insert(permission->id());
         continue;
       }
 
@@ -2771,8 +2770,11 @@ bool Extension::CanSpecifyAPIPermission(
     const ExtensionAPIPermission* permission,
     std::string* error) const {
   if (permission->is_component_only()) {
-    if (!CanSpecifyComponentOnlyPermission())
+    if (!CanSpecifyComponentOnlyPermission()) {
+      *error = ExtensionErrorUtils::FormatErrorMessage(
+          errors::kPermissionNotAllowed, permission->name());
       return false;
+    }
   }
 
   if (permission->id() == ExtensionAPIPermission::kExperimental) {
@@ -2783,8 +2785,11 @@ bool Extension::CanSpecifyAPIPermission(
   }
 
   if (is_hosted_app()) {
-    if (!CanSpecifyPermissionForHostedApp(permission))
+    if (!CanSpecifyPermissionForHostedApp(permission)) {
+      *error = ExtensionErrorUtils::FormatErrorMessage(
+          errors::kPermissionNotAllowed, permission->name());
       return false;
+    }
   }
 
   return true;
diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc
index b3b60d3..e37b6de 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -413,6 +413,8 @@ const char* kNoWildCardsInPaths =
   "Wildcards are not allowed in extent URL pattern paths.";
 const char* kOneUISurfaceOnly =
     "Only one of 'browser_action', 'page_action', and 'app' can be specified.";
+const char* kPermissionNotAllowed =
+    "Access to permission '*' denied.";
 const char* kReservedMessageFound =
     "Reserved key * found in message catalog.";
 const char* kSidebarExperimental =
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index 27904c6..804fb0e 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -270,6 +270,7 @@ namespace extension_manifest_errors {
   extern const char* kMissingFile;
   extern const char* kMultipleOverrides;
   extern const char* kNoWildCardsInPaths;
+  extern const char* kPermissionNotAllowed;
   extern const char* kOneUISurfaceOnly;
   extern const char* kReservedMessageFound;
   extern const char* kSidebarExperimental;
diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc
index 7c41fd5..0008ef7 100644
--- a/chrome/common/extensions/extension_manifests_unittest.cc
+++ b/chrome/common/extensions/extension_manifests_unittest.cc
@@ -620,39 +620,82 @@ TEST_F(ExtensionManifestTest, OptionsPageInApps) {
                      errors::kInvalidOptionsPageExpectUrlInPackage);
 }
 
-TEST_F(ExtensionManifestTest, AllowUnrecognizedPermissions) {
+TEST_F(ExtensionManifestTest, HostedAppPermissions) {
   std::string error;
   scoped_ptr<DictionaryValue> manifest(
-      LoadManifestFile("valid_app.json", &error));
+      LoadManifestFile("hosted_app_absolute_options.json", &error));
   ASSERT_TRUE(manifest.get());
-  CommandLine old_command_line = *CommandLine::ForCurrentProcess();
+  ListValue* permissions = NULL;
+  ASSERT_TRUE(manifest->GetList("permissions", &permissions));
 
-  ListValue *permissions = new ListValue();
-  manifest->Set(keys::kPermissions, permissions);
   ExtensionPermissionsInfo* info = ExtensionPermissionsInfo::GetInstance();
   ExtensionAPIPermissionSet api_perms = info->GetAll();
   for (ExtensionAPIPermissionSet::iterator i = api_perms.begin();
        i != api_perms.end(); ++i) {
+    if (*i == ExtensionAPIPermission::kExperimental)
+      continue;
+
     ExtensionAPIPermission* permission = info->GetByID(*i);
     const char* name = permission->name();
     StringValue* p = new StringValue(name);
     permissions->Clear();
     permissions->Append(p);
-    std::string message_name = base::StringPrintf("permission-%s", name);
+    Extension::Location location = Extension::INTERNAL;
 
-    if (*i == ExtensionAPIPermission::kExperimental) {
-      // Experimental permission is allowed, but requires this switch.
-      CommandLine::ForCurrentProcess()->AppendSwitch(
-          switches::kEnableExperimentalExtensionApis);
+    // Many permissions are not available to hosted apps.
+    if (!permission->is_hosted_app() || permission->is_component_only()) {
+      LoadAndExpectError(Manifest(manifest.get(), name),
+                         errors::kPermissionNotAllowed);
+
+      // ... unless the hosted app is a component app.
+      location = Extension::COMPONENT;
     }
 
-    // Extensions are allowed to contain unrecognized API permissions,
-    // so there shouldn't be any errors.
-    scoped_refptr<Extension> extension;
-    extension = LoadAndExpectSuccess(
-        Manifest(manifest.get(), message_name.c_str()));
+    LoadAndExpectSuccess(Manifest(manifest.get(), name), location);
   }
-  *CommandLine::ForCurrentProcess() = old_command_line;
+}
+
+TEST_F(ExtensionManifestTest, ComponentOnlyPermission) {
+  std::string error;
+  scoped_ptr<DictionaryValue> manifest(
+      LoadManifestFile("init_valid_minimal.json", &error));
+  ASSERT_TRUE(manifest.get());
+  ListValue* permissions = new ListValue();
+  manifest->Set("permissions", permissions);
+
+  ExtensionPermissionsInfo* info = ExtensionPermissionsInfo::GetInstance();
+  ExtensionAPIPermissionSet api_perms = info->GetAll();
+  for (ExtensionAPIPermissionSet::iterator i = api_perms.begin();
+       i != api_perms.end(); ++i) {
+    if (*i == ExtensionAPIPermission::kExperimental)
+      continue;
+
+    ExtensionAPIPermission* permission = info->GetByID(*i);
+    const char* name = permission->name();
+    StringValue* p = new StringValue(name);
+    permissions->Clear();
+    permissions->Append(p);
+
+    if (!permission->is_component_only())
+      continue;
+
+    // Component-only extensions should only be enabled for component
+    // extensions.
+    LoadAndExpectError(Manifest(manifest.get(), name),
+                       errors::kPermissionNotAllowed);
+    LoadAndExpectSuccess(Manifest(manifest.get(), name),
+                         Extension::COMPONENT);
+  }
+}
+
+TEST_F(ExtensionManifestTest, AllowUnrecognizedPermissions) {
+  std::string error;
+  scoped_ptr<DictionaryValue> manifest(
+      LoadManifestFile("valid_app.json", &error));
+  ListValue* permissions = NULL;
+  ASSERT_TRUE(manifest->GetList("permissions", &permissions));
+  permissions->Append(new StringValue("not-a-valid-permission"));
+  LoadAndExpectSuccess(Manifest(manifest.get(), ""));
 }
 
 TEST_F(ExtensionManifestTest, NormalizeIconPaths) {