Track permissions granted to extensions in prefs

chrome/common/extensions/extension.cc

Index: chrome/common/extensions/extension.cc
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 2bbf99bc81571580c60752476e1c1534c47e08e4..4239c744cc692af25781b589e4e80f7503e4daf1 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1129,6 +1129,37 @@ bool Extension::IsPrivilegeIncrease(const Extension* old_extension,
 }
 
 // static
+// TODO(jstritar): This can be combined with the other IsPrivilegeIncrease
+// so that extensions can remove and re-add permissions during upgrades.

[Aaron Boodman, 2010/11/12 00:05:24]

I don't understand this comment. Can you combine these now? They seem likely to
get out of date otherwise.

[jstritar, 2010/11/19 21:38:36]

Okay, I combined both IsPrivilegeIncreases and updated the calls to it from
ExtensionService::OnExtensionLoaded. As an added benefit, extensions can now
remove permissions in one upgrade, then add them back via a silent upgrade since
we know they were already granted.

In adding this, I created a static version of
Extension::HasEffectiveAccessToAllHosts. This isn't in the right spot in the
*.cc file based on the conventions we discussed in other code reviews, but none
of the static methods are, so it seemed better not to rearrange them given how
big the file is.

+bool Extension::IsPrivilegeIncrease(const std::set<std::string>* granted_apis,
+                                    const std::set<std::string>* granted_hosts,
+                                    const Extension* extension) {
+  // TODO(erikkay) This will trip when you add a new distinct hostname,
+  // but we should unique based on RCD as well.  crbug.com/57042
+  std::set<std::string> new_hosts;
+  std::set<std::string> new_hosts_only;
+
+  extension->GetEffectiveHostPermissions().GetAsStringSet(&new_hosts);
+  std::set_difference(new_hosts.begin(), new_hosts.end(),
+                      granted_hosts->begin(), granted_hosts->end(),
+                      std::inserter(new_hosts_only, new_hosts_only.end()));
+
+  if (!new_hosts_only.empty())
+    return true;
+
+  std::set<std::string> new_apis = extension->api_permissions();
+  std::set<std::string> new_apis_only;
+  std::set_difference(new_apis.begin(), new_apis.end(),
+                      granted_apis->begin(), granted_apis->end(),
+                      std::inserter(new_apis_only, new_apis_only.end()));
+
+  if (!new_apis_only.empty())
+    return true;
+
+  return false;
+}
+
+// static
 void Extension::DecodeIcon(const Extension* extension,
                            Icons icon_size,
                            scoped_ptr<SkBitmap>* result) {
@@ -1706,29 +1737,29 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_key,
         }
       }
 
-      // Otherwise, it's a host pattern permission.
+      // Check if it's a host pattern permission.
       URLPattern pattern = URLPattern(CanExecuteScriptEverywhere() ?
           URLPattern::SCHEME_ALL :
           (UserScript::kValidUserScriptSchemes |
               URLPattern::SCHEME_CHROMEUI) & ~URLPattern::SCHEME_FILE);
 
-      if (URLPattern::PARSE_SUCCESS != pattern.Parse(permission_str)) {
-        *error = ExtensionErrorUtils::FormatErrorMessage(
-            errors::kInvalidPermission, base::IntToString(i));
-        return false;
-      }
+      if (URLPattern::PARSE_SUCCESS == pattern.Parse(permission_str)) {
+        if (!CanSpecifyHostPermission(pattern)) {
+          *error = ExtensionErrorUtils::FormatErrorMessage(
+              errors::kInvalidPermissionScheme, base::IntToString(i));
+          return false;
+        }
 
-      if (!CanSpecifyHostPermission(pattern)) {
-        *error = ExtensionErrorUtils::FormatErrorMessage(
-            errors::kInvalidPermissionScheme, base::IntToString(i));
-        return false;
-      }
+        // The path component is not used for host permissions, so we force it
+        // to match all paths.
+        pattern.set_path("/*");
 
-      // The path component is not used for host permissions, so we force it to
-      // match all paths.
-      pattern.set_path("/*");
+        host_permissions_.push_back(pattern);
+      }
 
-      host_permissions_.push_back(pattern);
+      // If it's not a host permission, then it's probably an unknown API
+      // permission. Do not throw an error so extensions can retain
+      // backwards compatability.

[Aaron Boodman, 2010/11/12 00:05:24]

This is a good place to add a link to the bug, so future readers can dig deeper.

[jstritar, 2010/11/19 21:38:36]

Done.

     }
   }