NoState Prefetch: nostate prefetch browser tests.

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
new file mode 100644
index 0000000000000000000000000000000000000000..1766bcdfe9bf9a72f028529b07ef8451ede6a224
--- /dev/null
+++ b/chrome/browser/prerender/prerender_nostate_prefetch_test.cc
@@ -0,0 +1,473 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[pasko, 2016/10/03 18:54:17]

nit: prerender_nostate_prefetch_browsertest.cc

[mattcary, 2016/10/04 08:25:12]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/command_line.h"
+#include "base/strings/string_split.h"
+#include "chrome/browser/prerender/prerender_manager.h"
+#include "chrome/browser/prerender/prerender_manager_factory.h"
+#include "chrome/browser/prerender/prerender_test_utils.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/task_manager/task_manager_browsertest_util.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/grit/generated_resources.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/common/content_switches.h"
+#include "content/public/common/url_constants.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/base/escape.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/request_handler_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+
+using prerender::test_utils::CreateCountingInterceptorOnIO;
+using prerender::test_utils::DestructionWaiter;
+using prerender::test_utils::RequestCounter;
+using prerender::test_utils::TestPrerender;
+using prerender::test_utils::TestPrerenderContents;
+using task_manager::browsertest_util::WaitForTaskManagerRows;
+
+namespace {
+// 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
+// unchanged. The function checks that script execution works.
+bool GetJavascriptBoolean(const std::string& javascript_variable,
+                          content::WebContents* web_contents,
+                          bool* value) {
+  // In order to detect unknown variables a three-valued return is needed.
+  int result;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractInt(
+      web_contents,
+      "try { if (" + javascript_variable + ") { " +
+          "window.domAutomationController.send(1) } else { " +
+          "window.domAutomationController.send(0); } } catch(err) {" +
+          "window.domAutomationController.send(2) }",
+      &result));
+  if (result == 2) {
+    // This means an exception was caught, usually because of a missing
+    // variable.
+    return false;
+  }
+  *value = (result == 1);
+  return true;
+}
+
+// As above, but just checks for a missing variable.
+bool JavascriptVariableMissing(const std::string& javascript_variable,
+                               content::WebContents* web_contents) {
+  bool unused;
+  return !GetJavascriptBoolean(javascript_variable, web_contents, &unused);
+}
+
+}  // namespace
+
+namespace prerender {
+
+// The various uses of URLs in these tests (the test server, request capture,
+// etc) are fussy about relative versus absolute paths. With the exception of

[pasko, 2016/10/03 18:54:17]

I cannot see why the first sentence in this comment is useful. It may get out of
date because it is not easy to confirm that it is still true. Maybe remove it
and slightly rephrase the second sentence?

[mattcary, 2016/10/04 08:25:12]

Why don't you give me the exact wording you want--that will be faster.

[pasko, 2016/10/04 11:52:22]

.. because it is more to write? The explanation of why I want it this way is
still needed, I think. Plus I did not have much time before leaving in the
evening, sorry about that.

Here you are:
\begin{replacement comment}
// These URLs are relative with the exception of |PrefetchLoaderPath|, which is
// only used in |PrerenderTestURLImpl()|.
\end{replacement comment}

This is to replace the whole comment from line 71 to line 74, both boundaries
included.

I also formatted the text to fit 80 char columns o save you from pressing a
complicated Emacs keystroke. Hopefully now it is clearer and hence super fast.

[mattcary, 2016/10/04 16:24:17]

Done, with a slight clarification to make it more clear for authors of future
tests.

+// kPrefetchLoaderPath, which is only used in PrerenderTestURLImpl, all other
+// paths should be relative.
+const char kPrefetchImagePage[] = "prerender/prefetch_image.html";
+const char kPrefetchLoopPage[] = "prerender/prefetch_loop.html";
+const char kPrefetchJpeg[] = "prerender/image.jpeg";
+const char kPrefetchLoaderPath[] = "/prerender/prefetch_loader.html";
+const char kPrefetchPage[] = "prerender/prefetch_page.html";
+const char kPrefetchPage2[] = "prerender/prefetch_page2.html";
+const char kPrefetchPng[] = "prerender/image.png";
+const char kPrefetchScript[] = "prerender/prefetch.js";
+const char kPrefetchScript2[] = "prerender/prefetch2.js";
+const char kPrefetchSubresourceRedirectPage[] =
+    "prerender/prefetch_subresource_redirect.html";
+
+const char kPageBool[] = "pageBool";
+const char kScriptBool[] = "scriptBool";
+
+class NoStatePrefetchBrowserTest
+    : public test_utils::PrerenderInProcessBrowserTest {
+ public:
+  NoStatePrefetchBrowserTest() {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    PrerenderInProcessBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(
+        switches::kPrerenderMode, switches::kPrerenderModeSwitchValuePrefetch);
+  }
+
+  // Set up a request counter for the path.
+  void CountRequestFor(const std::string& path, RequestCounter* counter) {
+    GURL::Replacements replacement;
+    replacement.SetPathStr(path);
+    const GURL url = src_server()->base_url().ReplaceComponents(replacement);
+    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()));
+  }
+
+ private:
+  ScopedVector<TestPrerender> PrerenderTestURLImpl(
+      const GURL& prerender_url,
+      const std::vector<FinalStatus>& expected_final_status_queue,
+      int expected_number_of_loads) override {
+    base::StringPairs replacement_text;
+    replacement_text.push_back(
+        make_pair("REPLACE_WITH_PREFETCH_URL", prerender_url.spec()));
+    std::string replacement_path;
+    net::test_server::GetFilePathWithReplacements(
+        kPrefetchLoaderPath, replacement_text, &replacement_path);
+    GURL loader_url = src_server()->GetURL(replacement_path);
+
+    ScopedVector<TestPrerender> prerenders = NavigateWithPrerenders(
+        loader_url, expected_final_status_queue, expected_number_of_loads);
+
+    TestPrerenderContents* prerender_contents = prerenders[0]->contents();
+    CHECK(prerender_contents);
+    // Checks that the prerender contents final status is unchanged from its
+    // default value, meaning that the contents has not been destroyed.
+    EXPECT_EQ(FINAL_STATUS_MAX, prerender_contents->final_status());
+
+    EXPECT_EQ(expected_number_of_loads, prerenders[0]->number_of_loads());
+
+    return prerenders;
+  }
+};
+
+// Perform a full load of the target page and check that javascript values are

