Generalize permission types to make it safer for us to add new permissions and to know that we're handling installation and elevation warnings correctly.

BUG=54151
BUG=54332
TEST=ExtensionTest.IsPrivilegeIncrease (and others)

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

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

15 files changed, 197 insertions, 140 deletions

diff --git a/chrome/browser/extensions/extension_install_ui.cc b/chrome/browser/extensions/extension_install_ui.cc
index 0c5243b..91b4f44 100644
--- a/chrome/browser/extensions/extension_install_ui.cc
+++ b/chrome/browser/extensions/extension_install_ui.cc
@@ -78,7 +78,7 @@ static void GetV2Warnings(Extension* extension,
     return;
   }
 
-  if (extension->HasAccessToAllHosts()) {
+  if (extension->HasEffectiveAccessToAllHosts()) {
     warnings->push_back(
         l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT2_WARNING_ALL_HOSTS));
   } else {
@@ -110,27 +110,14 @@ static void GetV2Warnings(Extension* extension,
     }
   }
 
-  if (extension->HasEffectiveBrowsingHistoryPermission()) {
-    warnings->push_back(
-        l10n_util::GetStringUTF16(
-            IDS_EXTENSION_PROMPT2_WARNING_BROWSING_HISTORY));
-  }
-
-  const Extension::SimplePermissions& simple_permissions =
-      Extension::GetSimplePermissions();
-
-  for (Extension::SimplePermissions::const_iterator iter =
-           simple_permissions.begin();
-       iter != simple_permissions.end(); ++iter) {
-    if (extension->HasApiPermission(iter->first))
-      warnings->push_back(iter->second);
-  }
+  std::set<string16> api_messages = extension->GetPermissionMessages();
+  warnings->insert(warnings->end(), api_messages.begin(), api_messages.end());
 }
 
 }  // namespace
 
 std::vector<std::string> ExtensionInstallUI::GetDistinctHostsForDisplay(
-    const std::vector<URLPattern>& host_patterns) {
+    const URLPatternList& host_patterns) {
   // Vector because we later want to access these by index.
   std::vector<std::string> distinct_hosts;
 
diff --git a/chrome/browser/extensions/extension_install_ui.h b/chrome/browser/extensions/extension_install_ui.h
index 18481af..c5e0d81 100644
--- a/chrome/browser/extensions/extension_install_ui.h
+++ b/chrome/browser/extensions/extension_install_ui.h
@@ -11,6 +11,7 @@
 
 #include "base/string16.h"
 #include "chrome/browser/extensions/image_loading_tracker.h"
+#include "chrome/common/extensions/url_pattern.h"
 #include "gfx/native_widget_types.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
@@ -19,7 +20,6 @@ class MessageLoop;
 class Profile;
 class InfoBarDelegate;
 class TabContents;
-class URLPattern;
 
 // Displays all the UI around extension installation and uninstallation.
 class ExtensionInstallUI : public ImageLoadingTracker::Observer {
@@ -54,7 +54,7 @@ class ExtensionInstallUI : public ImageLoadingTracker::Observer {
   // easy as possible to process by users. In particular we disregard the scheme
   // and path components of URLPatterns and de-dupe the result.
   static std::vector<std::string> GetDistinctHostsForDisplay(
-      const std::vector<URLPattern>& host_patterns);
+    const URLPatternList& host_patterns);
 
   explicit ExtensionInstallUI(Profile* profile);
 
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 7cc4a91..c9271d9 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -861,10 +861,8 @@ bool ChromeURLRequestContext::CheckURLAccessToExtensionPermission(
   if (info == extension_info_.end())
     return false;
 
-  std::vector<std::string>& api_permissions = info->second->api_permissions;
-  return std::find(api_permissions.begin(),
-                   api_permissions.end(),
-                   permission_name) != api_permissions.end();
+  std::set<std::string>& api_permissions = info->second->api_permissions;
+  return api_permissions.count(permission_name) != 0;
 }
 
 bool ChromeURLRequestContext::URLIsForExtensionIcon(const GURL& url) {
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index ebae377..b7ff017 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -59,7 +59,7 @@ class ChromeURLRequestContext : public URLRequestContext {
                   bool incognito_split_mode,
                   const ExtensionExtent& extent,
                   const ExtensionExtent& effective_host_permissions,
-                  const std::vector<std::string>& api_permissions,
+                  const std::set<std::string>& api_permissions,
                   const ExtensionIconSet& icons)
         : name(name),
           path(path),
@@ -76,7 +76,7 @@ class ChromeURLRequestContext : public URLRequestContext {
     const bool incognito_split_mode;
     const ExtensionExtent extent;
     const ExtensionExtent effective_host_permissions;
-    std::vector<std::string> api_permissions;
+    std::set<std::string> api_permissions;
     ExtensionIconSet icons;
   };
 
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 009f164..a236809 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -14,6 +14,7 @@
 #include "base/file_util.h"
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
+#include "base/singleton.h"
 #include "base/stl_util-inl.h"
 #include "base/third_party/nss/blapi.h"
 #include "base/third_party/nss/sha256.h"
@@ -106,14 +107,14 @@ const char kTestModuleName[] = "test";
 // Names of modules that can be used without listing it in the permissions
 // section of the manifest.
 const char* kNonPermissionModuleNames[] = {
-    kBrowserActionModuleName,
-    kBrowserActionsModuleName,
-    kDevToolsModuleName,
-    kExtensionModuleName,
-    kI18NModuleName,
-    kPageActionModuleName,
-    kPageActionsModuleName,
-    kTestModuleName
+  kBrowserActionModuleName,
+  kBrowserActionsModuleName,
+  kDevToolsModuleName,
+  kExtensionModuleName,
+  kI18NModuleName,
+  kPageActionModuleName,
+  kPageActionsModuleName,
+  kTestModuleName
 };
 const size_t kNumNonPermissionModuleNames =
     arraysize(kNonPermissionModuleNames);
@@ -128,8 +129,39 @@ const char* kNonPermissionFunctionNames[] = {
 const size_t kNumNonPermissionFunctionNames =
     arraysize(kNonPermissionFunctionNames);
 
+
+// A map between permission name and its install warning message.
+class PermissionMap {
+ public:
+  static PermissionMap* GetSingleton() {
+    return Singleton<PermissionMap>::get();
+  }
+
+  int GetPermissionMessageId(const std::string& permission) {
+    return Extension::kPermissions[permission_map_[permission]].message_id;
+  }
+
+ private:
+  friend struct DefaultSingletonTraits<PermissionMap>;
+
+  PermissionMap() {
+    for (size_t i = 0; i < Extension::kNumPermissions; ++i)
+      permission_map_[Extension::kPermissions[i].name] = i;
+  };
+
+  ~PermissionMap() { }
+
+  std::map<const std::string, size_t> permission_map_;
+};
+
+
+// Aliased to kTabPermission for purposes of API checks, but not allowed
+// in the permissions field of the manifest.
+static const char kWindowPermission[] = "windows";
+
 }  // namespace
 
+
 const FilePath::CharType Extension::kManifestFilename[] =
     FILE_PATH_LITERAL("manifest.json");
 const FilePath::CharType Extension::kLocaleFolder[] =
@@ -173,23 +205,26 @@ const char Extension::kTabPermission[] = "tabs";
 const char Extension::kUnlimitedStoragePermission[] = "unlimitedStorage";
 const char Extension::kWebstorePrivatePermission[] = "webstorePrivate";
 
-const char* const Extension::kPermissionNames[] = {
-  Extension::kBackgroundPermission,
-  Extension::kBookmarkPermission,
-  Extension::kContextMenusPermission,
-  Extension::kCookiePermission,
-  Extension::kExperimentalPermission,
-  Extension::kGeolocationPermission,
-  Extension::kIdlePermission,
-  Extension::kHistoryPermission,
-  Extension::kNotificationPermission,
-  Extension::kProxyPermission,
-  Extension::kTabPermission,
-  Extension::kUnlimitedStoragePermission,
-  Extension::kWebstorePrivatePermission,
+// In general, all permissions should have an install message.
+// See ExtensionsTest.PermissionMessages for an explanation of each
+// exception.
+const Extension::Permission Extension::kPermissions[] = {
+  { kBackgroundPermission, 0 },
+  { kBookmarkPermission, IDS_EXTENSION_PROMPT2_WARNING_BOOKMARKS },
+  { kContextMenusPermission, 0 },
+  { kCookiePermission, 0 },
+  { kExperimentalPermission, 0 },
+  { kGeolocationPermission, IDS_EXTENSION_PROMPT2_WARNING_GEOLOCATION },
+  { kIdlePermission, 0 },
+  { kHistoryPermission, IDS_EXTENSION_PROMPT2_WARNING_BROWSING_HISTORY },
+  { kNotificationPermission, 0 },
+  { kProxyPermission, 0 },
+  { kTabPermission, IDS_EXTENSION_PROMPT2_WARNING_BROWSING_HISTORY },
+  { kUnlimitedStoragePermission, 0 },
+  { kWebstorePrivatePermission, 0 },
 };
 const size_t Extension::kNumPermissions =
-    arraysize(Extension::kPermissionNames);
+    arraysize(Extension::kPermissions);
 
 const char* const Extension::kHostedAppPermissionNames[] = {
   Extension::kBackgroundPermission,
@@ -205,17 +240,19 @@ const size_t Extension::kNumHostedAppPermissions =
 const char Extension::kOldUnlimitedStoragePermission[] = "unlimited_storage";
 
 // static
-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;
+int Extension::GetPermissionMessageId(const std::string& permission) {
+  return PermissionMap::GetSingleton()->GetPermissionMessageId(permission);
+}
+
+std::set<string16> Extension::GetPermissionMessages() {
+  std::set<string16> messages;
+  std::set<std::string>::iterator i;
+  for (i = api_permissions_.begin(); i != api_permissions_.end(); ++i) {
+    int message_id = GetPermissionMessageId(*i);
+    if (message_id)
+      messages.insert(l10n_util::GetStringUTF16(message_id));
+  }
+  return messages;
 }
 
 // static
@@ -913,8 +950,8 @@ bool Extension::IsPrivilegeIncrease(Extension* old_extension,
 
   // If we are increasing the set of hosts we have access to, it's a privilege
   // increase.
-  if (!old_extension->HasAccessToAllHosts()) {
-    if (new_extension->HasAccessToAllHosts())
+  if (!old_extension->HasEffectiveAccessToAllHosts()) {
+    if (new_extension->HasEffectiveAccessToAllHosts())
       return true;
 
     ExtensionExtent::PatternList old_hosts =
@@ -932,31 +969,21 @@ bool Extension::IsPrivilegeIncrease(Extension* old_extension,
       return true;
   }
 
-  if (!old_extension->HasEffectiveBrowsingHistoryPermission() &&
-      new_extension->HasEffectiveBrowsingHistoryPermission()) {
-    return true;
-  }
+  std::set<string16> old_messages = old_extension->GetPermissionMessages();
+  std::set<string16> new_messages = new_extension->GetPermissionMessages();
+  std::set<string16> new_only;
+  std::set_difference(new_messages.begin(), new_messages.end(),
+                      old_messages.begin(), old_messages.end(),
+                      std::inserter(new_only, new_only.end()));
 
-  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;
-    }
-  }
+  // If there are any new permission messages, then it's an increase.
+  if (!new_only.empty())
+    return true;
 
-  // Nothing much has changed.
   return false;
 }
 
 // static
-bool Extension::HasEffectiveBrowsingHistoryPermission() const {
-  return HasApiPermission(kTabPermission) ||
-      HasApiPermission(kHistoryPermission);
-}
-
-// static
 void Extension::DecodeIcon(Extension* extension,
                            Icons icon_size,
                            scoped_ptr<SkBitmap>* result) {
@@ -1508,13 +1535,13 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_key,
       if (web_extent().is_empty() || location() == Extension::COMPONENT) {
         // Check if it's a module permission.  If so, enable that permission.
         if (IsAPIPermission(permission_str)) {
-          api_permissions_.push_back(permission_str);
+          api_permissions_.insert(permission_str);
           continue;
         }
       } else {
         // Hosted apps only get access to a subset of the valid permissions.
         if (IsHostedAppPermission(permission_str)) {
-          api_permissions_.push_back(permission_str);
+          api_permissions_.insert(permission_str);
           continue;
         }
       }
@@ -1811,7 +1838,7 @@ bool Extension::CanAccessURL(const URLPattern pattern) const {
 
 // static.
 bool Extension::HasApiPermission(
-    const std::vector<std::string>& api_permissions,
+    const std::set<std::string>& api_permissions,
     const std::string& function_name) {
   std::string permission_name = function_name;
 
@@ -1828,11 +1855,10 @@ bool Extension::HasApiPermission(
     permission_name = function_name.substr(0, separator);
 
   // windows and tabs are the same permission.
-  if (permission_name == "windows")
+  if (permission_name == kWindowPermission)
     permission_name = Extension::kTabPermission;
 
-  if (std::find(api_permissions.begin(), api_permissions.end(),
-                permission_name) != api_permissions.end())
+  if (api_permissions.count(permission_name))
     return true;
 
   for (size_t i = 0; i < kNumNonPermissionModuleNames; ++i) {
@@ -1871,8 +1897,9 @@ const ExtensionExtent Extension::GetEffectiveHostPermissions() const {
   return effective_hosts;
 }
 
-bool Extension::HasAccessToAllHosts() const {
-  // Proxies effectively grant access to every site.
+bool Extension::HasEffectiveAccessToAllHosts() const {
+  // Some APIs effectively grant access to every site.  New ones should be
+  // added here.  (I'm looking at you, network API)
   if (HasApiPermission(kProxyPermission))
     return true;
 
@@ -1897,7 +1924,7 @@ bool Extension::HasAccessToAllHosts() const {
 
 bool Extension::IsAPIPermission(const std::string& str) {
   for (size_t i = 0; i < Extension::kNumPermissions; ++i) {
-    if (str == Extension::kPermissionNames[i]) {
+    if (str == Extension::kPermissions[i].name) {
       // Only allow the experimental API permission if the command line
       // flag is present, or if the extension is a component of Chrome.
       if (str == Extension::kExperimentalPermission) {
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 47df5a8..79f9156 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -31,7 +31,6 @@ class Version;
 // Represents a Chrome extension.
 class Extension {
  public:
-  typedef std::vector<URLPattern> URLPatternList;
   typedef std::map<const std::string, GURL> URLOverrideMap;
 
   // What an extension was loaded from.
@@ -77,6 +76,22 @@ class Extension {
     EXTENSION_ICON_BITTY = 16,
   };
 
+  // A permission is defined by its |name| (what is used in the manifest),
+  // and the |message_id| that's used by install/update UI.
+  struct Permission {
+    const char* const name;
+    const int message_id;
+  };
+
+  // The install message id for |permission|.  Returns 0 if none exists.
+  static int GetPermissionMessageId(const std::string& permission);
+
+  // The set of unique API install messages that the extension has.
+  // NOTE: This only includes messages related to permissions declared in the
+  // "permissions" key in the manifest.  Permissions implied from other features
+  // of the manifest, like plugins and content scripts are not included.
+  std::set<string16> GetPermissionMessages();
+
   bool apps_enabled() const { return apps_enabled_; }
   void set_apps_enabled(bool val) { apps_enabled_ = val; }
 
@@ -89,10 +104,8 @@ class Extension {
 
   // 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()
+  // NOTE: To add a new permission, define it here, and add an entry to
+  // Extension::kPermissions.
   static const char kBackgroundPermission[];
   static const char kBookmarkPermission[];
   static const char kContextMenusPermission[];
@@ -107,7 +120,7 @@ class Extension {
   static const char kUnlimitedStoragePermission[];
   static const char kWebstorePrivatePermission[];
 
-  static const char* const kPermissionNames[];
+  static const Permission kPermissions[];
   static const size_t kNumPermissions;
   static const char* const kHostedAppPermissionNames[];
   static const size_t kNumHostedAppPermissions;
@@ -116,13 +129,6 @@ class Extension {
   // still accepted as meaning the same thing as kUnlimitedStoragePermission.
   static const char kOldUnlimitedStoragePermission[];
 
-  // 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();
-
   // Returns true if the string is one of the known hosted app permissions (see
   // kHostedAppPermissionNames).
   static bool IsHostedAppPermission(const std::string& permission);
@@ -267,7 +273,7 @@ class Extension {
   const GURL& options_url() const { return options_url_; }
   const GURL& devtools_url() const { return devtools_url_; }
   const std::vector<GURL>& toolstrips() const { return toolstrips_; }
-  const std::vector<std::string>& api_permissions() const {
+  const std::set<std::string>& api_permissions() const {
     return api_permissions_;
   }
   const URLPatternList& host_permissions() const {
@@ -275,7 +281,7 @@ class Extension {
   }
 
   // Returns true if the extension has the specified API permission.
-  static bool HasApiPermission(const std::vector<std::string>& api_permissions,
+  static bool HasApiPermission(const std::set<std::string>& api_permissions,
                                const std::string& function_name);
 
   bool HasApiPermission(const std::string& function_name) const {
@@ -296,15 +302,12 @@ class Extension {
   // Whether the extension has access to the given URL.
   bool HasHostPermission(const GURL& url) const;
 
-  // Returns true if the extension effectively has access to the user's browsing
-  // history.  There are several permissions that we group together into this
-  // bucket.  For example: tabs, bookmarks, and history.
-  bool HasEffectiveBrowsingHistoryPermission() const;
-
-  // Whether the extension has access to all hosts. This is true if there is
-  // a content script that matches all hosts, or if there is a host permission
-  // for all hosts.
-  bool HasAccessToAllHosts() const;
+  // Whether the extension has effective access to all hosts. This is true if
+  // there is a content script that matches all hosts, if there is a host
+  // permission grants access to all hosts (like <all_urls>) or an api
+  // permission that effectively grants access to all hosts (e.g. proxy,
+  // network, etc.)
+  bool HasEffectiveAccessToAllHosts() const;
 
   const GURL& update_url() const { return update_url_; }
 
@@ -437,7 +440,7 @@ class Extension {
   bool ContainsNonThemeKeys(const DictionaryValue& source);
 
   // Returns true if the string is one of the known api permissions (see
-  // kPermissionNames).
+  // kPermissions).
   bool IsAPIPermission(const std::string& permission);
 
   // The absolute path to the directory the extension is stored in.
@@ -518,7 +521,7 @@ class Extension {
   bool is_theme_;
 
   // The set of module-level APIs this extension can use.
-  std::vector<std::string> api_permissions_;
+  std::set<std::string> api_permissions_;
 
   // The sites this extension has permission to talk to (using XHR, etc).
   URLPatternList host_permissions_;
diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc
index d6b47db3..a4c3a3d 100644
--- a/chrome/common/extensions/extension_manifests_unittest.cc
+++ b/chrome/common/extensions/extension_manifests_unittest.cc
@@ -318,7 +318,7 @@ TEST_F(ExtensionManifestTest, DisallowExtensionPermissions) {
   ListValue *permissions = new ListValue();
   manifest->Set(keys::kPermissions, permissions);
   for (size_t i = 0; i < Extension::kNumPermissions; i++) {
-    const char* name = Extension::kPermissionNames[i];
+    const char* name = Extension::kPermissions[i].name;
     StringValue* p = new StringValue(name);
     permissions->Clear();
     permissions->Append(p);
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index de6fc0f..45b13b6 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -739,20 +739,20 @@ TEST(ExtensionTest, EffectiveHostPermissions) {
 
   extension.reset(LoadManifest("effective_host_permissions", "empty.json"));
   EXPECT_EQ(0u, extension->GetEffectiveHostPermissions().patterns().size());
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions", "one_host.json"));
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_EQ(1u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "one_host_wildcard.json"));
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_EQ(1u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "two_hosts.json"));
@@ -760,21 +760,21 @@ TEST(ExtensionTest, EffectiveHostPermissions) {
   EXPECT_EQ(2u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.reddit.com")));
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "duplicate_host.json"));
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_EQ(1u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "https_not_considered.json"));
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_EQ(1u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "two_content_scripts.json"));
@@ -783,7 +783,7 @@ TEST(ExtensionTest, EffectiveHostPermissions) {
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.reddit.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://news.ycombinator.com")));
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "duplicate_content_script.json"));
@@ -791,14 +791,14 @@ TEST(ExtensionTest, EffectiveHostPermissions) {
   EXPECT_EQ(3u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.reddit.com")));
-  EXPECT_FALSE(extension->HasAccessToAllHosts());
+  EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "all_hosts.json"));
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_EQ(1u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://test/")));
-  EXPECT_TRUE(extension->HasAccessToAllHosts());
+  EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "all_hosts2.json"));
@@ -806,7 +806,7 @@ TEST(ExtensionTest, EffectiveHostPermissions) {
   EXPECT_EQ(2u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://test/")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
-  EXPECT_TRUE(extension->HasAccessToAllHosts());
+  EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "all_hosts3.json"));
@@ -814,11 +814,10 @@ TEST(ExtensionTest, EffectiveHostPermissions) {
   EXPECT_EQ(2u, hosts.patterns().size());
   EXPECT_TRUE(hosts.ContainsURL(GURL("https://test/")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
-  EXPECT_TRUE(extension->HasAccessToAllHosts());
+  EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts());
 }
 
-// See http://crbug.com/54332
-TEST(ExtensionTest, DISABLED_IsPrivilegeIncrease) {
+TEST(ExtensionTest, IsPrivilegeIncrease) {
   const struct {
     const char* base_name;
     bool expect_success;
@@ -861,6 +860,46 @@ TEST(ExtensionTest, DISABLED_IsPrivilegeIncrease) {
   }
 }
 
+TEST(ExtensionTest, PermissionMessages) {
+  // Ensure that all permissions that needs to show install UI actually have
+  // strings associated with them.
+
+  std::set<std::string> skip;
+
+  // These are considered "nuisance" or "trivial" permissions that don't need
+  // a prompt.
+  skip.insert(Extension::kContextMenusPermission);
+  skip.insert(Extension::kIdlePermission);
+  skip.insert(Extension::kNotificationPermission);
+  skip.insert(Extension::kUnlimitedStoragePermission);
+
+  // TODO(erikkay) add a string for this permission.
+  skip.insert(Extension::kBackgroundPermission);
+
+  // The cookie permission does nothing unless you have associated host
+  // permissions.
+  skip.insert(Extension::kCookiePermission);
+
+  // The proxy permission is warned as part of host permission checks.
+  skip.insert(Extension::kProxyPermission);
+
+  // If you've turned on the experimental command-line flag, we don't need
+  // to warn you further.
+  skip.insert(Extension::kExperimentalPermission);
+
+  // This is only usable by component extensions.
+  skip.insert(Extension::kWebstorePrivatePermission);
+
+  for (size_t i = 0; i < Extension::kNumPermissions; ++i) {
+    int message_id = Extension::kPermissions[i].message_id;
+    std::string name = Extension::kPermissions[i].name;
+    if (skip.count(name))
+      EXPECT_EQ(0, message_id) << "unexpected message_id for " << name;
+    else
+      EXPECT_NE(0, message_id) << "missing message_id for " << name;
+  }
+}
+
 // Returns a copy of |source| resized to |size| x |size|.
 static SkBitmap ResizedCopy(const SkBitmap& source, int size) {
   return skia::ImageOperations::Resize(source,
diff --git a/chrome/common/extensions/url_pattern.h b/chrome/common/extensions/url_pattern.h
index b1cda15..3fe6eb6 100644
--- a/chrome/common/extensions/url_pattern.h
+++ b/chrome/common/extensions/url_pattern.h
@@ -212,4 +212,6 @@ class URLPattern {
   mutable std::string path_escaped_;
 };
 
+typedef std::vector<URLPattern> URLPatternList;
+
 #endif  // CHROME_COMMON_EXTENSIONS_URL_PATTERN_H_
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index 78ecabd..db86235 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -750,7 +750,7 @@ IPC_BEGIN_MESSAGES(View)
   // Extension::Permissions for which elements correspond to which permissions.
   IPC_MESSAGE_CONTROL2(ViewMsg_Extension_SetAPIPermissions,
                        std::string /* extension_id */,
-                       std::vector<std::string> /* permissions */)
+                       std::set<std::string> /* permissions */)
 
   // Tell the renderer process which host permissions the given extension has.
   IPC_MESSAGE_CONTROL2(
diff --git a/chrome/renderer/extensions/extension_process_bindings.cc b/chrome/renderer/extensions/extension_process_bindings.cc
index 7026b59..a5b9fe5 100644
--- a/chrome/renderer/extensions/extension_process_bindings.cc
+++ b/chrome/renderer/extensions/extension_process_bindings.cc
@@ -58,7 +58,7 @@ namespace {
 typedef std::map< std::string, std::vector<std::string> > PageActionIdMap;
 
 // A list of permissions that are enabled for this extension.
-typedef std::vector<std::string> PermissionsList;
+typedef std::set<std::string> PermissionsList;
 
 // A map of extension ID to permissions map.
 typedef std::map<std::string, PermissionsList> ExtensionPermissionsList;
@@ -644,9 +644,10 @@ void ExtensionProcessBindings::SetPageActions(
 // static
 void ExtensionProcessBindings::SetAPIPermissions(
     const std::string& extension_id,
-    const std::vector<std::string>& permissions) {
+    const std::set<std::string>& permissions) {
   PermissionsList& permissions_list = *GetPermissionsList(extension_id);
-  permissions_list.assign(permissions.begin(), permissions.end());
+  permissions_list.clear();
+  permissions_list.insert(permissions.begin(), permissions.end());
 }
 
 // static
diff --git a/chrome/renderer/extensions/extension_process_bindings.h b/chrome/renderer/extensions/extension_process_bindings.h
index 8202c46..2cbbf4c 100644
--- a/chrome/renderer/extensions/extension_process_bindings.h
+++ b/chrome/renderer/extensions/extension_process_bindings.h
@@ -41,7 +41,7 @@ class ExtensionProcessBindings {
 
   // Sets the API permissions for a particular extension.
   static void SetAPIPermissions(const std::string& extension_id,
-                                const std::vector<std::string>& permissions);
+                                const std::set<std::string>& permissions);
 
   // Sets the host permissions for a particular extension.
   static void SetHostPermissions(const GURL& extension_url,
diff --git a/chrome/renderer/render_thread.cc b/chrome/renderer/render_thread.cc
index 6fdecc5..be4cac6 100644
--- a/chrome/renderer/render_thread.cc
+++ b/chrome/renderer/render_thread.cc
@@ -535,7 +535,7 @@ void RenderThread::OnPageActionsUpdated(
 
 void RenderThread::OnExtensionSetAPIPermissions(
     const std::string& extension_id,
-    const std::vector<std::string>& permissions) {
+    const std::set<std::string>& permissions) {
   ExtensionProcessBindings::SetAPIPermissions(extension_id, permissions);
 
   // This is called when starting a new extension page, so start the idle
diff --git a/chrome/renderer/render_thread.h b/chrome/renderer/render_thread.h
index 4572b49..6c42b18 100644
--- a/chrome/renderer/render_thread.h
+++ b/chrome/renderer/render_thread.h
@@ -263,7 +263,7 @@ class RenderThread : public RenderThreadBase,
   void OnDOMStorageEvent(const ViewMsg_DOMStorageEvent_Params& params);
   void OnExtensionSetAPIPermissions(
       const std::string& extension_id,
-      const std::vector<std::string>& permissions);
+      const std::set<std::string>& permissions);
   void OnExtensionSetHostPermissions(
       const GURL& extension_url,
       const std::vector<URLPattern>& permissions);
diff --git a/chrome/test/render_view_test.cc b/chrome/test/render_view_test.cc
index a30edfb..cd0fccd 100644
--- a/chrome/test/render_view_test.cc
+++ b/chrome/test/render_view_test.cc
@@ -98,10 +98,10 @@ void RenderViewTest::SetUp() {
   ExtensionFunctionDispatcher::GetAllFunctionNames(&names);
   ExtensionProcessBindings::SetFunctionNames(names);
 
-  std::vector<std::string> permissions(
-      Extension::kPermissionNames,
-      Extension::kPermissionNames + Extension::kNumPermissions);
-  ExtensionProcessBindings::SetAPIPermissions("", permissions);
+  std::set<std::string> all_permissions;
+  for (size_t i = 0; i < Extension::kNumPermissions; ++i)
+    all_permissions.insert(Extension::kPermissions[i].name);
+  ExtensionProcessBindings::SetAPIPermissions("", all_permissions);
 
   mock_process_.reset(new MockRenderProcess);