predictors: Add browsertests that check fetching through javascript.

chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

Index: chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
index ded071a052341777e9943a51a92fa64a142da4f1..3419afa4db04082945d16a7f5d2ee54aa430a691 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
+++ b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
@@ -23,23 +23,45 @@ namespace predictors {
 
 namespace {
 
-static const char kImagePath[] = "/predictors/image.png";
-static const char kStylePath[] = "/predictors/style.css";
-static const char kScriptPath[] = "/predictors/script.js";
-static const char kFontPath[] = "/predictors/font.ttf";
+// Paths to resources handled by a custom request handler. They return empty
+// responses with controllable response headers.
+static const char kImagePath[] = "/handled-by-test/image.png";

[Benoit L, 2016/12/05 14:03:30]

nit: Why static here? (and below).

[alexilin, 2016/12/06 09:10:44]

Initially, to force internal linkage.
https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Stat...
But now I realized that anonymous namespace does the same thing so static is
really unnecessary. However, we still should prefer to use static when declaring
const locally. (As in
https://cs.chromium.org/chromium/src/chrome/browser/predictors/resource_prefe...)
Done.

+static const char kImagePath2[] = "/handled-by-test/image2.png";
+static const char kStylePath[] = "/handled-by-test/style.css";
+static const char kStylePath2[] = "/handled-by-test/style2.css";
+static const char kScriptPath[] = "/handled-by-test/script.js";
+static const char kScriptPath2[] = "/handled-by-test/script2.js";
+static const char kFontPath[] = "/handled-by-test/font.ttf";
+static const char kRedirectPath[] = "/handled-by-test/redirect.html";
+static const char kRedirectPath2[] = "/handled-by-test/redirect2.html";
+static const char kRedirectPath3[] = "/handled-by-test/redirect3.html";
+
+// These are loaded from a file by the test server.
 static const char kHtmlSubresourcesPath[] =
     "/predictors/html_subresources.html";
-static const char kRedirectPath[] = "/predictors/redirect.html";
-static const char kRedirectPath2[] = "/predictors/redirect2.html";
-static const char kRedirectPath3[] = "/predictors/redirect3.html";
+static const char kHtmlDocumentWritePath[] = "/predictors/document_write.html";
+static const char kScriptDocumentWritePath[] = "/predictors/document_write.js";
+static const char kHtmlAppendChildPath[] = "/predictors/append_child.html";
+static const char kScriptAppendChildPath[] = "/predictors/append_child.js";
+static const char kHtmlInnerHtmlPath[] = "/predictors/inner_html.html";
+static const char kScriptInnerHtmlPath[] = "/predictors/inner_html.js";
+static const char kHtmlFetchPath[] = "/predictors/fetch.html";
+static const char kScriptFetchPath[] = "/predictors/fetch.js";
+static const char kHtmlIframePath[] = "/predictors/html_iframe.html";
 
 struct ResourceSummary {
-  ResourceSummary() : is_no_store(false), version(0) {}
+  ResourceSummary()
+      : is_no_store(false),
+        version(0),
+        is_external(false),
+        should_be_recorded(true) {}
 
   ResourcePrefetchPredictor::URLRequestSummary request;
   std::string content;
   bool is_no_store;
   size_t version;
+  bool is_external;
+  bool should_be_recorded;
 };
 
 struct RedirectEdge {
@@ -152,7 +174,7 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {
     GURL endpoint_url = GetRedirectEndpoint(main_frame_url);
     std::vector<URLRequestSummary> url_request_summaries;
     for (const auto& kv : resources_) {
-      if (kv.second.is_no_store)
+      if (kv.second.is_no_store || !kv.second.should_be_recorded)
         continue;
       url_request_summaries.push_back(
           GetURLRequestSummaryForResource(endpoint_url, kv.second));
@@ -180,6 +202,23 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {
     return resource;
   }
 
+  ResourceSummary* AddExternalResource(const GURL& resource_url,
+                                       content::ResourceType resource_type,
+                                       net::RequestPriority priority) {
+    auto resource = AddResource(resource_url, resource_type, priority);
+    resource->is_external = true;
+    return resource;
+  }
+
+  void AddUnrecordedResources(const std::vector<GURL>& resource_urls) {
+    for (const GURL& resource_url : resource_urls) {
+      auto resource =
+          AddResource(resource_url, content::RESOURCE_TYPE_SUB_RESOURCE,
+                      net::DEFAULT_PRIORITY);
+      resource->should_be_recorded = false;
+    }
+  }
+
   void AddRedirectChain(const GURL& initial_url,
                         const std::vector<RedirectEdge>& redirect_chain) {
     ASSERT_FALSE(redirect_chain.empty());
@@ -267,6 +306,9 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {
       return nullptr;
 
     const ResourceSummary& summary = resource_it->second;
+    if (summary.is_external)
+      return nullptr;
+
     auto http_response =
         base::MakeUnique<net::test_server::BasicHttpResponse>();
     http_response->set_code(net::HTTP_OK);
@@ -370,4 +412,74 @@ IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
   NavigateToURLAndCheckSubresources(GetURL(kRedirectPath));
 }
 
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
+                       LearningJavascriptDocumentWrite) {
+  auto externalScript =
+      AddExternalResource(GetURL(kScriptDocumentWritePath),
+                          content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  externalScript->request.mime_type = "application/javascript";

[Benoit L, 2016/12/05 14:03:30]

nit: Define kJavascriptMime?

[alexilin, 2016/12/06 09:10:44]

Done.

+  AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
+  AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,
+              net::HIGHEST);
+  AddResource(GetURL(kScriptPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlDocumentWritePath));
+}
+
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
+                       LearningJavascriptAppendChild) {
+  auto externalScript =
+      AddExternalResource(GetURL(kScriptAppendChildPath),
+                          content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  externalScript->request.mime_type = "application/javascript";
+  AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
+  AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,
+              net::HIGHEST);
+  // This script has net::LOWEST priority because it's executed asynchronously.
+  AddResource(GetURL(kScriptPath), content::RESOURCE_TYPE_SCRIPT, net::LOWEST);
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlAppendChildPath));
+}
+
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
+                       LearningJavascriptInnerHtml) {
+  auto externalScript = AddExternalResource(
+      GetURL(kScriptInnerHtmlPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  externalScript->request.mime_type = "application/javascript";
+  AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
+  AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,
+              net::HIGHEST);
+  // https://www.w3.org/TR/2014/REC-html5-20141028/scripting-1.html#the-script-element
+  // Script elements don't execute when inserted using innerHTML attribute.
+  AddUnrecordedResources({GetURL(kScriptPath)});
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlInnerHtmlPath));
+}
+
+// Requests originated by XMLHttpRequest have content::RESOURCE_TYPE_XHR.

[Benoit L, 2016/12/05 14:03:30]

This is no longer true, right?

[alexilin, 2016/12/06 09:10:44]

Why not? Notice that this comment has been changed since previous diff.
However I think it would be better to change this test so that resources have
mime-type.

+// Since resources in this test don't have mime-type specified it's impossible
+// to infer the real type of resource so predictor ignores them.
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
+                       LearningJavascriptFetchIgnored) {
+  auto externalScript = AddExternalResource(
+      GetURL(kScriptFetchPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  externalScript->request.mime_type = "application/javascript";
+  AddUnrecordedResources(
+      {GetURL(kImagePath), GetURL(kStylePath), GetURL(kScriptPath)});
+  // TODO(alexilin): response for fetch request anyway arrives after main frame
+  // load completes. Find the way to delay loading until fetch completes.

[Benoit L, 2016/12/05 14:03:30]

This can be done in Javascript.
Sync XHR?
https://developer.mozilla.org/fr/docs/Web/API/XMLHttpRequest/Synchronous_and_...

[alexilin, 2016/12/06 09:10:44]

Done.

+  NavigateToURLAndCheckSubresources(GetURL(kHtmlFetchPath));
+}
+
+// ResourcePrefetchPredictor ignores all resources requested from subframes.
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
+                       LearningWithIframe) {
+  // Included from html_iframe.html.
+  AddResource(GetURL(kImagePath2), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
+  AddResource(GetURL(kStylePath2), content::RESOURCE_TYPE_STYLESHEET,
+              net::HIGHEST);
+  AddResource(GetURL(kScriptPath2), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  // Included from <iframe src="html_subresources.html"> and not recored.
+  AddUnrecordedResources({GetURL(kImagePath), GetURL(kStylePath),
+                          GetURL(kScriptPath), GetURL(kFontPath)});
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlIframePath));
+}
+
 }  // namespace predictors