[pasko, 2016/10/03 18:54:17]

nit: s/Perform/Performs/

[mattcary, 2016/10/04 08:25:12]

Done.

+// set as expected. This confirms that our test system is working correctly, so
+// that when the target page is prefetched it can be confirmed that javascript
+// is not executed.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, CheckJavascript) {
+  ui_test_utils::NavigateToURL(
+      current_browser(), src_server()->GetURL(MakeAbsolute(kPrefetchPage)));
+  content::WebContents* web_contents =
+      current_browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Confirm we can get true and false values.
+  bool value = false;
+  EXPECT_TRUE(GetJavascriptBoolean(kPageBool, web_contents, &value));
+  EXPECT_TRUE(value);
+  value = true;
+  EXPECT_TRUE(GetJavascriptBoolean("pageAntiBool", web_contents, &value));
+  EXPECT_FALSE(value);
+
+  // Confirm a value from the script is plumbed through.
+  value = false;
+  EXPECT_TRUE(GetJavascriptBoolean(kScriptBool, web_contents, &value));
+  EXPECT_TRUE(value);
+
+  // Confirm that the expected happens when a value doesn't exist.
+  EXPECT_TRUE(JavascriptVariableMissing("iDontExist", web_contents));
+}
+
+// Checks that a page is correctly prefetched in the case of a
+// <link rel=prerender> tag and then loaded into a tab in response to a
+// navigation, when NoState Prefetch is enabled, but that the page is not loaded
+// (which confirmed by checking that javascript is not executed).
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchSimple) {
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+  RequestCounter main_counter;
+  CountRequestFor(kPrefetchPage, &main_counter);
+
+  std::unique_ptr<TestPrerender> test_prerender =
+      PrerenderTestURL(kPrefetchPage, FINAL_STATUS_APP_TERMINATING, 1);
+  main_counter.WaitForCount(1);
+  script_counter.WaitForCount(1);
+
+  content::WebContents* contents =
+      test_prerender->contents()->prerender_contents();
+  content::WebContents* active_contents =
+      current_browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_TRUE(JavascriptVariableMissing(kPageBool, contents));
+  EXPECT_TRUE(JavascriptVariableMissing(kScriptBool, contents));
+  EXPECT_TRUE(JavascriptVariableMissing(kPageBool, active_contents));
+  EXPECT_TRUE(JavascriptVariableMissing(kScriptBool, active_contents));
+  histogram_tester().ExpectTotalCount("Prerender.PerceivedPLT", 1);
+  histogram_tester().ExpectBucketCount(
+      "Prerender.websame_NoStatePrefetchResponseTypes",
+      4 /* cacheable, main resource */, 1);
+  histogram_tester().ExpectBucketCount(
+      "Prerender.websame_NoStatePrefetchResponseTypes",
+      0 /* cacheable, non-main resource */, 1);
+  histogram_tester().ExpectTotalCount("Prerender.none_MainResourceRedirects",
+                                      0);
+  histogram_tester().ExpectTotalCount("Prerender.none_SubResourceRedirects", 0);
+}
+
+// Checks the TTFCP histograms on reuse of a prefetch.

