Allow content script insertion on about:-URLs.

chrome/renderer/extensions/user_script_slave.cc

Index: chrome/renderer/extensions/user_script_slave.cc
diff --git a/chrome/renderer/extensions/user_script_slave.cc b/chrome/renderer/extensions/user_script_slave.cc
index bdaff10bb86692bbe69c3f3e69cadc438f90067c..24a0f0562ccec7957c9b8034ff5e084ff558877c 100644
--- a/chrome/renderer/extensions/user_script_slave.cc
+++ b/chrome/renderer/extensions/user_script_slave.cc
@@ -194,6 +194,20 @@ GURL UserScriptSlave::GetDataSourceURLForFrame(const WebFrame* frame) {
   return GURL(data_source->request().url());
 }
 
+GURL UserScriptSlave::GetOriginURLForFrame(const WebFrame* frame) {
+  // All pages served with the about:-scheme inherit the security origin from
+  // their parent document (i.e. either the page that contains the document or
+  // the page that opened a new window containing this page).
+  // If this parent document is accessible by the extension, then access to
+  // the about:-frame is allowed if the extension has requested access to it.
+  GURL document_origin_url(frame->document().securityOrigin().toString());
+  // TODO(robwu): Iframes with the sandbox HTML attribute are mistakenly

[not at google - send to devlin, 2014/04/21 19:56:22]

I .. think it makes sense to return an invalid URL if it's a sandbox. You can
check frame->document()->securityOrigin()->isUnique() for that.

[robwu, 2014/04/21 22:15:41]

Oops, this comment should be removed. I checked, and about:blank/about:srcdoc
can access its parent frame despite the sandbox attribute.

+  // excluded by this method, because their origin is "null" (i.e. unique).

[dcheng, 2014/04/21 21:02:05]

I don't think this should match sandboxed iframes.

1) Can't extensions still inject by matching on a wildcard pattern?
2) If a sandboxed iframe wants to be treated as same origin, it can specify that
option. If not specified, it really should be treated as a unique origin that
can't match anything else.

[robwu, 2014/04/21 22:15:41]

The "sandbox" attribute somehow doesn't apply to about:blank:
1. Visit http://jsfiddle.net/Q7tAw/show/
2. Open F12 devtools.
3. Change JavaScript context to the about:blank frame.
4. Type "parent" in the JS console and press enter.
5. Observe that the frame can access its parent document without being blocked.

Content scripts are declared for specific URLs, so the (non-)existence of the
sandbox attribute should not matter. Otherwise, adblockers could be circumvented
by sandboxing the iframe.

[dcheng, 2014/04/21 22:35:18]

Huh. I'm pretty sure that's a bug. I'll follow up in Blink on this.
That's the thing. Now we're mixing up origins and URLs, picking whichever one
happens to be convenient, and things start to get confusing. Wouldn't an
adblocker use wildcard permissions anyway, making the whole issue moot to begin
with?

[not at google - send to devlin, 2014/04/21 22:36:47]

Yes seems like this code should be explicitly checking for an about: URL.

+  if (document_origin_url.is_valid())
+    return document_origin_url;
+  return frame->document().url();

[not at google - send to devlin, 2014/04/21 19:56:22]

.GetOrigin()?

[robwu, 2014/04/21 22:15:41]

Done.

+}
+
 void UserScriptSlave::InjectScripts(WebFrame* frame,
                                     UserScript::RunLocation location) {
   GURL data_source_url = GetDataSourceURLForFrame(frame);
@@ -224,15 +238,24 @@ void UserScriptSlave::InjectScripts(WebFrame* frame,
     if (!extension)
       continue;
 
+    const bool isAboutScheme = data_source_url.SchemeIs(content::kAboutScheme);

[not at google - send to devlin, 2014/04/21 19:56:22]

is_about_scheme

[robwu, 2014/04/21 22:15:41]

Done.

+    if (isAboutScheme) {
+      if (!script->match_about_blank())
+        continue;
+      data_source_url = GetOriginURLForFrame(frame);
+    }
+
     // Content scripts are not tab-specific.
     const int kNoTabId = -1;
     // We don't have a process id in this context.
     const int kNoProcessId = -1;
+    // If the page is about:blank, check against the extension's origin
+    // permissions instead of the user script's URL patterns.
     if (!PermissionsData::CanExecuteScriptOnPage(extension,
                                                  data_source_url,
                                                  frame->top()->document().url(),
                                                  kNoTabId,
-                                                 script,
+                                                 isAboutScheme ? NULL : script,

[not at google - send to devlin, 2014/04/21 19:56:22]

don't have time to trace this down, why NULL here?

[robwu, 2014/04/21 22:15:41]

Moved to separate variable, preceeded by a comment:
// If the page is about:blank, pass NULL instead of a UserScript. This
// ensures that the URL is checked against the extension's host permissions
// instead of the script's URL patterns.
const UserScript* script_or_null = is_about_scheme ? NULL : script;

                                                  kNoProcessId,
                                                  NULL)) {
       continue;