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..cd97a76db582fcf087c38a91242f7d3fc93c11e4 100644
--- a/chrome/renderer/extensions/user_script_slave.cc
+++ b/chrome/renderer/extensions/user_script_slave.cc
@@ -194,6 +194,18 @@ 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

[dcheng, 2014/04/22 00:40:01]

As I mentioned earlier, this comment is incorrect. This should only happen for
non-sandboxed iframes; the fact that it does not is a bug in Blink.

+  // 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());
+  if (document_origin_url.is_valid())
+    return document_origin_url;
+  return GURL(frame->document().url()).GetOrigin();

[dcheng, 2014/04/22 00:40:01]

The implementation here does not match the documentation. Here's what the
documentation says: "Content scripts will only be injected on pages if your
extension has access to the page's parent document."

Consider the case where a content script wants to match against
"http://example.com/path/*", and the extension manifest specifies
matches_about_blank. http://example.com/path/foo.html includes an about:blank
iframe. My understanding is that we will never match the about:blank iframe
because an "origin" of "http://example.com" will never match
"http://example.com/path/*", so no script will be injected.

I think we should either:
1) Follow the documentation and just match against the parent document URL. This
makes the checking window.opener (when you create a new about:blank window)
harder since we'll have to duplicate some logic to go poke at window.opener in
this case. But at least it will do what developers expect.

2) Update the documentation to explicitly mention that this needs to fall back
to origins for about:blank iframes/windows, and force extensions that really
want to match unique security origins to do so by specifying the appropriate
wildcard pattern.

Of these options, I strongly prefer #2 since security origins are well defined
(and we already have the code to make it work in Blink) and we won't have a
security model that's different from the rest of the web.

[robwu, 2014/04/22 13:29:52]

I prefer #1, because it avoids special casing like
https://codereview.chromium.org/226663003/patch/60001/60014#newcode251. After
https://codereview.chromium.org/226663003/ gets merged, I will get rid of
GetOriginURLForFrame and use frame->document()->inheritedURL() instead.

+}
+
 void UserScriptSlave::InjectScripts(WebFrame* frame,
                                     UserScript::RunLocation location) {
   GURL data_source_url = GetDataSourceURLForFrame(frame);
@@ -224,15 +236,27 @@ void UserScriptSlave::InjectScripts(WebFrame* frame,
     if (!extension)
       continue;
 
+    const bool is_about_scheme =
+        data_source_url.SchemeIs(content::kAboutScheme);
+    if (is_about_scheme) {
+      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, 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;
     if (!PermissionsData::CanExecuteScriptOnPage(extension,
                                                  data_source_url,
                                                  frame->top()->document().url(),
                                                  kNoTabId,
-                                                 script,
+                                                 script_or_null,
                                                  kNoProcessId,
                                                  NULL)) {
       continue;