[pasko, 2016/10/03 18:54:17]

// Checks the TTFCP histograms on fast reuse of a prefetch.
IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchReusedWarm) {

[mattcary, 2016/10/04 08:25:12]

Done.

+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest,
+                       PrefetchReusedWarmHistograms) {
+  PrerenderTestURL(kPrefetchPage, FINAL_STATUS_APP_TERMINATING, 1);
+  GetTestPrerenderManager()->AdvanceTime(base::TimeDelta::FromSeconds(10));
+  ui_test_utils::NavigateToURL(
+      current_browser(), src_server()->GetURL(MakeAbsolute(kPrefetchPage)));
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_NoStatePrefetchTTFCP.Warm.Cacheable", 1);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_NoStatePrefetchTTFCP.Cold.Cacheable", 0);
+}
+
+// Checks the histograms on reuse of a prefetch.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest,
+                       PrefetchReusedColdHistograms) {
+  PrerenderTestURL(kPrefetchPage, FINAL_STATUS_TIMED_OUT, 1);
+  GetTestPrerenderManager()->AdvanceTime(base::TimeDelta::FromSeconds(600));
+  ui_test_utils::NavigateToURL(
+      current_browser(), src_server()->GetURL(MakeAbsolute(kPrefetchPage)));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_NoStatePrefetchTTFCP.Warm.Cacheable", 0);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_NoStatePrefetchTTFCP.Cold.Cacheable", 1);
+}
+
+// Checks the prefetch of an img tag.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchImage) {
+  RequestCounter image_counter;
+  CountRequestFor(kPrefetchJpeg, &image_counter);
+  base::StringPairs replacement_text;
+  replacement_text.push_back(
+      std::make_pair("REPLACE_WITH_IMAGE_URL", MakeAbsolute(kPrefetchJpeg)));
+  std::string main_page_path;
+  net::test_server::GetFilePathWithReplacements(
+      kPrefetchImagePage, replacement_text, &main_page_path);
+  // Note that we can't CountRequestFor the main page as we use the test server
+  // for handling the image url replacement.
+  PrerenderTestURL(main_page_path, FINAL_STATUS_APP_TERMINATING, 1);
+  image_counter.WaitForCount(1);
+}
+
+// Checks that a cross-domain prefetching works correctly by looking at
+// histograms.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchCrossDomain) {
+  static const std::string secondary_domain = "www.foo.com";
+  host_resolver()->AddRule(secondary_domain, "127.0.0.1");
+  GURL cross_domain_url(base::StringPrintf(
+      "http://%s:%d/%s", secondary_domain.c_str(),
+      embedded_test_server()->host_port_pair().port(), kPrefetchPage));
+  PrerenderTestURL(cross_domain_url, FINAL_STATUS_APP_TERMINATING, 1);
+  histogram_tester().ExpectTotalCount("Prerender.PerceivedPLT", 1);

[pasko, 2016/10/03 18:54:17]

would it be possible to check the Prefetch histograms here instead of the pure
Prerender ones? Is it A TODO?

PLT is being phased out, so let's try not to add dependencies on PerceivedPLT as
well.

[mattcary, 2016/10/04 08:25:12]

There are PLT histograms everywhere, let's do that in a different CL. When this
CL was started we didn't have any of the new histograms in.

[pasko, 2016/10/04 11:52:22]

I was not aware that this requires new histograms. If there are indeed no
prefetch histograms to check this, then let's have PerceivedPLT for now. I
already added a task for removing this dependency.

[mattcary, 2016/10/04 16:24:17]

Acknowledged.

+  histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.webcross_PrerenderNotSwappedInPLT", 1);
+}
+
+// Checks that we support simultaneous prefetch.

[pasko, 2016/10/03 18:54:17]

