Loosen error reporting for hosted app permissions.

BUG=101993
TEST=https://chrome.google.com/webstore/detail/imjhdahelgojehmfmkmdfjcpfbglbfmj should install without error.


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

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

2 files changed, 49 insertions, 13 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index ba5753c..63b608a 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -2571,9 +2571,10 @@ bool Extension::ParsePermissions(const DictionaryValue* source,
           ExtensionPermissionsInfo::GetInstance()->GetByName(permission_str);
 
       if (permission != NULL) {
-        if (!CanSpecifyAPIPermission(permission, error))
+        if (CanSpecifyAPIPermission(permission, error))
+          api_permissions->insert(permission->id());
+        if (!error->empty())
           return false;
-        api_permissions->insert(permission->id());
         continue;
       }
 
@@ -2808,8 +2809,17 @@ bool Extension::CanSpecifyAPIPermission(
 
   if (is_hosted_app()) {
     if (!CanSpecifyPermissionForHostedApp(permission)) {
-      *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kPermissionNotAllowed, permission->name());
+      // Some old versions of Chrome did not return errors here and we ended up
+      // with extensions in the store containing bad data: crbug.com/101993.
+      //
+      // TODO(aa): Consider just being a lot looser when loading and installing
+      // extensions. We can be strict when packing and in development mode. Then
+      // we won't have to maintain all these tricky backward compat issues:
+      // crbug.com/102328.
+      if (creation_flags_ & STRICT_ERROR_CHECKS) {
+        *error = ExtensionErrorUtils::FormatErrorMessage(
+            errors::kPermissionNotAllowed, permission->name());
+      }
       return false;
     }
   }
diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc
index 0008ef7..046c60e 100644
--- a/chrome/common/extensions/extension_manifests_unittest.cc
+++ b/chrome/common/extensions/extension_manifests_unittest.cc
@@ -640,18 +640,44 @@ TEST_F(ExtensionManifestTest, HostedAppPermissions) {
     StringValue* p = new StringValue(name);
     permissions->Clear();
     permissions->Append(p);
-    Extension::Location location = Extension::INTERNAL;
 
-    // Many permissions are not available to hosted apps.
-    if (!permission->is_hosted_app() || permission->is_component_only()) {
+    // Some permissions are only available to component hosted apps.
+    if (permission->is_component_only()) {
       LoadAndExpectError(Manifest(manifest.get(), name),
-                         errors::kPermissionNotAllowed);
-
-      // ... unless the hosted app is a component app.
-      location = Extension::COMPONENT;
+                         errors::kPermissionNotAllowed,
+                         Extension::INTERNAL);
+      scoped_refptr<Extension> extension(
+          LoadAndExpectSuccess(Manifest(manifest.get(), name),
+                               Extension::COMPONENT));
+      EXPECT_TRUE(extension->GetActivePermissions()->HasAPIPermission(
+          permission->id()));
+
+    } else if (!permission->is_hosted_app()) {
+      // Most normal extension permissions also aren't available to hosted apps.
+      // For these, the error is only reported in strict mode for legacy
+      // reasons: crbug.com/101993.
+      LoadAndExpectError(Manifest(manifest.get(), name),
+                         errors::kPermissionNotAllowed,
+                         Extension::INTERNAL,
+                         Extension::STRICT_ERROR_CHECKS);
+      scoped_refptr<Extension> extension(
+          LoadAndExpectSuccess(Manifest(manifest.get(), name),
+                               Extension::INTERNAL));
+      EXPECT_FALSE(extension->GetActivePermissions()->HasAPIPermission(
+          permission->id()));
+
+      // These permissions are also allowed for component hosted apps.
+      extension = LoadAndExpectSuccess(Manifest(manifest.get(), name),
+                                       Extension::COMPONENT);
+      EXPECT_TRUE(extension->GetActivePermissions()->HasAPIPermission(
+          permission->id()));
+
+    } else {
+      scoped_refptr<Extension> extension(
+          LoadAndExpectSuccess(Manifest(manifest.get(), name)));
+      EXPECT_TRUE(extension->GetActivePermissions()->HasAPIPermission(
+          permission->id()));
     }
-
-    LoadAndExpectSuccess(Manifest(manifest.get(), name), location);
   }
 }