Refactor PermissionSet to move chrome-specific host checks to ChromeExtensionsClient.

Combine GetChromeSchemeHosts and GetDistinctHostsForDisplay logic for special permission messages for chrome://favicon into GetSpecialHostPermissionWarnings.

BUG=298586

Review URL: https://codereview.chromium.org/25713006

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

12 files changed, 106 insertions, 200 deletions

diff --git a/chrome/browser/extensions/api/permissions/permissions_api.cc b/chrome/browser/extensions/api/permissions/permissions_api.cc
index 0e888eb..cf40ae1 100644
--- a/chrome/browser/extensions/api/permissions/permissions_api.cc
+++ b/chrome/browser/extensions/api/permissions/permissions_api.cc
@@ -176,15 +176,9 @@ bool PermissionsRequestFunction::RunImpl() {
     }
   }
 
-  // Filter out permissions that do not need to be listed in the optional
-  // section of the manifest.
-  scoped_refptr<PermissionSet> manifest_required_requested_permissions =
-      PermissionSet::ExcludeNotInManifestPermissions(
-          requested_permissions_.get());
-
   // The requested permissions must be defined as optional in the manifest.
   if (!PermissionsData::GetOptionalPermissions(GetExtension())
-          ->Contains(*manifest_required_requested_permissions.get())) {
+          ->Contains(*requested_permissions_.get())) {
     error_ = kNotInOptionalPermissionsError;
     return false;
   }
diff --git a/chrome/common/extensions/chrome_extensions_client.cc b/chrome/common/extensions/chrome_extensions_client.cc
index 3fa2135..fbebb8c 100644
--- a/chrome/common/extensions/chrome_extensions_client.cc
+++ b/chrome/common/extensions/chrome_extensions_client.cc
@@ -6,6 +6,13 @@
 
 #include "chrome/common/extensions/chrome_manifest_handlers.h"
 #include "chrome/common/extensions/features/base_feature_provider.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/common/url_constants.h"
