NoState Prefetch: CSP test

chrome/browser/prerender/prerender_nostate_prefetch_test.cc

Index: chrome/browser/prerender/prerender_nostate_prefetch_test.cc
diff --git a/chrome/browser/prerender/prerender_nostate_prefetch_test.cc b/chrome/browser/prerender/prerender_nostate_prefetch_test.cc
index c6a751dc5ac5827aa5ad900486802349458dd34f..c373f0865c6d13ab304c16e85e3301eff7217767 100644
--- a/chrome/browser/prerender/prerender_nostate_prefetch_test.cc
+++ b/chrome/browser/prerender/prerender_nostate_prefetch_test.cc
@@ -36,16 +36,18 @@ namespace prerender {
 // etc) are fussy about relative versus absolute paths. With the exception of
 // kPrefetchLoaderPath, which is only used in PrerenderTestURLImpl, all other
 // paths should be relative.
+const char kPrefetchMetaCSP[] = "prerender/prerender_prefetch_meta_csp.html";
+const char kPrefetchResponseHeaderCSP[] =
+    "prerender/prerender_prefetch_response_csp.html";
+const char kPrefetchImagePage[] = "prerender/prerender_prefetch_image.html";
+const char kPrefetchJpeg[] = "prerender/image.jpeg";
 const char kPrefetchLoaderPath[] = "/prerender/prerender_prefetch_loader.html";
+const char kPrefetchLoopPage[] = "prerender/prerender_prefetch_loop.html";
 const char kPrefetchPage[] = "prerender/prerender_prefetch_page.html";
-const char kPrefetchScript[] = "prerender/prerender_prefetch.js";
 const char kPrefetchPage2[] = "prerender/prerender_prefetch_page2.html";
-const char kPrefetchScript2[] = "prerender/prerender_prefetch2.js";
-const char kPrefetchImagePage[] = "prerender/prerender_prefetch_image.html";
-const char kPrefetchLoopPage[] = "prerender/prerender_prefetch_loop.html";
-
-const char kPrefetchJpeg[] = "prerender/image.jpeg";
 const char kPrefetchPng[] = "prerender/image.png";
+const char kPrefetchScript[] = "prerender/prerender_prefetch.js";
+const char kPrefetchScript2[] = "prerender/prerender_prefetch2.js";
 
 const char kPageBool[] = "pageBool";
 const char kScriptBool[] = "scriptBool";
@@ -74,6 +76,18 @@ class NoStatePrefetchBrowserTest
                    counter->AsWeakPtr()));
   }
 
+  // Set up a request counter for a specific URL, serving data from path.
+  void CountRequestFor(const GURL& url,

[pasko, 2016/09/26 17:05:10]

CountRequestForUrl() looks better than this overload, to make grepping easier.

[mattcary, 2016/09/27 08:13:18]

Done.

+                       const std::string& path,
+                       RequestCounter* counter) {
+    base::FilePath url_file =
+        ui_test_utils::GetTestFilePath(base::FilePath(), base::FilePath(path));
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&CreateCountingInterceptorOnIO, url, url_file,
+                   counter->AsWeakPtr()));
+  }
+
   // Fetches a boolean value from javascript. Returns whether the fetch
   // succeeded; the value of the variable is returned in value. If
   // javascript_variable does not exist, this returns false and value is
@@ -217,6 +231,47 @@ IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchCrossDomain) {
       "Prerender.webcross_PrerenderNotSwappedInPLT", 1);
 }
 
+// Check that response header CSP is handled correctly.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, ResponseHeaderCSP) {
+  static const std::string secondary_domain = "foo.bar";
+  host_resolver()->AddRule(secondary_domain, "127.0.0.1");
+  RequestCounter main_page;
+  CountRequestFor(kPrefetchResponseHeaderCSP, &main_page);
+  RequestCounter first_script;
+  CountRequestFor(kPrefetchScript, &first_script);
+  RequestCounter second_script;
+  GURL second_script_url(std::string("http://foo.bar/") + kPrefetchScript2);
+  CountRequestFor(second_script_url, kPrefetchScript2, &second_script);
+  PrerenderTestURL(kPrefetchResponseHeaderCSP, FINAL_STATUS_APP_TERMINATING, 1);
+  // The second script is in the correct domain for CSP, but the first script is
+  // not.
+  main_page.WaitForCount(1);
+  second_script.WaitForCount(1);
+  first_script.WaitForCount(0);

