Remove PermissionSet::InitImplicitExtensionPermissions.

Removes references to the manifest from PermissionSet.

BUG=162530


Review URL: https://chromiumcodereview.appspot.com/13529025

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

8 files changed, 62 insertions, 62 deletions

diff --git a/chrome/common/extensions/api/plugins/plugins_handler.cc b/chrome/common/extensions/api/plugins/plugins_handler.cc
index 05e1623..2f255f1 100644
--- a/chrome/common/extensions/api/plugins/plugins_handler.cc
+++ b/chrome/common/extensions/api/plugins/plugins_handler.cc
@@ -10,6 +10,8 @@
 #include "base/values.h"
 #include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/extensions/manifest.h"
+#include "chrome/common/extensions/permissions/api_permission.h"
+#include "chrome/common/extensions/permissions/api_permission_set.h"
 #include "extensions/common/error_utils.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -113,8 +115,10 @@ bool PluginsHandler::Parse(Extension* extension, string16* error) {
         is_public));
   }
 
-  if (!plugins_data->plugins.empty())
+  if (!plugins_data->plugins.empty()) {
     extension->SetManifestData(keys::kPlugins, plugins_data.release());
+    extension->initial_api_permissions()->insert(APIPermission::kPlugin);
+  }
 
   return true;
 }
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 6d6bace..bb30380 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -41,6 +41,7 @@
 #include "chrome/common/extensions/manifest.h"
 #include "chrome/common/extensions/manifest_handler.h"
 #include "chrome/common/extensions/manifest_handler_helpers.h"
+#include "chrome/common/extensions/manifest_handlers/content_scripts_handler.h"
 #include "chrome/common/extensions/manifest_handlers/kiosk_enabled_info.h"
 #include "chrome/common/extensions/manifest_handlers/offline_enabled_info.h"
 #include "chrome/common/extensions/manifest_url_handler.h"
@@ -1228,12 +1229,14 @@ bool Extension::InitFromValue(int flags, string16* error) {
     return false;
   }
 
+  URLPatternSet scriptable_hosts = ContentScriptsInfo::GetScriptableHosts(this);
+
   finished_parsing_manifest_ = true;
 
   runtime_data_.SetActivePermissions(new PermissionSet(
-      this, *initial_api_permissions_, host_permissions));
+      *initial_api_permissions_, host_permissions, scriptable_hosts));
   required_permission_set_ = new PermissionSet(
-      this, *initial_api_permissions_, host_permissions);
+      *initial_api_permissions_, host_permissions, scriptable_hosts);
   optional_permission_set_ = new PermissionSet(
       optional_api_permissions, optional_host_permissions, URLPatternSet());
   initial_api_permissions_.reset();
diff --git a/chrome/common/extensions/manifest_handlers/content_scripts_handler.cc b/chrome/common/extensions/manifest_handlers/content_scripts_handler.cc
index bed7070..534275da 100644
--- a/chrome/common/extensions/manifest_handlers/content_scripts_handler.cc
+++ b/chrome/common/extensions/manifest_handlers/content_scripts_handler.cc
@@ -17,6 +17,7 @@
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension_resource.h"
 #include "extensions/common/url_pattern.h"
+#include "extensions/common/url_pattern_set.h"
 #include "googleurl/src/gurl.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -348,6 +349,23 @@ bool ContentScriptsInfo::ExtensionHasScriptAtURL(const Extension* extension,
   return false;
 }
 
+// static
+URLPatternSet ContentScriptsInfo::GetScriptableHosts(
+    const Extension* extension) {
+  const UserScriptList& content_scripts = GetContentScripts(extension);
+  URLPatternSet scriptable_hosts;
+  for (UserScriptList::const_iterator content_script =
+           content_scripts.begin();
+       content_script != content_scripts.end();
+       ++content_script) {
+    URLPatternSet::const_iterator pattern =
+        content_script->url_patterns().begin();
+    for (; pattern != content_script->url_patterns().end(); ++pattern)
+      scriptable_hosts.AddPattern(*pattern);
+  }
+  return scriptable_hosts;
+}
+
 ContentScriptsHandler::ContentScriptsHandler() {
 }
 
@@ -414,13 +432,13 @@ bool ContentScriptsHandler::Validate(
     symlink_policy = ExtensionResource::SYMLINKS_MUST_RESOLVE_WITHIN_ROOT;
   }
 
