fix EffectiveHostPermissions test to match implementation

also fix comment to match implementation

BUG=None
TEST=ExtensionTest.EffectiveHostPermissions

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

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

4 files changed, 13 insertions, 52 deletions

diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index ffefd74..925dde2 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -132,9 +132,11 @@ class Extension {
     // Defines the set of URLs in the extension's web content.
     ExtensionExtent extent;
 
-    // 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.
+    // The set of host permissions that the extension effectively has access to,
+    // which is a merge of host_permissions_ and all of the match patterns in
+    // any content scripts the extension has. This is used to determine which
+    // URLs have the ability to load an extension's resources via embedded
+    // chrome-extension: URLs (see extension_protocols.cc).
     ExtensionExtent effective_host_permissions;
 
     // The set of module-level APIs this extension can use.
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index fcbec3c..bbee4ba 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -736,79 +736,61 @@ static Extension* LoadManifest(const std::string& dir,
   return extension.release();
 }
 
-// TODO(erikkay): reenable this test once we actually merge overlapping host
-// permissions together.
-TEST(ExtensionTest, FAILS_EffectiveHostPermissions) {
+TEST(ExtensionTest, EffectiveHostPermissions) {
   scoped_ptr<Extension> extension;
   ExtensionExtent hosts;
 
   extension.reset(LoadManifest("effective_host_permissions", "empty.json"));
   EXPECT_EQ(0u, extension->GetEffectiveHostPermissions().patterns().size());
+  EXPECT_FALSE(hosts.ContainsURL(GURL("http://www.google.com")));
   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(hosts.ContainsURL(GURL("https://www.google.com")));
   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_TRUE(hosts.ContainsURL(GURL("http://foo.google.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "two_hosts.json"));
   hosts = extension->GetEffectiveHostPermissions();
-  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->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->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_TRUE(hosts.ContainsURL(GURL("https://google.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "two_content_scripts.json"));
   hosts = extension->GetEffectiveHostPermissions();
-  EXPECT_EQ(3u, hosts.patterns().size());
   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->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
-                               "duplicate_content_script.json"));
-  hosts = extension->GetEffectiveHostPermissions();
-  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->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_FALSE(hosts.ContainsURL(GURL("https://test/")));
+  EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts());
 
   extension.reset(LoadManifest("effective_host_permissions",
                                "all_hosts2.json"));
   hosts = extension->GetEffectiveHostPermissions();
-  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->HasEffectiveAccessToAllHosts());
@@ -816,7 +798,7 @@ TEST(ExtensionTest, FAILS_EffectiveHostPermissions) {
   extension.reset(LoadManifest("effective_host_permissions",
                                "all_hosts3.json"));
   hosts = extension->GetEffectiveHostPermissions();
-  EXPECT_EQ(2u, hosts.patterns().size());
+  EXPECT_FALSE(hosts.ContainsURL(GURL("http://test/")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("https://test/")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts());
diff --git a/chrome/test/data/extensions/effective_host_permissions/duplicate_content_script.json b/chrome/test/data/extensions/effective_host_permissions/duplicate_content_script.json
deleted file mode 100644
index 3da0e9f..0000000
--- a/chrome/test/data/extensions/effective_host_permissions/duplicate_content_script.json
+++ /dev/null
@@ -1,15 +0,0 @@
-{
-  "name": "effective host permisions test",
-  "version": "1.0",
-  "permissions": [
-    "http://*.google.com/",
-    "http://google.com/",
-    "https://google.com/"
-  ],
-  "content_scripts": [
-    {
-      "matches": ["http://www.reddit.com/", "http://google.com/"],
-      "js": ["foo.js"]
-    }
-  ]
-}
diff --git a/chrome/test/data/extensions/effective_host_permissions/duplicate_host.json b/chrome/test/data/extensions/effective_host_permissions/duplicate_host.json
deleted file mode 100644
index 1be3912..0000000
--- a/chrome/test/data/extensions/effective_host_permissions/duplicate_host.json
+++ /dev/null
@@ -1,8 +0,0 @@
-{
-  "name": "effective host permisions test",
-  "version": "1.0",
-  "permissions": [
-    "http://*.google.com/",
-    "http://google.com/"
-  ]
-}