+#include "extensions/common/permissions/permission_message.h"
+#include "extensions/common/url_pattern_set.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "url/gurl.h"
 
 namespace extensions {
 
@@ -33,6 +40,28 @@ void ChromeExtensionsClient::RegisterManifestHandlers() const {
   RegisterChromeManifestHandlers();
 }
 
+void ChromeExtensionsClient::FilterHostPermissions(
+    const URLPatternSet& hosts,
+    URLPatternSet* new_hosts,
+    std::set<PermissionMessage>* messages) const {
+  for (URLPatternSet::const_iterator i = hosts.begin();
+       i != hosts.end(); ++i) {
+    // Filters out every URL pattern that matches chrome:// scheme.
+    if (i->scheme() == chrome::kChromeUIScheme) {
+      // chrome://favicon is the only URL for chrome:// scheme that we
+      // want to support. We want to deprecate the "chrome" scheme.
+      // We should not add any additional "host" here.
+      if (GURL(chrome::kChromeUIFaviconURL).host() != i->host())
+        continue;
+      messages->insert(PermissionMessage(
+          PermissionMessage::kFavicon,
+          l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_FAVICON)));
+    } else {
+      new_hosts->AddPattern(*i);
+    }
+  }
+}
+
 // static
 ChromeExtensionsClient* ChromeExtensionsClient::GetInstance() {
   return g_client.Pointer();
diff --git a/chrome/common/extensions/chrome_extensions_client.h b/chrome/common/extensions/chrome_extensions_client.h
index a74a890..4f14a9c 100644
--- a/chrome/common/extensions/chrome_extensions_client.h
+++ b/chrome/common/extensions/chrome_extensions_client.h
@@ -25,6 +25,10 @@ class ChromeExtensionsClient : public ExtensionsClient {
   virtual void RegisterManifestHandlers() const OVERRIDE;
   virtual FeatureProvider* GetFeatureProviderByName(const std::string& name)
       const OVERRIDE;
+  virtual void FilterHostPermissions(
+      const URLPatternSet& hosts,
+      URLPatternSet* new_hosts,
+      std::set<PermissionMessage>* messages) const OVERRIDE;
 
   // Get the LazyInstance for ChromeExtensionsClient.
   static ChromeExtensionsClient* GetInstance();
diff --git a/chrome/common/extensions/permissions/api_permission.cc b/chrome/common/extensions/permissions/api_permission.cc
index e1ce208..d6a365e 100644
--- a/chrome/common/extensions/permissions/api_permission.cc
+++ b/chrome/common/extensions/permissions/api_permission.cc
@@ -105,10 +105,6 @@ const char* APIPermission::name() const {
   return info()->name();
 }
 
-bool APIPermission::ManifestEntryForbidden() const {
-  return false;
-}
-
 PermissionMessage APIPermission::GetMessage_() const {
   return info()->GetMessage_();
 }
diff --git a/chrome/common/extensions/permissions/api_permission.h b/chrome/common/extensions/permissions/api_permission.h
index 6d7ddfd..a8842a4 100644
--- a/chrome/common/extensions/permissions/api_permission.h
+++ b/chrome/common/extensions/permissions/api_permission.h
@@ -180,9 +180,6 @@ class APIPermission {
     return info_;
   }
 
-  // Returns true if this permission cannot be found in the manifest.
-  virtual bool ManifestEntryForbidden() const;
-
   // Returns true if this permission has any PermissionMessages.
   virtual bool HasMessages() const = 0;
 
diff --git a/chrome/common/extensions/permissions/chrome_scheme_hosts.cc b/chrome/common/extensions/permissions/chrome_scheme_hosts.cc
index d32d055..6833597 100644
--- a/chrome/common/extensions/permissions/chrome_scheme_hosts.cc
+++ b/chrome/common/extensions/permissions/chrome_scheme_hosts.cc
@@ -8,8 +8,6 @@
 #include "chrome/common/url_constants.h"
 #include "extensions/common/url_pattern.h"
 #include "extensions/common/url_pattern_set.h"
-#include "grit/generated_resources.h"
-#include "ui/base/l10n/l10n_util.h"
 
 namespace {
 const char kThumbsWhiteListedExtension[] = "khopmbdjffemhegeeobelklnbglcdgfh";
@@ -17,26 +15,6 @@ const char kThumbsWhiteListedExtension[] = "khopmbdjffemhegeeobelklnbglcdgfh";
 
 namespace extensions {
 
-PermissionMessages GetChromeSchemePermissionWarnings(
-    const URLPatternSet& hosts) {
-  PermissionMessages messages;
-  for (URLPatternSet::const_iterator i = hosts.begin();
-       i != hosts.end(); ++i) {
-    if (i->scheme() != chrome::kChromeUIScheme)
-      continue;
-    // chrome://favicon is the only URL for chrome:// scheme that we
-    // want to support. We want to deprecate the "chrome" scheme.
-    // We should not add any additional "host" here.
-    if (GURL(chrome::kChromeUIFaviconURL).host() != i->host())
-      continue;
-    messages.push_back(PermissionMessage(
-        PermissionMessage::kFavicon,
-        l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_FAVICON)));
-    break;
-  }
-  return messages;
-}
-
 URLPatternSet GetPermittedChromeSchemeHosts(
     const Extension* extension,
     const APIPermissionSet& api_permissions) {
diff --git a/chrome/common/extensions/permissions/chrome_scheme_hosts.h b/chrome/common/extensions/permissions/chrome_scheme_hosts.h
index 8d2a6dd..4ac81622 100644
--- a/chrome/common/extensions/permissions/chrome_scheme_hosts.h
+++ b/chrome/common/extensions/permissions/chrome_scheme_hosts.h
@@ -5,8 +5,6 @@
 #ifndef CHROME_COMMON_EXTENSIONS_PERMISSIONS_CHROME_SCHEME_HOSTS_H_
 #define CHROME_COMMON_EXTENSIONS_PERMISSIONS_CHROME_SCHEME_HOSTS_H_
 
-#include "extensions/common/permissions/permission_message.h"
-
 // Chrome-specific special case handling for permissions on hosts in
 // the chrome:// scheme.
 namespace extensions {
@@ -15,9 +13,6 @@ class APIPermissionSet;
 class Extension;
 class URLPatternSet;
 
-PermissionMessages GetChromeSchemePermissionWarnings(
-    const URLPatternSet& hosts);
-
 URLPatternSet GetPermittedChromeSchemeHosts(
     const Extension* extension,
     const APIPermissionSet& permissions);
diff --git a/chrome/common/extensions/permissions/permission_set.cc b/chrome/common/extensions/permissions/permission_set.cc
index 75d145c..7406bb5 100644
--- a/chrome/common/extensions/permissions/permission_set.cc
+++ b/chrome/common/extensions/permissions/permission_set.cc
@@ -9,11 +9,10 @@
 #include <string>
 
 #include "base/stl_util.h"
-#include "chrome/common/extensions/permissions/chrome_scheme_hosts.h"
-#include "chrome/common/extensions/permissions/media_galleries_permission.h"
 #include "chrome/common/extensions/permissions/permission_message_util.h"
 #include "chrome/common/extensions/permissions/permissions_info.h"
 #include "content/public/common/url_constants.h"
+#include "extensions/common/extensions_client.h"
 #include "extensions/common/url_pattern.h"
 #include "extensions/common/url_pattern_set.h"
 #include "grit/generated_resources.h"
@@ -140,23 +139,6 @@ PermissionSet* PermissionSet::CreateUnion(
   return new PermissionSet(apis, explicit_hosts, scriptable_hosts);
 }
 
-// static
-PermissionSet* PermissionSet::ExcludeNotInManifestPermissions(
-    const PermissionSet* set) {
-  if (!set)
-    return new PermissionSet();
-
-  APIPermissionSet apis;
-  for (APIPermissionSet::const_iterator i = set->apis().begin();
-       i != set->apis().end(); ++i) {
-    if (!i->ManifestEntryForbidden())
-      apis.insert(i->Clone());
-  }
-
-  return new PermissionSet(
-      apis, set->explicit_hosts(), set->scriptable_hosts());
-}
-
 bool PermissionSet::operator==(
     const PermissionSet& rhs) const {
   return apis_ == rhs.apis_ &&
@@ -179,18 +161,6 @@ std::set<std::string> PermissionSet::GetAPIsAsStrings() const {
   return apis_str;
 }
 
-std::set<std::string> PermissionSet::GetDistinctHostsForDisplay() const {
-  URLPatternSet hosts_displayed_as_url;
-  // Filters out every URL pattern that matches chrome:// scheme.
-  for (URLPatternSet::const_iterator i = effective_hosts_.begin();
-       i != effective_hosts_.end(); ++i) {
-    if (i->scheme() != chrome::kChromeUIScheme) {
-      hosts_displayed_as_url.AddPattern(*i);
-    }
-  }
-  return GetDistinctHosts(hosts_displayed_as_url, true, true);
-}
-
 PermissionMessages PermissionSet::GetPermissionMessages(
     Manifest::Type extension_type) const {
   PermissionMessages messages;
@@ -264,14 +234,6 @@ std::vector<string16> PermissionSet::GetWarningMessages(
       }
     }
 
-    // The warning message for declarativeWebRequest permissions speaks about
-    // blocking parts of pages, which is a subset of what the "<all_urls>"
-    // access allows. Therefore we display only the "<all_urls>" warning message
-    // if both permissions are required.
-    if (id == PermissionMessage::kDeclarativeWebRequest &&
-        HasEffectiveAccessToAllHosts())
-      continue;
-
     messages.push_back(i->message());
   }
 
@@ -496,6 +458,18 @@ std::set<PermissionMessage> PermissionSet::GetAPIPermissionMessages() const {
     messages.erase(
         PermissionMessage(PermissionMessage::kFileSystemDirectory, string16()));
   }
+
+  // A special hack: The warning message for declarativeWebRequest
+  // permissions speaks about blocking parts of pages, which is a
+  // subset of what the "<all_urls>" access allows. Therefore we
+  // display only the "<all_urls>" warning message if both permissions
+  // are required.
+  if (HasEffectiveAccessToAllHosts()) {
+    messages.erase(
+        PermissionMessage(
+            PermissionMessage::kDeclarativeWebRequest, string16()));
+  }
+
   return messages;
 }
 
@@ -514,12 +488,11 @@ std::set<PermissionMessage> PermissionSet::GetHostPermissionMessages(
         PermissionMessage::kHostsAll,
         l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS)));
   } else {
-    PermissionMessages additional_warnings =
-        GetChromeSchemePermissionWarnings(effective_hosts_);
-    for (size_t i = 0; i < additional_warnings.size(); ++i)
-      messages.insert(additional_warnings[i]);
+    URLPatternSet regular_hosts;
+    ExtensionsClient::Get()->FilterHostPermissions(
+        effective_hosts_, &regular_hosts, &messages);
 
-    std::set<std::string> hosts = GetDistinctHostsForDisplay();
+    std::set<std::string> hosts = GetDistinctHosts(regular_hosts, true, true);
     if (!hosts.empty())
       messages.insert(permission_message_util::CreateFromHostList(hosts));
   }
@@ -537,14 +510,6 @@ bool PermissionSet::HasLessAPIPrivilegesThan(
   PermissionMsgSet delta_warnings =
       base::STLSetDifference<PermissionMsgSet>(new_warnings, current_warnings);
 
-  // A special hack: the DWR permission is weaker than all hosts permission.
-  if (delta_warnings.size() == 1u &&
-      delta_warnings.begin()->id() ==
-          PermissionMessage::kDeclarativeWebRequest &&
-      HasEffectiveAccessToAllHosts()) {
-    return false;
-  }
-
   // A special hack: kFileSystemWriteDirectory implies kFileSystemDirectory and
   // kFileSystemWrite.
   // TODO(sammc): Remove this. See http://crbug.com/284849.
diff --git a/chrome/common/extensions/permissions/permission_set.h b/chrome/common/extensions/permissions/permission_set.h
index f385aa6..0083a08 100644
--- a/chrome/common/extensions/permissions/permission_set.h
+++ b/chrome/common/extensions/permissions/permission_set.h
@@ -55,11 +55,6 @@ class PermissionSet
   static PermissionSet* CreateUnion(
       const PermissionSet* set1, const PermissionSet* set2);
 
-  // Creates a new permission set that only contains permissions that must be
-  // in the manifest.  Passes ownership of the new set to the caller.
-  static PermissionSet* ExcludeNotInManifestPermissions(
-      const PermissionSet* set);
-
   bool operator==(const PermissionSet& rhs) const;
 
   // Returns true if every API or host permission available to |set| is also
@@ -137,15 +132,15 @@ class PermissionSet
  private:
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest, HasLessHostPrivilegesThan);
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest, GetWarningMessages_AudioVideo);
-  FRIEND_TEST_ALL_PREFIXES(PermissionsTest, GetDistinctHostsForDisplay);
+  FRIEND_TEST_ALL_PREFIXES(PermissionsTest, GetDistinctHosts);
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest,
-                           GetDistinctHostsForDisplay_ComIsBestRcd);
+                           GetDistinctHosts_ComIsBestRcd);
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest,
-                           GetDistinctHostsForDisplay_NetIs2ndBestRcd);
+                           GetDistinctHosts_NetIs2ndBestRcd);
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest,
-                           GetDistinctHostsForDisplay_OrgIs3rdBestRcd);
+                           GetDistinctHosts_OrgIs3rdBestRcd);
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest,
-                           GetDistinctHostsForDisplay_FirstInListIs4thBestRcd);
+                           GetDistinctHosts_FirstInListIs4thBestRcd);
   friend class base::RefCountedThreadSafe<PermissionSet>;
 
   ~PermissionSet();