I did not realize that we allow it. Sounds like a bunch of link rel=prerenders
can evict whole lots of tabs from memory and bring the browser to its knees.
Should he have an item to fix it?

[mattcary, 2016/10/04 08:25:12]

Yup, probably.... I feel like there are several tests here that document current
behavior that we will want to change.

[pasko, 2016/10/04 11:52:22]

OK, added this as our future task. Could be a TODO, but those are harder to
track.

[mattcary, 2016/10/04 16:24:17]

Acknowledged.

+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchSimultaneous) {
+  RequestCounter first_main_counter;
+  CountRequestFor(kPrefetchPage, &first_main_counter);
+  RequestCounter second_main_counter;
+  CountRequestFor(kPrefetchPage2, &second_main_counter);
+  RequestCounter first_script_counter;
+  CountRequestFor(kPrefetchScript, &first_script_counter);
+  RequestCounter second_script_counter;
+  CountRequestFor(kPrefetchScript2, &second_script_counter);
+
+  // The first prerender is marked as canceled as when the second starts, it
+  // sees that the first has been abandoned (presumably because we detach
+  // immediately and so it dies quickly).
+  PrerenderTestURL(kPrefetchPage, FINAL_STATUS_CANCELLED, 1);
+  PrerenderTestURL(kPrefetchPage2, FINAL_STATUS_APP_TERMINATING, 1);
+  first_main_counter.WaitForCount(1);
+  second_main_counter.WaitForCount(1);
+  first_script_counter.WaitForCount(1);
+  second_script_counter.WaitForCount(1);
+  histogram_tester().ExpectTotalCount("Prerender.PerceivedPLT", 2);
+  histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 2);
+}
+
+// Checks that we correctly handle a prefetch to a nonexisting page.

[pasko, 2016/10/03 18:54:17]

'correctly handle' is too general. Please avoid 'we' here and in all comments
below, thanks ;)

[mattcary, 2016/10/04 08:25:12]

Done.

+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchNonexisting) {
+  PrerenderTestURL("nonexisting-page.html", FINAL_STATUS_APP_TERMINATING, 0);
+  // TODO(mattcary): we fire up a prerenderer before we discover that the main
+  // page doesn't exist, we still count this as a prerender. Also we don't fail
+  // the renderer (presumably because we've detached the resource, etc). Is this
+  // what we want?

[pasko, 2016/10/03 18:54:17]

Will this leave the renderer up until it expires?

I think counting it is fine, but there is an implicit TODO to kill the renderer
in this case, which is not killed automatically, I assume.

Meta-question: will these tests accumulate many renderers when run? If so, may
make some other tests flaky by sitting there, occupying memory and making OOM
killers happy.

[mattcary, 2016/10/04 08:25:12]

Yeah, see comment above.
As far as I understand these browser tests, we spin up a new chrome instance for
each test. So all processes are killed and none of the tests should interfere
with each other.

[pasko, 2016/10/04 11:52:22]

Acknowledged.

+  histogram_tester().ExpectTotalCount("Prerender.PerceivedPLT", 1);
+}
+
+// Checks that we follow a 301 redirect.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, Prefetch301Redirect) {
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+  PrerenderTestURL(
+      "/server-redirect/?" +
+          net::EscapeQueryParamValue(MakeAbsolute(kPrefetchPage), false),
+      FINAL_STATUS_APP_TERMINATING, 1);
+  script_counter.WaitForCount(1);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_NoStatePrefetchMainResourceRedirects", 1);
+}
+
+// Checks that we follow a subresource 301 redirect.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, Prefetch301Subresource) {
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+  PrerenderTestURL(kPrefetchSubresourceRedirectPage,
+                   FINAL_STATUS_APP_TERMINATING, 1);
+  script_counter.WaitForCount(1);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_NoStatePrefetchMainResourceRedirects", 1);
+}
+
+// Checks that we don't follow a client redirect.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchClientRedirect) {
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+  // We use a sentinel via a complete load kPrefetchPage2. Otherwise we end
+  // before script_counter would reliable see the load of kPrefetchScript, were
+  // it to happen.
+  RequestCounter sentinel_counter;
+  CountRequestFor(kPrefetchScript2, &sentinel_counter);
+  PrerenderTestURL(
+      "/client-redirect/?" +
+          net::EscapeQueryParamValue(MakeAbsolute(kPrefetchPage), false),
+      FINAL_STATUS_APP_TERMINATING, 1);
+  ui_test_utils::NavigateToURL(
+      current_browser(), src_server()->GetURL(MakeAbsolute(kPrefetchPage2)));
+  sentinel_counter.WaitForCount(1);
+  script_counter.WaitForCount(0);
+}
+
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchHttps) {
+  UseHttpsSrcServer();
+  RequestCounter main_counter;
+  CountRequestFor(kPrefetchPage, &main_counter);
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+  PrerenderTestURL(kPrefetchPage, FINAL_STATUS_APP_TERMINATING, 1);
+  main_counter.WaitForCount(1);
+  script_counter.WaitForCount(1);
+}
+
+// Checks that if an SSL error happens we don't fetch.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, SSLError) {
+  // We only send the loaded page, not the loader, through SSL.
+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  https_server.SetSSLConfig(net::EmbeddedTestServer::CERT_MISMATCHED_NAME);
+  https_server.ServeFilesFromSourceDirectory("chrome/test/data");
+  ASSERT_TRUE(https_server.Start());
+  std::unique_ptr<TestPrerender> prerender =
+      PrerenderTestURL(https_server.GetURL(MakeAbsolute(kPrefetchPage)),
+                       FINAL_STATUS_SSL_ERROR, 0);
+  DestructionWaiter waiter(prerender->contents(), FINAL_STATUS_SSL_ERROR);
+  EXPECT_TRUE(waiter.WaitForDestroy());
+}
+
+// We should not have a problem if a subresource fails SSL.