[pasko, 2016/09/26 17:05:10]

does WaitForCount actually check that it won't count up to 1 at some point
later?

[mattcary, 2016/09/27 08:13:18]

No. There doesn't seem to be an easy way to confirm that, especially since we
don't currently have any hooks into the prerenderer (for instance, it's not
obvious to me how to tell that the prefetch scanner had finished, so even if we
did hook into the queue of requests (which also seems hard), we couldn't be sure
it was finished.

Heuristically, by doing the WaitForCount(0) a the end, we ensure that the
message loop has at least been pumped, which has in my experience caught when a
request for the first script was actually made. But I suspect it's not reliable.

[pasko, 2016/10/05 14:09:55]

Agreed. I am working on signalling from the scanner on when it is done (needed
anyway to kill the renderer or give green light to a new prefetch, etc), it will
open the opportunity to check our counter after that signal arrives. For now I
think it is appropriate to add TODO(pasko): Wait for prefetch to be finished
before checking the counts, once the signal is available.

[mattcary, 2016/10/12 15:09:10]

Done.

+}
+
+// Check CSP in the meta tag cancels the prefetch.
+// TODO(mattcary): probably we don't want to cancel this so we're consistent

[pasko, 2016/09/26 17:05:10]

by 'this' do you mean the prefetch or the request? Why should the csp meta tag
cancel the whole prefetch and not just a request? Is it this particular csp meta
tag in the test that prevents the page from loading or you want it this way to
be on extremely safe side?

(I know, stupid questions in the evening..)

[mattcary, 2016/09/27 08:13:18]

Clarified the comment.

By "this" I mean the prerender. As soon as the prefetch scanner sees a meta csp
tag in the main resource, it bails, which means no prefetching is done at all.

In contrast, if there are CSP directives in the response headers, then the
appropriate tests are done.

I'm not sure what we want, this test is just documenting behavior for now. I
think the issue is that plumbing the CSP stuff from the background parser up to
where ever in the net stack it should be is hard.

Probably we should put this on the list to fix, but due to the apparent rarity
of the meta CSP tag I don't think this should block initial testing. I figured
I'd make a bug, etc, to do the right thing with CSP, as soon as we figured out
what that was, depending on comments from this CL.

[pasko, 2016/10/05 14:09:55]

Ah, good to know. It is indeed good to have a test for this early stop, to get
notified it changes in the parser.
Yes, I think it is hard to plumb it. And in any case, the recommended way to do
CSP seems to be over headers, not <meta>, so hopefully we get good coverage even
without this complexity.
Agreed to track it as low pri. Before going this rote let's see if it affects
performance. I added a task for this.

+// with response-header CSP. But it seems to be difficult, and probably meta tag
+// CSP is not as common, so it may not be worth the complexity.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, MetaTagCSP) {
+  static const std::string secondary_domain = "foo.bar";
+  host_resolver()->AddRule(secondary_domain, "127.0.0.1");
+  RequestCounter main_page;
+  CountRequestFor(kPrefetchMetaCSP, &main_page);
+  RequestCounter first_script;
+  CountRequestFor(kPrefetchScript, &first_script);
+  RequestCounter second_script;
+  GURL second_script_url(std::string("http://foo.bar/") + kPrefetchScript2);
+  CountRequestFor(second_script_url, kPrefetchScript2, &second_script);
+  PrerenderTestURL(kPrefetchMetaCSP, FINAL_STATUS_APP_TERMINATING, 1);
+  // Probably we should be loading the second script. But currently we bail on
+  // the preload scanner if we see the CSP meta tag

[pasko, 2016/09/26 17:05:10]

I am confused. What does it mean to 'bail out on the preload scanner'? Are you
describing intentional behavior and the current one? What are they?

[mattcary, 2016/09/27 08:13:18]

Clarified the comment. See above.

+  main_page.WaitForCount(1);
+  second_script.WaitForCount(0);
+  first_script.WaitForCount(0);
+}
+
 // Check that we support simultaneous prefetch.
 IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchSimultaneous) {
   RequestCounter first_main_counter;