-  const extensions::UserScriptList& content_scripts =
-      extensions::ContentScriptsInfo::GetContentScripts(extension);
+  const UserScriptList& content_scripts =
+      ContentScriptsInfo::GetContentScripts(extension);
   for (size_t i = 0; i < content_scripts.size(); ++i) {
-    const extensions::UserScript& script = content_scripts[i];
+    const UserScript& script = content_scripts[i];
 
     for (size_t j = 0; j < script.js_scripts().size(); j++) {
-      const extensions::UserScript::File& js_script = script.js_scripts()[j];
+      const UserScript::File& js_script = script.js_scripts()[j];
       const base::FilePath& path = ExtensionResource::GetFilePath(
           js_script.extension_root(), js_script.relative_path(),
           symlink_policy);
@@ -430,7 +448,7 @@ bool ContentScriptsHandler::Validate(
     }
 
     for (size_t j = 0; j < script.css_scripts().size(); j++) {
-      const extensions::UserScript::File& css_script = script.css_scripts()[j];
+      const UserScript::File& css_script = script.css_scripts()[j];
       const base::FilePath& path = ExtensionResource::GetFilePath(
           css_script.extension_root(), css_script.relative_path(),
           symlink_policy);
diff --git a/chrome/common/extensions/manifest_handlers/content_scripts_handler.h b/chrome/common/extensions/manifest_handlers/content_scripts_handler.h
index c3b8d06..6f010b5 100644
--- a/chrome/common/extensions/manifest_handlers/content_scripts_handler.h
+++ b/chrome/common/extensions/manifest_handlers/content_scripts_handler.h
@@ -13,6 +13,8 @@
 
 namespace extensions {
 
+class URLPatternSet;
+
 struct ContentScriptsInfo : public Extension::ManifestData {
   ContentScriptsInfo();
   virtual ~ContentScriptsInfo();
@@ -24,6 +26,9 @@ struct ContentScriptsInfo : public Extension::ManifestData {
   // no content scripts, this returns an empty list).
   static const UserScriptList& GetContentScripts(const Extension* extension);
 
+  // Returns the list of hosts that this extension can run content scripts on.
+  static URLPatternSet GetScriptableHosts(const Extension* extension);
+
   // Returns true if the extension has a content script declared at |url|.
   static bool ExtensionHasScriptAtURL(const Extension* extension,
                                       const GURL& url);
diff --git a/chrome/common/extensions/manifest_handlers/content_scripts_manifest_unittest.cc b/chrome/common/extensions/manifest_handlers/content_scripts_manifest_unittest.cc
index 679357c0..c08c154 100644
--- a/chrome/common/extensions/manifest_handlers/content_scripts_manifest_unittest.cc
+++ b/chrome/common/extensions/manifest_handlers/content_scripts_manifest_unittest.cc
@@ -24,7 +24,7 @@ class ContentScriptsManifestTest : public ExtensionManifestTest {
   }
 };
 
-TEST_F(ContentScriptsManifestTest, ContentScriptMatchPattern) {
+TEST_F(ContentScriptsManifestTest, MatchPattern) {
   Testcase testcases[] = {
     // chrome:// urls are not allowed.
     Testcase("content_script_chrome_url_invalid.json",
@@ -48,16 +48,28 @@ TEST_F(ContentScriptsManifestTest, ContentScriptMatchPattern) {
   LoadAndExpectSuccess("ports_in_content_scripts.json");
 }
 
-TEST_F(ContentScriptsManifestTest, ContentScriptsOnChromeUrlsWithFlag) {
+TEST_F(ContentScriptsManifestTest, OnChromeUrlsWithFlag) {
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kExtensionsOnChromeURLs);
-  std::string error;
-  scoped_refptr<extensions::Extension> extension =
+  scoped_refptr<Extension> extension =
     LoadAndExpectSuccess("content_script_chrome_url_invalid.json");
-  EXPECT_EQ("", error);
   const GURL newtab_url("chrome://newtab/");
   EXPECT_TRUE(ContentScriptsInfo::ExtensionHasScriptAtURL(extension,
                                                           newtab_url));
 }
 
+TEST_F(ContentScriptsManifestTest, ScriptableHosts) {
+  // TODO(yoz): Test GetScriptableHosts.
+  scoped_refptr<Extension> extension =
+      LoadAndExpectSuccess("content_script_yahoo.json");
+  URLPatternSet scriptable_hosts =
+      ContentScriptsInfo::GetScriptableHosts(extension);
+
+  URLPatternSet expected;
+  expected.AddPattern(
+      URLPattern(URLPattern::SCHEME_HTTP, "http://yahoo.com/*"));
+
+  EXPECT_EQ(expected, scriptable_hosts);
+}
+
 }  // namespace extensions
diff --git a/chrome/common/extensions/manifest_url_handler.cc b/chrome/common/extensions/manifest_url_handler.cc
index 2299ec5..2e18eb1 100644
--- a/chrome/common/extensions/manifest_url_handler.cc
+++ b/chrome/common/extensions/manifest_url_handler.cc
@@ -15,6 +15,8 @@
 #include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/extensions/manifest.h"
+#include "chrome/common/extensions/permissions/api_permission.h"
+#include "chrome/common/extensions/permissions/api_permission_set.h"
 #include "chrome/common/url_constants.h"
 #include "extensions/common/error_utils.h"
 #include "grit/generated_resources.h"
@@ -104,6 +106,7 @@ bool DevToolsPageHandler::Parse(Extension* extension, string16* error) {
   }
   manifest_url->url_ = extension->GetResourceURL(devtools_str);
   extension->SetManifestData(keys::kDevToolsPage, manifest_url.release());
+  extension->initial_api_permissions()->insert(APIPermission::kDevtools);
   return true;
 }
 
diff --git a/chrome/common/extensions/permissions/permission_set.cc b/chrome/common/extensions/permissions/permission_set.cc
index 704255b..ea347c7 100644
--- a/chrome/common/extensions/permissions/permission_set.cc
+++ b/chrome/common/extensions/permissions/permission_set.cc
@@ -8,10 +8,6 @@
 #include <iterator>
 #include <string>
 
-#include "chrome/common/extensions/api/plugins/plugins_handler.h"
-#include "chrome/common/extensions/extension.h"
-#include "chrome/common/extensions/manifest_handlers/content_scripts_handler.h"
-#include "chrome/common/extensions/manifest_url_handler.h"
 #include "chrome/common/extensions/permissions/permissions_info.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/common/url_pattern.h"
@@ -107,18 +103,6 @@ namespace extensions {
 PermissionSet::PermissionSet() {}
 
 PermissionSet::PermissionSet(
-    const extensions::Extension* extension,
-    const APIPermissionSet& apis,
-    const URLPatternSet& explicit_hosts)
-    : apis_(apis) {
-  DCHECK(extension);
-  AddPatternsAndRemovePaths(explicit_hosts, &explicit_hosts_);
-  InitImplicitExtensionPermissions(extension);
-  InitImplicitPermissions();
-  InitEffectiveHosts();
-}
-
-PermissionSet::PermissionSet(
     const APIPermissionSet& apis,
     const URLPatternSet& explicit_hosts,
     const URLPatternSet& scriptable_hosts)
@@ -525,27 +509,6 @@ void PermissionSet::InitImplicitPermissions() {
     apis_.insert(APIPermission::kFileBrowserHandlerInternal);
 }
 
-void PermissionSet::InitImplicitExtensionPermissions(
-    const extensions::Extension* extension) {
-  // Add the implied permissions.
-  if (extensions::PluginInfo::HasPlugins(extension))
-    apis_.insert(APIPermission::kPlugin);
-
-  if (!ManifestURL::GetDevToolsPage(extension).is_empty())
-    apis_.insert(APIPermission::kDevtools);
-
-  // Add the scriptable hosts.
-  for (extensions::UserScriptList::const_iterator content_script =
-           ContentScriptsInfo::GetContentScripts(extension).begin();
-       content_script != ContentScriptsInfo::GetContentScripts(extension).end();
-       ++content_script) {
-    URLPatternSet::const_iterator pattern =
-        content_script->url_patterns().begin();
-    for (; pattern != content_script->url_patterns().end(); ++pattern)
-      scriptable_hosts_.AddPattern(*pattern);
-  }
-}
-
 void PermissionSet::InitEffectiveHosts() {
   effective_hosts_.ClearPatterns();
 
diff --git a/chrome/common/extensions/permissions/permission_set.h b/chrome/common/extensions/permissions/permission_set.h
index d242abde..b8fd3ad 100644
--- a/chrome/common/extensions/permissions/permission_set.h
+++ b/chrome/common/extensions/permissions/permission_set.h
@@ -32,15 +32,10 @@ class PermissionSet
   // Creates an empty permission set (e.g. default permissions).
   PermissionSet();
 
-  // Creates a new permission set based on the |extension| manifest data, and
-  // the api and host permissions (|apis| and |hosts|). The effective hosts
-  // of the newly created permission set will be inferred from the |extension|
-  // manifest, |apis| and |hosts|.
-  PermissionSet(const extensions::Extension* extension,
-                const APIPermissionSet& apis,
-                const URLPatternSet& explicit_hosts);
-
-  // Creates a new permission set based on the specified data.
+  // Creates a new permission set based on the specified data: the API
+  // permissions, host permissions, and scriptable hosts. The effective hosts
+  // of the newly created permission set will be inferred from the given
+  // host permissions.
   PermissionSet(const APIPermissionSet& apis,
                 const URLPatternSet& explicit_hosts,
                 const URLPatternSet& scriptable_hosts);
@@ -164,9 +159,6 @@ class PermissionSet
       bool include_rcd,
       bool exclude_file_scheme);
 
-  // Initializes the set based on |extension|'s manifest data.
-  void InitImplicitExtensionPermissions(const extensions::Extension* extension);
-
   // Adds permissions implied independently of other context.
   void InitImplicitPermissions();