[pasko, 2016/10/03 18:54:17]

'a problem' is a bit too general, how can we clarify?

// Checks that a subresource with an SSL error does not stop other subresources
from being prefetched.

?

[mattcary, 2016/10/04 08:25:12]

Done.

+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, SSLSubresourceError) {
+  // First confirm that the image loads as expected.
+
+  // Note that we start a separate HTTPS server for the subresource;
+  // src_server() is non-HTTPS.
+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  https_server.SetSSLConfig(net::EmbeddedTestServer::CERT_MISMATCHED_NAME);
+  https_server.ServeFilesFromSourceDirectory("chrome/test/data");
+  ASSERT_TRUE(https_server.Start());
+  GURL https_url = https_server.GetURL("/prerender/image.jpeg");
+  base::StringPairs replacement_text;
+  replacement_text.push_back(
+      std::make_pair("REPLACE_WITH_IMAGE_URL", https_url.spec()));
+  std::string main_page_path;
+  net::test_server::GetFilePathWithReplacements(
+      kPrefetchImagePage, replacement_text, &main_page_path);
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+
+  std::unique_ptr<TestPrerender> prerender =
+      PrerenderTestURL(main_page_path, FINAL_STATUS_APP_TERMINATING, 1);
+  // Checks that the presumed failure of the image load didn't affect the script
+  // fetch. This assumes waiting for the script load is enough to see any error
+  // from the image load.
+  script_counter.WaitForCount(1);
+}
+
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, Loop) {
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+  RequestCounter main_counter;
+  CountRequestFor(kPrefetchLoopPage, &main_counter);
+
+  std::unique_ptr<TestPrerender> test_prerender =
+      PrerenderTestURL(kPrefetchLoopPage, FINAL_STATUS_APP_TERMINATING, 1);
+  main_counter.WaitForCount(1);
+  script_counter.WaitForCount(1);
+}
+
+#if defined(ENABLE_TASK_MANAGER)
+
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest,
+                       OpenTaskManagerBeforePrefetch) {
+  const base::string16 any_prerender = MatchTaskManagerPrerender("*");
+  const base::string16 any_tab = MatchTaskManagerTab("*");
+  const base::string16 original = MatchTaskManagerTab("Prefetch Loader");
+  // Presumably we don't see the title in the task manager as the page has not
+  // been fully parsed.
+  const base::string16 prerender =
+      MatchTaskManagerPrerender("*prefetch_page.html*");
+
+  // Show the task manager. This populates the model.
+  chrome::OpenTaskManager(current_browser());
+  ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, any_tab));
+  ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, any_prerender));
+
+  // Prerender a page in addition to the original tab.
+  PrerenderTestURL(kPrefetchPage, FINAL_STATUS_APP_TERMINATING, 1);
+
+  // A TaskManager entry should appear like "Prerender: Prerender Page"
+  // alongside the original tab entry. There should be just these two entries.
+  ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, prerender));
+  ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, original));
+  ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, any_prerender));
+  ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, any_tab));
+}
+
+#endif  // defined(ENABLE_TASK_MANAGER)
+
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, RendererCrash) {
+  std::unique_ptr<TestPrerender> prerender =
+      PrerenderTestURL(kPrefetchPage, FINAL_STATUS_RENDERER_CRASHED, 1);
+  prerender->contents()->prerender_contents()->GetController().LoadURL(
+      GURL(content::kChromeUICrashURL), content::Referrer(),
+      ui::PAGE_TRANSITION_TYPED, std::string());
+  prerender->WaitForStop();
+}
+
+// Checks that the prefetch of png correctly loads only the png.

