Update of the extension install UI:

- Give the user more information about which hosts an
  extension can access.
- Remove the red severe warning because it doesn't play well
  with themes and because it adds nothing when the other
  text is more specific.
- Make the image a bit smaller.

Also integrate this new idea with the silent/not-silent update flow.

BUG=12129,12140,19582

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

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

5 files changed, 247 insertions, 113 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 1eb43e3..134ee3f 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -526,6 +526,51 @@ bool Extension::FormatPEMForFileOutput(const std::string input,
   return true;
 }
 
+// static
+// TODO(aa): A problem with this code is that we silently allow upgrades to
+// extensions that require less permissions than the current version, but then
+// we don't silently allow them to go back. In order to fix this, we would need
+// to remember the max set of permissions we ever granted a single extension.
+bool Extension::AllowSilentUpgrade(Extension* old_extension,
+                                   Extension* new_extension) {
+  // If the old extension had native code access, we don't need to go any
+  // further. Things can't get any worse.
+  if (old_extension->plugins().size() > 0)
+    return true;
+
+  // Otherwise, if the new extension has a plugin, no silent upgrade.
+  if (new_extension->plugins().size() > 0)
+    return false;
+
+  // If we are increasing the set of hosts we have access to, no silent upgrade.
+  if (!old_extension->HasAccessToAllHosts()) {
+    if (new_extension->HasAccessToAllHosts())
+      return false;
+
+    std::set<std::string> old_hosts =
+        old_extension->GetEffectiveHostPermissions();
+    std::set<std::string> new_hosts =
+        new_extension->GetEffectiveHostPermissions();
+
+    std::set<std::string> difference;
+    std::set_difference(new_hosts.begin(), new_hosts.end(),
+                        old_hosts.begin(), old_hosts.end(),
+                        std::insert_iterator<std::set<std::string> >(
+                            difference, difference.end()));
+    if (difference.size() > 0)
+      return false;
+  }
+
+  // If we're going from not having api permissions to having them, no silent
+  // upgrade.
+  if (old_extension->api_permissions().size() == 0 &&
+      new_extension->api_permissions().size() > 0)
+    return false;
+
+  // Nothing much has changed. Allow the silent upgrade.
+  return true;
+}
+
 bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
                               std::string* error) {
   if (source.HasKey(keys::kPublicKey)) {
@@ -989,25 +1034,6 @@ std::set<FilePath> Extension::GetBrowserImages() {
   return image_paths;
 }
 
-Extension::PermissionClass Extension::GetPermissionClass() {
-  // Native code can do anything. Highest class.
-  if (!plugins_.empty())
-    return PERMISSION_CLASS_FULL;
-
-  // Access to other sites means the extension can steal cookies (login data)
-  // from those sites.
-  // TODO(mpcomplete): should we only classify for host access outside the
-  // extension's origin? how?
-  if (!host_permissions_.empty() || !content_scripts_.empty())
-    return PERMISSION_CLASS_HIGH;
-
-  // Extension can access history data, bookmarks, other personal info.
-  if (!api_permissions_.empty())
-    return PERMISSION_CLASS_MEDIUM;
-
-  return PERMISSION_CLASS_LOW;
-}
-
 bool Extension::GetBackgroundPageReady() {
   return background_page_ready_ || background_url().is_empty();
 }
@@ -1028,3 +1054,41 @@ FilePath Extension::GetIconPath(Icons icon) {
     return FilePath();
   return GetResourcePath(iter->second);
 }
+
+const std::set<std::string> Extension::GetEffectiveHostPermissions() const {
+  std::set<std::string> effective_hosts;
+
+  for (HostPermissions::const_iterator host = host_permissions_.begin();
+       host != host_permissions_.end(); ++host)
+    effective_hosts.insert(host->host());
+
+  for (UserScriptList::const_iterator content_script = content_scripts_.begin();
+       content_script != content_scripts_.end(); ++content_script) {
+    UserScript::PatternList::const_iterator pattern =
+        content_script->url_patterns().begin();
+    for (; pattern != content_script->url_patterns().end(); ++pattern)
+      effective_hosts.insert(pattern->host());
+  }
+
+  return effective_hosts;
+}
+
+bool Extension::HasAccessToAllHosts() const {
+  for (HostPermissions::const_iterator host = host_permissions_.begin();
+       host != host_permissions_.end(); ++host) {
+    if (host->match_subdomains() && host->host().empty())
+      return true;
+  }
+
+  for (UserScriptList::const_iterator content_script = content_scripts_.begin();
+       content_script != content_scripts_.end(); ++content_script) {
+    UserScript::PatternList::const_iterator pattern =
+        content_script->url_patterns().begin();
+    for (; pattern != content_script->url_patterns().end(); ++pattern) {
+      if (pattern->match_subdomains() && pattern->host().empty())
+        return true;
+    }
+  }
+
+  return false;
+}
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 34685fc..ab68d60 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -23,6 +23,8 @@
 // Represents a Chrome extension.
 class Extension {
  public:
+  typedef std::vector<URLPattern> HostPermissions;
+
   // What an extension was loaded from.
   enum Location {
     INVALID,
@@ -65,15 +67,6 @@ class Extension {
   static const char* kPermissionNames[];
   static const size_t kNumPermissions;
 
-  // A classification of how dangerous an extension can be, based on what it has
-  // access to.
-  enum PermissionClass {
-    PERMISSION_CLASS_LOW = 0,  // green
-    PERMISSION_CLASS_MEDIUM,  // yellow
-    PERMISSION_CLASS_HIGH,  // orange
-    PERMISSION_CLASS_FULL,  // red
-  };
-
   struct PrivacyBlacklistInfo {
     FilePath path;  // Path to the plain-text blacklist.
   };
@@ -175,6 +168,11 @@ class Extension {
   static bool FormatPEMForFileOutput(const std::string input,
       std::string* output, bool is_public);
 
+  // Determine whether we should allow a silent upgrade from |old_extension| to
+  // |new_extension|. If not, the user will have to approve the upgrade.
+  static bool AllowSilentUpgrade(Extension* old_extension,
+                                 Extension* new_extension);
+
   // Initialize the extension from a parsed manifest.
   // If |require_id| is true, will return an error if the "id" key is missing
   // from the value.
@@ -201,12 +199,23 @@ class Extension {
   const std::vector<PluginInfo>& plugins() const { return plugins_; }
   const GURL& background_url() const { return background_url_; }
   const std::vector<ToolstripInfo>& toolstrips() const { return toolstrips_; }
-  const std::vector<URLPattern>& host_permissions() const {
+  const HostPermissions& host_permissions() const {
     return host_permissions_;
   }
   const std::vector<std::string>& api_permissions() const {
     return api_permissions_;
   }
+
+  // Returns the set of hosts that the extension effectively has access to. This
+  // is used in the permissions UI and is a combination of the hosts accessible
+  // through content scripts and the hosts accessible through XHR.
+  const std::set<std::string> GetEffectiveHostPermissions() 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;
+
   const GURL& update_url() const { return update_url_; }
   const std::map<int, std::string>& icons() { return icons_; }
 
@@ -230,9 +239,6 @@ class Extension {
   // the browser might load (like themes and page action icons).
   std::set<FilePath> GetBrowserImages();
 
-  // Calculates and returns the permission class this extension is in.
-  PermissionClass GetPermissionClass();
-
   // Returns an absolute path to the given icon inside of the extension. Returns
   // an empty FilePath if the extension does not have that icon.
   FilePath GetIconPath(Icons icon);
@@ -359,7 +365,7 @@ class Extension {
   std::vector<std::string> api_permissions_;
 
   // The sites this extension has permission to talk to (using XHR, etc).
-  std::vector<URLPattern> host_permissions_;
+  HostPermissions host_permissions_;
 
   // The paths to the icons the extension contains mapped by their width.
   std::map<int, std::string> icons_;
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 10edbbb..f0d7cce 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -21,14 +21,6 @@ namespace errors = extension_manifest_errors;
 class ExtensionTest : public testing::Test {
 };
 
-static Value* ValueFromJSON(const std::string& json_string) {
-  std::string error;
-  JSONStringValueSerializer content_scripts(json_string);
-  Value* result = content_scripts.Deserialize(&error);
-  DCHECK(result) << error;
-  return result;
-}
-
 TEST(ExtensionTest, InitFromValueInvalid) {
 #if defined(OS_WIN)
   FilePath path(FILE_PATH_LITERAL("c:\\foo"));
@@ -485,74 +477,144 @@ TEST(ExtensionTest, MimeTypeSniffing) {
   EXPECT_EQ("application/octet-stream", result);
 }
 
-TEST(ExtensionTest, PermissionClass) {
-#if defined(OS_WIN)
-  FilePath path(FILE_PATH_LITERAL("C:\\foo"));
-#elif defined(OS_POSIX)
-  FilePath path(FILE_PATH_LITERAL("/foo"));
-#endif
-  Extension::ResetGeneratedIdCounter();
+static Extension* LoadManifest(const std::string& dir,
+                               const std::string& test_file) {
+  FilePath path;
+  PathService::Get(chrome::DIR_TEST_DATA, &path);
+  path = path.AppendASCII("extensions")
+             .AppendASCII(dir)
+             .AppendASCII(test_file);
+
+  JSONFileValueSerializer serializer(path);
+  scoped_ptr<Value> result(serializer.Deserialize(NULL));
+  if (!result.get())
+    return NULL;
 
-  Extension extension(path);
   std::string error;
-  DictionaryValue bare_manifest;
-  scoped_ptr<DictionaryValue> manifest;
-
-  // Start with a minimalist extension.
-  bare_manifest.SetString(keys::kVersion, "1.0.0.0");
-  bare_manifest.SetString(keys::kName, "my extension");
-  EXPECT_TRUE(extension.InitFromValue(bare_manifest, false, &error));
-  EXPECT_EQ(Extension::PERMISSION_CLASS_LOW, extension.GetPermissionClass());
-
-  // Toolstrips don't affect the permission class.
-  manifest.reset(static_cast<DictionaryValue*>(bare_manifest.DeepCopy()));
-  manifest->Set(keys::kToolstrips, ValueFromJSON(
-      "[\"toolstrip.html\", \"toolstrip2.html\"]"));
-  EXPECT_TRUE(extension.InitFromValue(*manifest, false, &error));
-  EXPECT_EQ(Extension::PERMISSION_CLASS_LOW, extension.GetPermissionClass());
-
-  // Requesting API permissions bumps you to medium.
-  manifest.reset(static_cast<DictionaryValue*>(bare_manifest.DeepCopy()));
-  manifest->Set(keys::kPermissions, ValueFromJSON(
-      "[\"tabs\", \"bookmarks\"]"));
-  EXPECT_TRUE(extension.InitFromValue(*manifest, false, &error));
-  EXPECT_EQ(Extension::PERMISSION_CLASS_MEDIUM, extension.GetPermissionClass());
-
-  // Adding a content script bumps you to high.
-  manifest.reset(static_cast<DictionaryValue*>(bare_manifest.DeepCopy()));
-  manifest->Set(keys::kContentScripts, ValueFromJSON(
-      "[{"
-      "  \"matches\": [\"http://*.google.com/*\"],"
-      "  \"js\": [\"script.js\"]"
-      "}]"));
-  EXPECT_TRUE(extension.InitFromValue(*manifest, false, &error));
-  EXPECT_EQ(Extension::PERMISSION_CLASS_HIGH, extension.GetPermissionClass());
-
-  // ... or asking for a host permission.
-  manifest.reset(static_cast<DictionaryValue*>(bare_manifest.DeepCopy()));
-  manifest->Set(keys::kPermissions, ValueFromJSON(
-      "[\"tabs\", \"http://google.com/*\"]"));
-  EXPECT_TRUE(extension.InitFromValue(*manifest, false, &error));
-  EXPECT_EQ(Extension::PERMISSION_CLASS_HIGH, extension.GetPermissionClass());
-
-  // Using native code (NPAPI) is automatically the max class.
-  manifest.reset(static_cast<DictionaryValue*>(bare_manifest.DeepCopy()));
-  manifest->Set(keys::kPlugins, ValueFromJSON(
-      "[{\"path\": \"harddrive_exploder.dll\"}]"));
-  EXPECT_TRUE(extension.InitFromValue(*manifest, false, &error));
-  EXPECT_EQ(Extension::PERMISSION_CLASS_FULL, extension.GetPermissionClass());
-
-  // Using everything at once should obviously be the max class as well.
-  manifest.reset(static_cast<DictionaryValue*>(bare_manifest.DeepCopy()));
-  manifest->Set(keys::kPlugins, ValueFromJSON(
-      "[{\"path\": \"harddrive_exploder.dll\"}]"));
-  manifest->Set(keys::kPermissions, ValueFromJSON(
-      "[\"tabs\", \"http://google.com/*\"]"));
-  manifest->Set(keys::kContentScripts, ValueFromJSON(
-      "[{"
-      "  \"matches\": [\"http://*.google.com/*\"],"
-      "  \"js\": [\"script.js\"]"
-      "}]"));
-  EXPECT_TRUE(extension.InitFromValue(*manifest, false, &error));
-  EXPECT_EQ(Extension::PERMISSION_CLASS_FULL, extension.GetPermissionClass());
+  scoped_ptr<Extension> extension(new Extension);
+  extension->InitFromValue(*static_cast<DictionaryValue*>(result.get()),
+                           false, &error);
+
+  result.release();
+  return extension.release();
+}
+
+TEST(ExtensionTest, EffectiveHostPermissions) {
+  scoped_ptr<Extension> extension;
+  std::set<std::string> hosts;
+
+  extension.reset(LoadManifest("effective_host_permissions", "empty.json"));
+  EXPECT_EQ(0u, extension->GetEffectiveHostPermissions().size());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions", "one_host.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(1u, hosts.size());
+  EXPECT_TRUE(hosts.find("www.google.com") != hosts.end());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "one_host_wildcard.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(1u, hosts.size());
+  EXPECT_TRUE(hosts.find("google.com") != hosts.end());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "two_hosts.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(2u, hosts.size());
+  EXPECT_TRUE(hosts.find("www.google.com") != hosts.end());
+  EXPECT_TRUE(hosts.find("www.reddit.com") != hosts.end());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "duplicate_host.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(1u, hosts.size());
+  EXPECT_TRUE(hosts.find("google.com") != hosts.end());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "https_not_considered.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(1u, hosts.size());
+  EXPECT_TRUE(hosts.find("google.com") != hosts.end());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "two_content_scripts.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(3u, hosts.size());
+  EXPECT_TRUE(hosts.find("google.com") != hosts.end());
+  EXPECT_TRUE(hosts.find("www.reddit.com") != hosts.end());
+  EXPECT_TRUE(hosts.find("news.ycombinator.com") != hosts.end());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "duplicate_content_script.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(2u, hosts.size());
+  EXPECT_TRUE(hosts.find("google.com") != hosts.end());
+  EXPECT_TRUE(hosts.find("www.reddit.com") != hosts.end());
+  EXPECT_FALSE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "all_hosts.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(1u, hosts.size());
+  EXPECT_TRUE(hosts.find("") != hosts.end());
+  EXPECT_TRUE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "all_hosts2.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(2u, hosts.size());
+  EXPECT_TRUE(hosts.find("") != hosts.end());
+  EXPECT_TRUE(hosts.find("www.google.com") != hosts.end());
+  EXPECT_TRUE(extension->HasAccessToAllHosts());
+
+  extension.reset(LoadManifest("effective_host_permissions",
+                               "all_hosts3.json"));
+  hosts = extension->GetEffectiveHostPermissions();
+  EXPECT_EQ(2u, hosts.size());
+  EXPECT_TRUE(hosts.find("") != hosts.end());
+  EXPECT_TRUE(hosts.find("www.google.com") != hosts.end());
+  EXPECT_TRUE(extension->HasAccessToAllHosts());
+}
+
+TEST(ExtensionTest, AllowSilentUpgrade) {
+  const struct {
+    const char* base_name;
+    bool expect_success;
+  } kTests[] = {
+    { "allhosts1", true },  // all -> all
+    { "allhosts2", true },  // all -> one
+    { "allhosts3", false },  // one -> all
+    { "hosts1", true },  // http://a,http://b -> http://a,http://b
+    { "hosts2", true },  // http://a,http://b -> https://a,http://*.b
+    { "hosts3", true },  // http://a,http://b -> http://a
+    { "hosts4", false },  // http://a -> http://a,http://b
+    { "permissions1", true},  // tabs -> tabs
+    { "permissions2", true},  // tabs -> tabs,bookmarks
+    { "permissions3", false},  // http://a -> http://a,tabs
+    { "permissions4", true},  // plugin -> plugin,tabs
+    { "plugin1", true},  // plugin -> plugin
+    { "plugin2", true},  // plugin -> none
+    { "plugin3", false}  // none -> plugin
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) {
+    scoped_ptr<Extension> old_extension(
+        LoadManifest("allow_silent_upgrade",
+                     std::string(kTests[i].base_name) + "_old.json"));
+    scoped_ptr<Extension> new_extension(
+        LoadManifest("allow_silent_upgrade",
+                     std::string(kTests[i].base_name) + "_new.json"));
+
+    EXPECT_EQ(kTests[i].expect_success,
+              Extension::AllowSilentUpgrade(old_extension.get(),
+                                            new_extension.get()))
+        << kTests[i].base_name;
+  }
 }
diff --git a/chrome/common/extensions/user_script.cc b/chrome/common/extensions/user_script.cc
index 22fa510..df34e52 100644
--- a/chrome/common/extensions/user_script.cc
+++ b/chrome/common/extensions/user_script.cc
@@ -14,7 +14,7 @@ bool UserScript::MatchesUrl(const GURL& url) const {
       return true;
   }
 
-  for (std::vector<URLPattern>::const_iterator pattern = url_patterns_.begin();
+  for (PatternList::const_iterator pattern = url_patterns_.begin();
        pattern != url_patterns_.end(); ++pattern) {
     if (pattern->MatchesUrl(url))
       return true;
@@ -52,7 +52,7 @@ void UserScript::Pickle(::Pickle* pickle) const {
 
   // Write url patterns.
   pickle->WriteSize(url_patterns_.size());
-  for (std::vector<URLPattern>::const_iterator pattern = url_patterns_.begin();
+  for (PatternList::const_iterator pattern = url_patterns_.begin();
        pattern != url_patterns_.end(); ++pattern) {
     pickle->WriteString(pattern->GetAsString());
   }
diff --git a/chrome/common/extensions/user_script.h b/chrome/common/extensions/user_script.h
index 57f8515..7416472 100644
--- a/chrome/common/extensions/user_script.h
+++ b/chrome/common/extensions/user_script.h
@@ -19,6 +19,8 @@ class Pickle;
 // extension.
 class UserScript {
  public:
+  typedef std::vector<URLPattern> PatternList;
+
   // Locations that user scripts can be run inside the document.
   enum RunLocation {
     DOCUMENT_START,  // After the documentElemnet is created, but before
@@ -97,7 +99,7 @@ class UserScript {
 
   // The URLPatterns, if any, that determine which pages this script runs
   // against.
-  const std::vector<URLPattern>& url_patterns() const { return url_patterns_; }
+  const PatternList& url_patterns() const { return url_patterns_; }
   void add_url_pattern(const URLPattern& pattern) {
     url_patterns_.push_back(pattern);
   }
@@ -139,7 +141,7 @@ class UserScript {
 
   // URLPatterns that determine pages to inject the script into. These are
   // only used with scripts that are part of extensions.
-  std::vector<URLPattern> url_patterns_;
+  PatternList url_patterns_;
 
   // List of js scripts defined in content_scripts
   FileList js_scripts_;