@@ -179,11 +174,6 @@ class PermissionSet
   bool HasLessHostPrivilegesThan(const PermissionSet* permissions,
                                  Manifest::Type extension_type) const;
 
-  // Gets a list of the distinct hosts for displaying to the user.
-  // NOTE: do not use this for comparing permissions, since this disgards some
-  // information.
-  std::set<std::string> GetDistinctHostsForDisplay() const;
-
   // The api list is used when deciding if an extension can access certain
   // extension APIs and features.
   APIPermissionSet apis_;
diff --git a/chrome/common/extensions/permissions/permission_set_unittest.cc b/chrome/common/extensions/permissions/permission_set_unittest.cc
index e684d35..b0b7235 100644
--- a/chrome/common/extensions/permissions/permission_set_unittest.cc
+++ b/chrome/common/extensions/permissions/permission_set_unittest.cc
@@ -751,12 +751,6 @@ TEST(PermissionsTest, PermissionMessages) {
     const APIPermissionInfo* permission_info = i->info();
     EXPECT_TRUE(permission_info != NULL);
 
-    // Always skip permissions that cannot be in the manifest.
-    scoped_ptr<const APIPermission> permission(
-        permission_info->CreateAPIPermission());
-    if (permission->ManifestEntryForbidden())
-      continue;
-
     if (skip.count(i->id())) {
       EXPECT_EQ(PermissionMessage::kNone, permission_info->message_id())
           << "unexpected message_id for " << permission_info->name();
@@ -1014,15 +1008,12 @@ TEST(PermissionsTest, GetWarningMessages_PlatformApppHosts) {
   ASSERT_EQ(0u, warnings.size());
 }
 
-TEST(PermissionsTest, GetDistinctHostsForDisplay) {
-  scoped_refptr<PermissionSet> perm_set;
-  APIPermissionSet empty_perms;
+TEST(PermissionsTest, GetDistinctHosts) {
+  URLPatternSet explicit_hosts;
   std::set<std::string> expected;
   expected.insert("www.foo.com");
   expected.insert("www.bar.com");
   expected.insert("www.baz.com");
-  URLPatternSet explicit_hosts;
-  URLPatternSet scriptable_hosts;
 
   {
     SCOPED_TRACE("no dupes");
@@ -1034,9 +1025,8 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
         URLPattern(URLPattern::SCHEME_HTTP, "http://www.bar.com/path"));
     explicit_hosts.AddPattern(
         URLPattern(URLPattern::SCHEME_HTTP, "http://www.baz.com/path"));
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 
   {
@@ -1047,9 +1037,8 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
         URLPattern(URLPattern::SCHEME_HTTP, "http://www.foo.com/path"));
     explicit_hosts.AddPattern(
         URLPattern(URLPattern::SCHEME_HTTP, "http://www.baz.com/path"));
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 
   {
@@ -1058,9 +1047,8 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
     // Add a pattern that differs only by scheme. This should be filtered out.
     explicit_hosts.AddPattern(
         URLPattern(URLPattern::SCHEME_HTTPS, "https://www.bar.com/path"));
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 
   {
@@ -1069,9 +1057,8 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
     // Add some dupes by path.
     explicit_hosts.AddPattern(
         URLPattern(URLPattern::SCHEME_HTTP, "http://www.bar.com/pathypath"));
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 
   {
@@ -1086,9 +1073,8 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
     expected.insert("monkey.www.bar.com");
     expected.insert("bar.com");
 
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 
   {
@@ -1117,9 +1103,8 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
 
     expected.insert("www.foo.xyzzy");
 
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 
   {
@@ -1130,15 +1115,16 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
 
     expected.insert("*.google.com");
 
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 
   {
     SCOPED_TRACE("scriptable hosts");
+
+    APIPermissionSet empty_perms;
     explicit_hosts.ClearPatterns();
-    scriptable_hosts.ClearPatterns();
+    URLPatternSet scriptable_hosts;
     expected.clear();
 
     explicit_hosts.AddPattern(
@@ -1149,32 +1135,30 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay) {
     expected.insert("*.google.com");
     expected.insert("*.example.com");
 
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    scoped_refptr<PermissionSet> perm_set(new PermissionSet(
+        empty_perms, explicit_hosts, scriptable_hosts));
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(perm_set->effective_hosts(),
+                                              true, true));
   }
 
   {
     // We don't display warnings for file URLs because they are off by default.
     SCOPED_TRACE("file urls");
+
     explicit_hosts.ClearPatterns();
-    scriptable_hosts.ClearPatterns();
     expected.clear();
 
     explicit_hosts.AddPattern(
         URLPattern(URLPattern::SCHEME_FILE, "file:///*"));
 
-    perm_set = new PermissionSet(
-        empty_perms, explicit_hosts, scriptable_hosts);
-    EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+    EXPECT_EQ(expected,
+              PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
   }
 }
 
-TEST(PermissionsTest, GetDistinctHostsForDisplay_ComIsBestRcd) {
-  scoped_refptr<PermissionSet> perm_set;
-  APIPermissionSet empty_perms;
+TEST(PermissionsTest, GetDistinctHosts_ComIsBestRcd) {
   URLPatternSet explicit_hosts;
-  URLPatternSet scriptable_hosts;
   explicit_hosts.AddPattern(
       URLPattern(URLPattern::SCHEME_HTTP, "http://www.foo.ca/path"));
   explicit_hosts.AddPattern(
@@ -1190,16 +1174,12 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay_ComIsBestRcd) {
 
   std::set<std::string> expected;
   expected.insert("www.foo.com");
-  perm_set = new PermissionSet(
-      empty_perms, explicit_hosts, scriptable_hosts);
-  EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+  EXPECT_EQ(expected,
+            PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
 }
 
-TEST(PermissionsTest, GetDistinctHostsForDisplay_NetIs2ndBestRcd) {
-  scoped_refptr<PermissionSet> perm_set;
-  APIPermissionSet empty_perms;
+TEST(PermissionsTest, GetDistinctHosts_NetIs2ndBestRcd) {
   URLPatternSet explicit_hosts;
-  URLPatternSet scriptable_hosts;
   explicit_hosts.AddPattern(
       URLPattern(URLPattern::SCHEME_HTTP, "http://www.foo.ca/path"));
   explicit_hosts.AddPattern(
@@ -1214,17 +1194,12 @@ TEST(PermissionsTest, GetDistinctHostsForDisplay_NetIs2ndBestRcd) {
 
   std::set<std::string> expected;
   expected.insert("www.foo.net");
-  perm_set = new PermissionSet(
-      empty_perms, explicit_hosts, scriptable_hosts);
-  EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+  EXPECT_EQ(expected,
+            PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
 }
 
-TEST(PermissionsTest,
-     GetDistinctHostsForDisplay_OrgIs3rdBestRcd) {
-  scoped_refptr<PermissionSet> perm_set;
-  APIPermissionSet empty_perms;
+TEST(PermissionsTest, GetDistinctHosts_OrgIs3rdBestRcd) {
   URLPatternSet explicit_hosts;
-  URLPatternSet scriptable_hosts;
   explicit_hosts.AddPattern(
       URLPattern(URLPattern::SCHEME_HTTP, "http://www.foo.ca/path"));
   explicit_hosts.AddPattern(
@@ -1238,17 +1213,12 @@ TEST(PermissionsTest,
 
   std::set<std::string> expected;
   expected.insert("www.foo.org");
-  perm_set = new PermissionSet(
-      empty_perms, explicit_hosts, scriptable_hosts);
-  EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+  EXPECT_EQ(expected,
+            PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
 }
 
-TEST(PermissionsTest,
-     GetDistinctHostsForDisplay_FirstInListIs4thBestRcd) {
-  scoped_refptr<PermissionSet> perm_set;
-  APIPermissionSet empty_perms;
+TEST(PermissionsTest, GetDistinctHosts_FirstInListIs4thBestRcd) {
   URLPatternSet explicit_hosts;
-  URLPatternSet scriptable_hosts;
   explicit_hosts.AddPattern(
       URLPattern(URLPattern::SCHEME_HTTP, "http://www.foo.ca/path"));
   // No http://www.foo.org/path
@@ -1261,9 +1231,8 @@ TEST(PermissionsTest,
 
   std::set<std::string> expected;
   expected.insert("www.foo.ca");
-  perm_set = new PermissionSet(
-      empty_perms, explicit_hosts, scriptable_hosts);
-  EXPECT_EQ(expected, perm_set->GetDistinctHostsForDisplay());
+  EXPECT_EQ(expected,
+            PermissionSet::GetDistinctHosts(explicit_hosts, true, true));
 }
 
 TEST(PermissionsTest, HasLessHostPrivilegesThan) {
diff --git a/chrome/common/extensions/permissions/permissions_data.cc b/chrome/common/extensions/permissions/permissions_data.cc
index 89ba420..d9e56ef 100644
--- a/chrome/common/extensions/permissions/permissions_data.cc
+++ b/chrome/common/extensions/permissions/permissions_data.cc
@@ -38,21 +38,6 @@ namespace {
 
 PermissionsData::PolicyDelegate* g_policy_delegate = NULL;
 
-bool ContainsManifestForbiddenPermission(const APIPermissionSet& apis,
-                                         string16* error) {
-  CHECK(error);
-  for (APIPermissionSet::const_iterator iter = apis.begin();
-       iter != apis.end(); ++iter) {
-    if ((*iter)->ManifestEntryForbidden()) {
-      *error = ErrorUtils::FormatErrorMessageUTF16(
-          errors::kPermissionNotAllowedInManifest,
-          (*iter)->info()->name());
-      return true;
-    }
-  }
-  return false;
-}
-
 // Custom checks for the experimental permission that can't be expressed in
 // _permission_features.json.
 bool CanSpecifyExperimentalPermission(const Extension* extension) {
@@ -617,13 +602,6 @@ bool PermissionsData::ParsePermissions(Extension* extension, string16* error) {
     return false;
   }
 
-  if (ContainsManifestForbiddenPermission(
-          initial_required_permissions_->api_permissions, error) ||
-      ContainsManifestForbiddenPermission(
-          initial_optional_permissions_->api_permissions, error)) {
-    return false;
-  }
-
   return true;
 }
 
diff --git a/extensions/common/extensions_client.h b/extensions/common/extensions_client.h
index 83550a9..eb3ebfe 100644
--- a/extensions/common/extensions_client.h
+++ b/extensions/common/extensions_client.h
@@ -5,12 +5,15 @@
 #ifndef EXTENSIONS_COMMON_EXTENSIONS_CLIENT_H_
 #define EXTENSIONS_COMMON_EXTENSIONS_CLIENT_H_
 
+#include <set>
 #include <string>
 
 namespace extensions {
 
 class FeatureProvider;
+class PermissionMessage;
 class PermissionsProvider;
+class URLPatternSet;
 
 // Sets up global state for the extensions system. Should be Set() once in each
 // process. This should be implemented by the client of the extensions system.
@@ -26,6 +29,14 @@ class ExtensionsClient {
   // Called at startup. Registers the handlers for parsing manifests.
   virtual void RegisterManifestHandlers() const = 0;
 
+  // Takes the list of all hosts and filters out those with special
+  // permission strings. Adds the regular hosts to |new_hosts|,
+  // and adds the special permission messages to |messages|.
+  virtual void FilterHostPermissions(
+      const URLPatternSet& hosts,
+      URLPatternSet* new_hosts,
+      std::set<PermissionMessage>* messages) const = 0;
+
   // Return the extensions client.
   static ExtensionsClient* Get();