[pasko, 2016/10/03 18:54:17]

is it essential to check different img types? Are we in fact testing that only
the png is prefetched?

If prefetch also had pulled the favicon? Would this test have noticed this?

[mattcary, 2016/10/04 08:25:12]

We do pull the favicon on prefetch, and we could check for that. Should I? None
of the prerender tests were concerned.

These are just copied from the prerender browsertests. It's probably worth
having one test were we try a prefetch on a non-html main resource, but I agree
it may be a little excessive to try many different formats, unless we know
there's some different code path in the preload scanner somehow.

[pasko, 2016/10/04 11:52:22]

OK, got it. This slightly excessive testing is OK with me, just wanted to know
where it is coming from because it seemed that I am missing something important.

So if we are pulling the favicon, and the test does not check all extra stuff
being pulled (I have no idea how to make a test that would check this), then we
should remove the word 'only' from the comment.

[mattcary, 2016/10/04 16:24:17]

Oh, I see what you mean. Done

+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, Png) {
+  RequestCounter counter;
+  CountRequestFor(kPrefetchPng, &counter);
+  PrerenderTestURL(kPrefetchPng, FINAL_STATUS_APP_TERMINATING, 1);
+  counter.WaitForCount(1);
+}
+
+// Checks that the prefetch of png correctly loads only the jpeg.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, Jpeg) {
+  RequestCounter counter;
+  CountRequestFor(kPrefetchJpeg, &counter);
+  PrerenderTestURL(kPrefetchJpeg, FINAL_STATUS_APP_TERMINATING, 1);
+  counter.WaitForCount(1);
+}
+
+// Checks that nothing is prefetched from malware sites.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest,
+                       PrerenderSafeBrowsingTopLevel) {
+  GURL url = src_server()->GetURL(MakeAbsolute(kPrefetchPage));
+  GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
+      url, safe_browsing::SB_THREAT_TYPE_URL_MALWARE);
+  // Prefetch resource are blocked, but the prerender is not killed in any

[pasko, 2016/10/03 18:54:17]

s/resource/resources/

[mattcary, 2016/10/04 08:25:12]

Done.

+  // special way.
+  // TODO(mattcary): do we care about detecting if the main resource is fetched

[pasko, 2016/10/03 18:54:17]

We should not prefetch the main resource to cache if it is blocked by SB. How
does regular Prerender solve this problem?

[mattcary, 2016/10/04 08:25:12]

The prerender terminates with FINAL_STATUS_SAFE_BROWSING. For us, however,
because the main resource fetch doesn't ever come back we just terminate. We
should probably investigate dying in a more controlled way, this should probably
be grouped under the general category of prefetch dying gracefully that has come
up a couple more times above (nonexisting pages, multiple prefetches, etc).

The TODO about testing for the main resource not being fetched is a test-harness
question: it's hard to confirm that a fetch does not happen. I can do it in some
instances, where some resources are being fetched and others are not, because,
for example, we can arrange for the second of two subresources to be fetched
while the first is not, and then we're confident that if we only see the second
fetch, the first one didn't happen (although even that is making assumptions
about serialization in the network stack that may not be valid).

In this case, when nothing at all is fetched, it's tricky to see what to do. In
particular, the implementation of the RequestCounter can't pump the message loop
(because there might not be any messages), but that means that the test could
terminate before we saw an erroneous fetch.

So, in summary, I think it would be a good idea to improve the test framework to
handle this sort of thing, but the amount of work makes me think we should start
performance testing first.

[pasko, 2016/10/04 11:52:22]

OK. Agreed.

Later we will need some sort of way to distinguish the reasons of the failed
prefetches, but polishing performance should not be blocked on it, at least for
now.

[mattcary, 2016/10/04 16:24:17]

Done.

+  // and preload scanning has started?
+  std::unique_ptr<TestPrerender> prerender =
+      PrerenderTestURL(kPrefetchPage, FINAL_STATUS_APP_TERMINATING, 0);
+}
+
+}  // namespace prerender