fix host permission handling when sites add support for https for the same set of hosts. this shouldn't be treated as a privilege increase, but is.

Also, centralize the logic for permission messages in preparation for splitting this out of Extension altogether.

BUG=56794
TEST=ExtensionTest.IsPrivilegeIncrease

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

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

3 files changed, 108 insertions, 22 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index a236809..943297e 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -244,7 +244,25 @@ int Extension::GetPermissionMessageId(const std::string& permission) {
   return PermissionMap::GetSingleton()->GetPermissionMessageId(permission);
 }
 
-std::set<string16> Extension::GetPermissionMessages() {
+std::vector<string16> Extension::GetPermissionMessages() {
+  std::vector<string16> messages;
+  if (!plugins_.empty()) {
+    messages.push_back(
+        l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT2_WARNING_FULL_ACCESS));
+    return messages;
+  }
+
+  string16 host_msg = GetHostPermissionMessage();
+  if (!host_msg.empty())
+    messages.push_back(host_msg);
+
+  std::set<string16> simple_msgs = GetSimplePermissionMessages();
+  messages.insert(messages.end(), simple_msgs.begin(), simple_msgs.end());
+
+  return messages;
+}
+
+std::set<string16> Extension::GetSimplePermissionMessages() {
   std::set<string16> messages;
   std::set<std::string>::iterator i;
   for (i = api_permissions_.begin(); i != api_permissions_.end(); ++i) {
@@ -255,6 +273,56 @@ std::set<string16> Extension::GetPermissionMessages() {
   return messages;
 }
 
+std::vector<std::string> Extension::GetDistinctHosts() {
+  return GetDistinctHosts(GetEffectiveHostPermissions().patterns());
+}
+
+// static
+std::vector<std::string> Extension::GetDistinctHosts(
+    const URLPatternList& host_patterns) {
+
+  // Vector because we later want to access these by index.
+  std::vector<std::string> distinct_hosts;
+
+  for (size_t i = 0; i < host_patterns.size(); ++i) {
+    std::string candidate = host_patterns[i].host();
+    if (std::find(distinct_hosts.begin(), distinct_hosts.end(), candidate) ==
+                  distinct_hosts.end()) {
+      distinct_hosts.push_back(candidate);
+    }
+  }
+
+  return distinct_hosts;
+}
+
+string16 Extension::GetHostPermissionMessage() {
+  if (HasEffectiveAccessToAllHosts())
+    return l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT2_WARNING_ALL_HOSTS);
+
+  std::vector<std::string> hosts = GetDistinctHosts();
+  if (hosts.size() == 1) {
+    return l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT2_WARNING_1_HOST,
+                                      UTF8ToUTF16(hosts[0]));
+  } else if (hosts.size() == 2) {
+    return l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT2_WARNING_2_HOSTS,
+                                      UTF8ToUTF16(hosts[0]),
+                                      UTF8ToUTF16(hosts[1]));
+  } else if (hosts.size() == 3) {
+    return l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT2_WARNING_3_HOSTS,
+                                      UTF8ToUTF16(hosts[0]),
+                                      UTF8ToUTF16(hosts[1]),
+                                      UTF8ToUTF16(hosts[2]));
+  } else if (hosts.size() >= 4) {
+    return l10n_util::GetStringFUTF16(
+        IDS_EXTENSION_PROMPT2_WARNING_4_OR_MORE_HOSTS,
+        UTF8ToUTF16(hosts[0]),
+        UTF8ToUTF16(hosts[1]),
+        base::IntToString16(hosts.size() - 2));
+  }
+
+  return string16();
+}
+
 // static
 bool Extension::IsHostedAppPermission(const std::string& str) {
   for (size_t i = 0; i < Extension::kNumHostedAppPermissions; ++i) {
@@ -948,29 +1016,30 @@ bool Extension::IsPrivilegeIncrease(Extension* old_extension,
   if (new_extension->plugins().size() > 0)
     return true;
 
-  // If we are increasing the set of hosts we have access to, it's a privilege
-  // increase.
+  // If we are increasing the set of hosts we have access to (not
+  // counting scheme differences), it's a privilege increase.
   if (!old_extension->HasEffectiveAccessToAllHosts()) {
     if (new_extension->HasEffectiveAccessToAllHosts())
       return true;
 
-    ExtensionExtent::PatternList old_hosts =
-        old_extension->GetEffectiveHostPermissions().patterns();
-    ExtensionExtent::PatternList new_hosts =
-        new_extension->GetEffectiveHostPermissions().patterns();
-
-    std::set<URLPattern, URLPattern::EffectiveHostCompareFunctor> diff;
-    std::set_difference(new_hosts.begin(), new_hosts.end(),
-        old_hosts.begin(), old_hosts.end(),
-        std::insert_iterator<std::set<URLPattern,
-            URLPattern::EffectiveHostCompareFunctor> >(diff, diff.end()),
-        URLPattern::EffectiveHostCompare);
-    if (diff.size() > 0)
+    // 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::vector<std::string> old_hosts = old_extension->GetDistinctHosts();
+    std::vector<std::string> new_hosts = new_extension->GetDistinctHosts();
+    std::set<std::string> old_hosts_set(old_hosts.begin(), old_hosts.end());
+    std::set<std::string> new_hosts_set(new_hosts.begin(), new_hosts.end());
+    std::set<std::string> new_only;
+    std::set_difference(new_hosts_set.begin(), new_hosts_set.end(),
+                      old_hosts_set.begin(), old_hosts_set.end(),
+                      std::inserter(new_only, new_only.end()));
+    if (new_only.size())
       return true;
   }
 
-  std::set<string16> old_messages = old_extension->GetPermissionMessages();
-  std::set<string16> new_messages = new_extension->GetPermissionMessages();
+  std::set<string16> old_messages =
+      old_extension->GetSimplePermissionMessages();
+  std::set<string16> new_messages =
+      new_extension->GetSimplePermissionMessages();
   std::set<string16> new_only;
   std::set_difference(new_messages.begin(), new_messages.end(),
                       old_messages.begin(), old_messages.end(),
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 79f9156..acb8edf 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -86,11 +86,17 @@ class Extension {
   // 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();
+  // Returns the full list of permission messages that this extension
+  // should display at install time.
+  std::vector<string16> GetPermissionMessages();
+
+  // Returns the distinct hosts that should be displayed in the install UI. This
+  // discards some of the detail that is present in the manifest to make it as
+  // 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> GetDistinctHosts(
+      const URLPatternList& host_patterns);
+  std::vector<std::string> GetDistinctHosts();
 
   bool apps_enabled() const { return apps_enabled_; }
   void set_apps_enabled(bool val) { apps_enabled_ = val; }
@@ -443,6 +449,16 @@ class Extension {
   // kPermissions).
   bool IsAPIPermission(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> GetSimplePermissionMessages();
+
+  // The permission message displayed related to the host permissions for
+  // this extension.
+  string16 GetHostPermissionMessage();
+
   // The absolute path to the directory the extension is stored in.
   FilePath path_;
 
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 45b13b6..0d2b406 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -829,6 +829,7 @@ TEST(ExtensionTest, IsPrivilegeIncrease) {
     { "hosts2", false },  // http://a,http://b -> https://a,http://*.b
     { "hosts3", false },  // http://a,http://b -> http://a
     { "hosts4", true },  // http://a -> http://a,http://b
+    { "hosts5", false },  // http://a,b,c -> http://a,b,c + https://a,b,c
     { "permissions1", false },  // tabs -> tabs
     { "permissions2", true },  // tabs -> tabs,bookmarks
     { "permissions3", true },  // http://a -> http://a,tabs