Pull gallery special case down into Extension::CanAccessHost() to avoid duplicating it in multiple places.

Also rename that method to better reflect what it is used for.

BUG=35382

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

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

8 files changed, 27 insertions, 28 deletions

diff --git a/chrome/browser/extensions/execute_code_in_tab_function.cc b/chrome/browser/extensions/execute_code_in_tab_function.cc
index 2443442..da16d80 100644
--- a/chrome/browser/extensions/execute_code_in_tab_function.cc
+++ b/chrome/browser/extensions/execute_code_in_tab_function.cc
@@ -66,23 +66,10 @@ bool ExecuteCodeInTabFunction::RunImpl() {
   DCHECK(browser);
   DCHECK(contents);
 
-  // Disallow executeScript when the target contents is a gallery page.
-  // This mirrors a check in UserScriptSlave::InjectScripts
   // NOTE: This can give the wrong answer due to race conditions, but it is OK,
   // we check again in the renderer.
-  if (contents->GetURL().host() ==
-      GURL(extension_urls::kGalleryBrowsePrefix).host()) {
-    error_ = keys::kCannotScriptGalleryError;
+  if (!GetExtension()->CanExecuteScriptOnHost(contents->GetURL(), &error_))
     return false;
-  }
-
-  // NOTE: This can give the wrong answer due to race conditions, but it is OK,
-  // we check again in the renderer.
-  if (!GetExtension()->CanAccessHost(contents->GetURL())) {
-    error_ = ExtensionErrorUtils::FormatErrorMessage(
-        keys::kCannotAccessPageError, contents->GetURL().spec());
-    return false;
-  }
 
   if (script_info->HasKey(keys::kAllFramesKey)) {
     if (!script_info->GetBoolean(keys::kAllFramesKey, &all_frames_))
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 14621bf..e00964d 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -592,11 +592,8 @@ bool UpdateTabFunction::RunImpl() {
     // JavaScript URLs can do the same kinds of things as cross-origin XHR, so
     // we need to check host permissions before allowing them.
     if (new_gurl.SchemeIs(chrome::kJavaScriptScheme)) {
-      if (!GetExtension()->CanAccessHost(contents->GetURL())) {
-        error_ = ExtensionErrorUtils::FormatErrorMessage(
-            keys::kCannotAccessPageError, contents->GetURL().spec());
+      if (!GetExtension()->CanExecuteScriptOnHost(contents->GetURL(), &error_))
         return false;
-      }
 
       // TODO(aa): How does controller queue URLs? Is there any chance that this
       // JavaScript URL will end up applying to something other than
diff --git a/chrome/browser/extensions/extension_tabs_module_constants.cc b/chrome/browser/extensions/extension_tabs_module_constants.cc
index 4232a77..96176ea 100644
--- a/chrome/browser/extensions/extension_tabs_module_constants.cc
+++ b/chrome/browser/extensions/extension_tabs_module_constants.cc
@@ -45,10 +45,6 @@ const char kInvalidUrlError[] = "Invalid url: \"*\".";
 const char kInternalVisibleTabCaptureError[] =
     "Internal error while trying to capture visible region of the current tab";
 const char kNotImplementedError[] = "This call is not yet implemented";
-const char kCannotAccessPageError[] = "Cannot access contents of url \"*\". "
-    "Extension manifest must request permission to access this host.";
-const char kCannotScriptGalleryError[] = "The extensions gallery cannot be "
-    "scripted.";
 const char kSupportedInWindowsOnlyError[] = "Supported in Windows only";
 
 const char kNoCodeOrFileToExecuteError[] = "No source code or file specified.";
diff --git a/chrome/browser/extensions/extension_tabs_module_constants.h b/chrome/browser/extensions/extension_tabs_module_constants.h
index ce340e5..f51bc5a 100644
--- a/chrome/browser/extensions/extension_tabs_module_constants.h
+++ b/chrome/browser/extensions/extension_tabs_module_constants.h
@@ -50,8 +50,6 @@ extern const char kNoSelectedTabError[];
 extern const char kInvalidUrlError[];
 extern const char kInternalVisibleTabCaptureError[];
 extern const char kNotImplementedError[];
-extern const char kCannotAccessPageError[];
-extern const char kCannotScriptGalleryError[];
 extern const char kSupportedInWindowsOnlyError[];
 
 extern const char kNoCodeOrFileToExecuteError[];
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 6b40129..99a9dfe 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1379,13 +1379,27 @@ ExtensionResource Extension::GetIconPath(Icons icon) {
   return GetResource(iter->second);
 }
 
-bool Extension::CanAccessHost(const GURL& url) const {
+bool Extension::CanExecuteScriptOnHost(const GURL& url,
+                                       std::string* error) const {
+  // No extensions are allowed to execute script on the gallery because that
+  // would allow extensions to manipulate their own install pages.
+  if (url.host() == GURL(extension_urls::kGalleryBrowsePrefix).host()) {
+    if (error)
+      *error = errors::kCannotScriptGallery;
+    return false;
+  }
+
   for (URLPatternList::const_iterator host = host_permissions_.begin();
        host != host_permissions_.end(); ++host) {
     if (host->MatchesUrl(url))
       return true;
   }
 
+  if (error) {
+    *error = ExtensionErrorUtils::FormatErrorMessage(errors::kCannotAccessPage,
+                                                     url.spec());
+  }
+
   return false;
 }
 
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 1445943..880131d 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -221,9 +221,11 @@ class Extension {
     return host_permissions_;
   }
 
-  // Returns true if the extension has permission to access the host for the
-  // specified URL.
-  bool CanAccessHost(const GURL& url) const;
+  // Returns true if the extension has permission to execute script on a
+  // particular host.
+  // TODO(aa): Also use this in the renderer, for normal content script
+  // injection. Currently, that has its own copy of this code.
+  bool CanExecuteScriptOnHost(const GURL& url, std::string* error) const;
 
   // Returns true if the extension has the specified API permission.
   bool HasApiPermission(const std::string& permission) const {
diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc
index 570e90d..d5233be 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -217,6 +217,9 @@ const char* kInvalidOptionsPage =
     "Invalid value for 'options_page'.";
 const char* kReservedMessageFound =
     "Reserved key * found in message catalog.";
+const char* kCannotAccessPage = "Cannot access contents of url \"*\". "
+    "Extension manifest must request permission to access this host.";
+const char* kCannotScriptGallery = "The extensions gallery cannot be scripted.";
 }  // namespace extension_manifest_errors
 
 namespace extension_urls {
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index f71d1eb..b990022 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -145,6 +145,8 @@ namespace extension_manifest_errors {
   extern const char* kLocalesMessagesFileMissing;
   extern const char* kInvalidOptionsPage;
   extern const char* kReservedMessageFound;
+  extern const char* kCannotAccessPage;
+  extern const char* kCannotScriptGallery;
 }  // namespace extension_manifest_errors
 
 namespace extension_urls {