Split out the bookmark permission into its own line in

ExtensionInstallUI.

Also, fix a bug where a change to the geolocation permission
wasn't triggering the privilege increase path and refactor
code to try and reduce the chance of that kind of bug in the
future.

BUG=45715

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

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

3 files changed, 36 insertions, 2 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 97ee0201..b8c5163 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -32,6 +32,7 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/common/url_constants.h"
 #include "grit/chromium_strings.h"
+#include "grit/generated_resources.h"
 #include "webkit/glue/image_decoder.h"
 
 #if defined(OS_WIN)
@@ -146,6 +147,19 @@ const char* Extension::kPermissionNames[] = {
 const size_t Extension::kNumPermissions =
     arraysize(Extension::kPermissionNames);
 
+const Extension::SimplePermissions& Extension::GetSimplePermissions() {
+  static SimplePermissions permissions;
+  if (permissions.empty()) {
+    permissions[Extension::kBookmarkPermission] =
+        l10n_util::GetStringUTF16(
+            IDS_EXTENSION_PROMPT2_WARNING_BOOKMARKS);
+    permissions[Extension::kGeolocationPermission] =
+        l10n_util::GetStringUTF16(
+            IDS_EXTENSION_PROMPT2_WARNING_GEOLOCATION);
+  }
+  return permissions;
+}
+
 Extension::~Extension() {
 }
 
@@ -850,12 +864,20 @@ bool Extension::IsPrivilegeIncrease(Extension* old_extension,
       return true;
   }
 
-  // If we're going from not having history to not having it, it's an increase.
   if (!old_extension->HasEffectiveBrowsingHistoryPermission() &&
       new_extension->HasEffectiveBrowsingHistoryPermission()) {
     return true;
   }
 
+  const SimplePermissions& simple_permissions = GetSimplePermissions();
+  for (SimplePermissions::const_iterator iter = simple_permissions.begin();
+       iter != simple_permissions.end(); ++iter) {
+    if (!old_extension->HasApiPermission(iter->first) &&
+        new_extension->HasApiPermission(iter->first)) {
+      return true;
+    }
+  }
+
   // Nothing much has changed.
   return false;
 }
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 8748deb..15aca06 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -89,6 +89,11 @@ class Extension {
   static const int kBrowserActionIconMaxSize;
 
   // Each permission is a module that the extension is permitted to use.
+  //
+  // NOTE: If you add a permission, consider also changing:
+  // - Extension::GetSimplePermissions()
+  // - Extension::IsPrivilegeIncrease()
+  // - ExtensionInstallUI::GetV2Warnings()
   static const char* kBackgroundPermission;
   static const char* kBookmarkPermission;
   static const char* kExperimentalPermission;
@@ -101,6 +106,13 @@ class Extension {
   static const char* kPermissionNames[];
   static const size_t kNumPermissions;
 
+  // A "simple permission" is one that has a one-to-one mapping with a message
+  // that is displayed in the install UI. This is in contrast to more complex
+  // permissions like http access, where the exact message displayed depends on
+  // several factors.
+  typedef std::map<std::string, string16> SimplePermissions;
+  static const SimplePermissions& GetSimplePermissions();
+
   // An NPAPI plugin included in the extension.
   struct PluginInfo {
     FilePath path;  // Path to the plugin.
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 72da48d..fdba5de 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -823,7 +823,7 @@ TEST(ExtensionTest, IsPrivilegeIncrease) {
     { "hosts3", false },  // http://a,http://b -> http://a
     { "hosts4", true },  // http://a -> http://a,http://b
     { "permissions1", false },  // tabs -> tabs
-    { "permissions2", false },  // tabs -> tabs,bookmarks
+    { "permissions2", true },  // tabs -> tabs,bookmarks
     { "permissions3", true },  // http://a -> http://a,tabs
     { "permissions4", false },  // plugin -> plugin,tabs
     { "plugin1", false },  // plugin -> plugin