[NoStatePrefetch] Remove JS injection in tests

chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // 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/string16.h"
 #include "base/strings/string_split.h"
 #include "base/task_scheduler/post_task.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
@@ skipping 15 matching lines @@
 #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 {
 
 // These URLs used for test resources must be relative with the exception of
 // |PrefetchLoaderPath|, which is only used in |PrerenderTestURLImpl()|.
 const char kPrefetchImagePage[] = "prerender/prefetch_image.html";
 const char kPrefetchJpeg[] = "prerender/image.jpeg";
 const char kPrefetchLoaderPath[] = "/prerender/prefetch_loader.html";
 const char kPrefetchLoopPage[] = "prerender/prefetch_loop.html";
 const char kPrefetchMetaCSP[] = "prerender/prefetch_meta_csp.html";
 const char kPrefetchPage[] = "prerender/prefetch_page.html";
 const char kPrefetchPage2[] = "prerender/prefetch_page2.html";
 const char kPrefetchPng[] = "prerender/image.png";
 const char kPrefetchResponseHeaderCSP[] =
     "prerender/prefetch_response_csp.html";
 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:
   class BrowserTestTime : public PrerenderManager::TimeOverride {
    public:
     BrowserTestTime() {}
 
     base::Time GetCurrentTime() const override {
       if (delta_.is_zero()) {
         return base::Time::Now();
@@ skipping 88 matching lines @@
       EXPECT_EQ(FINAL_STATUS_MAX, prerender_contents->final_status());
     }
     EXPECT_EQ(expected_number_of_loads, prerenders[0]->number_of_loads());
 
     return prerenders;
   }
 
   BrowserTestTime* browser_test_time_;
 };
 
-// Performs a full load of the target page and check that javascript values are
-// 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();
-
-  // Confirms that true and false values can appear.
-  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).
+// <link rel=prerender> tag and the JavaScript on the page is not executed.
 IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchSimple) {
   RequestCounter script_counter;
-  CountRequestFor(kPrefetchScript, &script_counter);
+  CountRequestFor(kPrefetchScript2, &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));
+  EXPECT_EQ(0, script_counter.count());

[mattcary, 2016/10/20 07:40:53]

Better to do script_counter.WaitForCount(0). Even though right now it's
equivalent to what you're doing, in the future it might not be if we figure out
a way to reliably detect that scripts are not loaded.

[pasko, 2016/10/20 10:05:07]

Done.

 }
 
 // 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;
@@ skipping 65 matching lines @@
 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 it is detached
+  // The first prerender is marked as canceled when the second starts, it sees

[mattcary, 2016/10/20 07:40:53]

Agrammatical, you need something connecting the clauses or use a full stop and
then probably disambiguate the pronoun.

[pasko, 2016/10/20 10:05:07]

Thanks. I was confused by the initial form and indeed made things worse. PTAL at
the new wording with multiple sentences (I am still not sure I preserved the
original meaning, sry). I also removed 'presumably' because comments humans make
should be assumed to be about presumable matters by default.

[mattcary, 2016/10/20 10:09:40]

The current version SGTM.

Possibly my version could have been made more clear by adding a comma before the
second "as". But this is just fine as you've done it.

+  // that the first has been abandoned (presumably because it is detached
   // immediately and so 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);
 }
 
 // Checks a prefetch to a nonexisting page.
@@ skipping 184 matching lines @@
   // special way.
   // TODO(mattcary): since the prerender will count itself as loaded even if the
   // fetch of the main resource fails, the test doesn't actually confirm what we
   // want it to confirm. This may be fixed by planned changes to the prerender
   // lifecycle.
   std::unique_ptr<TestPrerender> prerender =
       PrerenderTestURL(kPrefetchPage, FINAL_STATUS_SAFE_BROWSING, 1);
 }
 
 }  // namespace prerender