Have the Debugger extension api check that it has access to the tab

extensions/common/permissions/permissions_data.cc

Index: extensions/common/permissions/permissions_data.cc
diff --git a/extensions/common/permissions/permissions_data.cc b/extensions/common/permissions/permissions_data.cc
index 72717f7617d493ac2c9955155082def54ae0e951..620359b181d96088dc51297ceddbd990759632b9 100644
--- a/extensions/common/permissions/permissions_data.cc
+++ b/extensions/common/permissions/permissions_data.cc
@@ -17,6 +17,7 @@
 #include "extensions/common/url_pattern_set.h"
 #include "extensions/common/user_script.h"
 #include "url/gurl.h"
+#include "url/url_constants.h"
 
 namespace extensions {
 
@@ -70,6 +71,50 @@ bool PermissionsData::CanExecuteScriptEverywhere(const Extension* extension) {
          whitelist.end();
 }
 
+// static
+bool PermissionsData::IsRestrictedUrl(const GURL& document_url,
+                                      const GURL& top_frame_url,
+                                      const Extension* extension,
+                                      std::string* error) {
+  if (CanExecuteScriptEverywhere(extension))
+    return false;
+
+  // Check if the scheme is valid for extensions. If not, return.
+  // For some reason, about urls are valid but not listed in the valid schemes.
+  // Hmm....

[meacer, 2014/06/26 17:32:08]

Is this because of the somewhat recent change that allowed content scripts on
about: pages? AFAIR that CL only modified "matches" and didn't add a host
permission for about:, but still.

That change also checked if the parent of the about: page was owned by a page
which the extension can access, so I'm not sure if
document_url.SchemeIs(url::kAboutScheme) will be too broad of a check?

[Devlin, 2014/06/26 17:37:13]

Mostly, this is going off the PermissionsData tests, in which we don't consider
about:flags a restricted url (even though we can't script on it, we can capture
it).  Is this the right behavior?  Most likely not.  But I'm also not sure that
no extensions rely on capturing about: urls.  Note that this is doesn't allow
anything that wasn't allowed before (unlike the chrome-extensions:// change).

[not at google - send to devlin, 2014/06/26 18:11:28]

If this is just so that tabCapture can work, can't you put this extra check into
CanCaptureTab or whatever it's called? If not, a more specific comment would be
better.

[meacer, 2014/06/26 19:24:39]

I meant this bug, not specific to tab capture: http://crbug.com/76429

+  if (!URLPattern::IsValidSchemeForExtensions(document_url.scheme()) &&
+      !document_url.SchemeIs(url::kAboutScheme)) {
+    if (error) {
+      *error = ErrorUtils::FormatErrorMessage(
+                   manifest_errors::kCannotAccessPage,
+                   document_url.spec());
+    }
+    return true;
+  }
+
+  if (!ExtensionsClient::Get()->IsScriptableURL(document_url, error))
+    return true;
+
+  bool allow_on_chrome_urls = base::CommandLine::ForCurrentProcess()->HasSwitch(
+                                  switches::kExtensionsOnChromeURLs);
+  if (document_url.SchemeIs(content::kChromeUIScheme) &&
+      !allow_on_chrome_urls) {
+    if (error)
+      *error = manifest_errors::kCannotAccessChromeUrl;
+    return true;
+  }
+
+  if (top_frame_url.SchemeIs(kExtensionScheme) &&
+      top_frame_url.host() != extension->id() &&
+      !allow_on_chrome_urls) {
+    if (error)
+      *error = manifest_errors::kCannotAccessExtensionUrl;
+    return true;
+  }
+
+  return false;
+}
+
 void PermissionsData::SetActivePermissions(
     const PermissionSet* permissions) const {
   base::AutoLock auto_lock(runtime_lock_);
@@ -283,30 +328,8 @@ bool PermissionsData::CanRunOnPage(const Extension* extension,
     return false;
   }
 
-  bool can_execute_everywhere = CanExecuteScriptEverywhere(extension);
-  if (!can_execute_everywhere &&
-      !ExtensionsClient::Get()->IsScriptableURL(document_url, error)) {
-    return false;
-  }
-
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kExtensionsOnChromeURLs)) {
-    if (document_url.SchemeIs(content::kChromeUIScheme) &&
-        !can_execute_everywhere) {
-      if (error)
-        *error = manifest_errors::kCannotAccessChromeUrl;
-      return false;
-    }
-  }
-
-  if (top_frame_url.SchemeIs(kExtensionScheme) &&
-      top_frame_url.GetOrigin() !=
-          Extension::GetBaseURLFromExtensionId(extension->id()).GetOrigin() &&
-      !can_execute_everywhere) {
-    if (error)
-      *error = manifest_errors::kCannotAccessExtensionUrl;
+  if (IsRestrictedUrl(document_url, top_frame_url, extension, error))
     return false;
-  }
 
   if (HasTabSpecificPermissionToExecuteScript(tab_id, top_frame_url))
     return true;