NoState Prefetch: nostate prefetch browser tests.

chrome/browser/prerender/prerender_test_utils.h

Index: chrome/browser/prerender/prerender_test_utils.h
diff --git a/chrome/browser/prerender/prerender_test_utils.h b/chrome/browser/prerender/prerender_test_utils.h
index 280cde00336ed448eb263eb1673394b9a3cd5ab7..777f18dc1b052b444770b22f61d726340264b17e 100644
--- a/chrome/browser/prerender/prerender_test_utils.h
+++ b/chrome/browser/prerender/prerender_test_utils.h
@@ -5,10 +5,14 @@
 #ifndef CHROME_BROWSER_PRERENDER_PRERENDER_TEST_UTILS_H_
 #define CHROME_BROWSER_PRERENDER_PRERENDER_TEST_UTILS_H_
 
+#include <functional>
+
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
+#include "base/test/histogram_tester.h"
 #include "chrome/browser/external_protocol/external_protocol_handler.h"
 #include "chrome/browser/prerender/prerender_contents.h"
+#include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/safe_browsing_db/test_database_manager.h"
@@ -182,7 +186,47 @@ class TestPrerender : public PrerenderContents::Observer,
   DISALLOW_COPY_AND_ASSIGN(TestPrerender);
 };
 
-// PrerenderManager that uses TestPrerenderContents.
+// Blocks until a TestPrerenderContents has been destroyed with the given final
+// status. Should be created with a TestPrerenderContents, and then
+// WaitForDestroy should be called and its return value checked.
+class DestructionWaiter {
+ public:
+  // Does not own the prerender_contents, which must outlive any call to
+  // WaitForDestroy().
+  DestructionWaiter(TestPrerenderContents* prerender_contents,
+                    FinalStatus expected_final_status);
+
+  ~DestructionWaiter();
+
+  // Returns true if the TestPrerenderContents was destroyed with the correct
+  // final status, or false otherwise. Note this also may hang if the contents
+  // is never destroyed (which will presumably cause the test to time out).
+  bool WaitForDestroy();
+
+ private:
+  class DestructionMarker : public PrerenderContents::Observer {
+   public:
+    // Does not own the waiter which must outlive the TestPrerenderContents.
+    explicit DestructionMarker(DestructionWaiter* waiter);
+
+    ~DestructionMarker() override;
+
+    void OnPrerenderStop(PrerenderContents* contents) override;
+
+   private:
+    DestructionWaiter* waiter_;
+  };
+
+  // To be called by a DestructionMarker.
+  void MarkDestruction(FinalStatus reason);
+
+  base::RunLoop wait_loop_;
+  FinalStatus expected_final_status_;
+  bool saw_correct_status_;
+  std::unique_ptr<DestructionMarker> marker_;
+};
+
+// PrerenderContentsFactory that uses TestPrerenderContents.
 class TestPrerenderContentsFactory : public PrerenderContents::Factory {
  public:
   TestPrerenderContentsFactory();
@@ -214,6 +258,26 @@ class TestPrerenderContentsFactory : public PrerenderContents::Factory {
   std::deque<ExpectedContents> expected_contents_queue_;
 };
 
+// A PrerenderManager with custom time handling. PrerenderInProcessBrowserTest
+// will set the testing factor for the PrerenderManagerFactory.
+class TestPrerenderManager : public PrerenderManager {
+ public:
+  explicit TestPrerenderManager(Profile* profile);
+
+  // Returns the real current time, or a time set by AdvanceTime.
+  base::Time GetCurrentTime() const override;
+  base::TimeTicks GetCurrentTimeTicks() const override;
+
+  // Advance time by |delta|.  Once this is called, the real time is no longer
+  // used, and GetCurrentTime*() will only reflect changes by AdvanceTime().
+  void AdvanceTime(base::TimeDelta delta);
+
+ private:
+  base::Time time_;
+  base::TimeTicks time_ticks_;
+  base::TimeDelta delta_;
+};
+
 class PrerenderInProcessBrowserTest : virtual public InProcessBrowserTest {
  public:
   PrerenderInProcessBrowserTest();
@@ -226,6 +290,11 @@ class PrerenderInProcessBrowserTest : virtual public InProcessBrowserTest {
   void SetUpOnMainThread() override;
   content::SessionStorageNamespace* GetSessionStorageNamespace() const;
 
+  // Many of the file and server manipulation commands are fussy about paths
+  // being relative or absolute. This makes path absolute if it is not
+  // already. The path must not be empty.
+  std::string MakeAbsolute(const std::string& path);
+
   bool UrlIsInPrerenderManager(const std::string& html_file) const;
   bool UrlIsInPrerenderManager(const GURL& url) const;
 
@@ -234,6 +303,9 @@ class PrerenderInProcessBrowserTest : virtual public InProcessBrowserTest {
   content::WebContents* GetActiveWebContents() const;
 
   PrerenderManager* GetPrerenderManager() const;
+  TestPrerenderManager* GetTestPrerenderManager() const {
+    return static_cast<TestPrerenderManager*>(GetPrerenderManager());

[pasko, 2016/09/30 18:29:21]

Problems with having a new TestPrerenderManager as far as I can see:
* This downcast
* Hard to follow differences between TestPrerenderManager::AdvanceTime() and
UnitTestPrerenderManager::AdvanceTime(), having AdvanceTimeTicks() in one and
not having it in another
* crazy static testing_create_function_ - not for limited brains like mine

A simpler solution is to have a utility class that can be mixed into the
whatever prerendermanager we have.

class SyntheticTimeOverride {
 public:
  SyntheticTimeOverride(base::Time, base::TimeTicks);
  GetCurrentTime();
  GetCurrentTimeTicks();
 private:
  ...
};

In PrerenderManager we would have:

void SetSyntheticTimeForTesting(std::unique_ptr<SyntheticTimeOverride>);

void GetCurrentTime() {
  if (synthetic_time_override_)
    return synthetic_time_override_->GetCurrentTime();
}

As usual, would be glad to know why this might not work :)

[mattcary, 2016/10/03 10:58:35]

I really dislike mixing testing overrides into production code, especially for
things like time management which is hard enough to do right just one time. So
the dependency injection style of testing works well from this perspective. 

I agree there is extra cognitive load to having different test time adjustments
in the unittests vs the browser tests. But, OTOH, one would expect the test
setup to be different.

But, you know, whatever. If you feel that the advantages to something like this
outweigh this disadvantage of having testing hooks live in production code I can
change it.

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

Yes please. I agree that mocking out methods for testing via overrides is a good
tool, but when it requires messing up with complicated initialization order
versus three injection checks in production code, I am voting for injection.

+  }
 
   TestPrerenderContents* GetPrerenderContentsFor(const GURL& url) const;
 
@@ -242,20 +314,43 @@ class PrerenderInProcessBrowserTest : virtual public InProcessBrowserTest {
       FinalStatus expected_final_status,
       int expected_number_of_loads);
 
+  std::unique_ptr<TestPrerender> PrerenderTestURL(
+      const GURL& url,
+      FinalStatus expected_final_status,
+      int expected_number_of_loads);
+
   ScopedVector<TestPrerender> PrerenderTestURL(
       const std::string& html_file,
       const std::vector<FinalStatus>& expected_final_status_queue,
       int expected_number_of_loads);
 
-  std::unique_ptr<TestPrerender> PrerenderTestURL(
-      const GURL& url,
-      FinalStatus expected_final_status,
-      int expected_number_of_loads);
+  // Set up an HTTPS server. The lambda passed in is run after the server is
+  // created but before Start() is called.
+  void UseHttpsSrcServer(std::function<void(net::EmbeddedTestServer*)> startup);

[pasko, 2016/09/30 18:29:21]

This form with std::function is not called anywhere except with the trivial
lambda below. Hard to tell why you need it when reviewing this. Please remove it
in this change and re-introduce in the change that actually needs it. We will be
able to discuss alternatives there.

[mattcary, 2016/10/03 10:58:35]

Oops, I guess this was a git screwup to have this appear here.

However, AFAIU std:function is the right way to declare lambda types, as there
is no direct lambda declaration.

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

Yes, the type of lambda is hidden from the language. I never understood it,
since the debuggers will need to print this type anyway.

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

Acknowledged.

+  void UseHttpsSrcServer() {
+    UseHttpsSrcServer([](net::EmbeddedTestServer*) {});
+  }
+
+  // Returns the currently active server (see UseHttpsSrcServer).

[pasko, 2016/09/30 18:29:21]

nit: 
// Returns the currently active server. See UseHttpsSrcServer().

[mattcary, 2016/10/03 10:58:35]

Done.

+  net::EmbeddedTestServer* src_server() {

[pasko, 2016/09/30 18:29:21]

non-trivial implementation -> .cc

[mattcary, 2016/10/03 10:58:35]

Done.

+    if (https_src_server_)
+      return https_src_server_.get();
+    return embedded_test_server();
+  }
 
   safe_browsing::TestSafeBrowsingServiceFactory* safe_browsing_factory() const {
     return safe_browsing_factory_.get();
   }
 
+  test_utils::FakeSafeBrowsingDatabaseManager*
+  GetFakeSafeBrowsingDatabaseManager() {

[pasko, 2016/09/30 18:29:21]

non-trivial implementation -> .cc

[mattcary, 2016/10/03 10:58:35]

Done.

+    return static_cast<test_utils::FakeSafeBrowsingDatabaseManager*>(
+        safe_browsing_factory()
+            ->test_safe_browsing_service()
+            ->database_manager()
+            .get());
+  }
+
   TestPrerenderContentsFactory* prerender_contents_factory() const {
     return prerender_contents_factory_;
   }
@@ -268,6 +363,23 @@ class PrerenderInProcessBrowserTest : virtual public InProcessBrowserTest {
     return explicitly_set_browser_ ? explicitly_set_browser_ : browser();
   }
 
+  const base::HistogramTester& histogram_tester() { return histogram_tester_; }
+
+  // Returns a string for pattern-matching TaskManager tab entries.
+  base::string16 MatchTaskManagerTab(const char* page_title);
+
+  // Returns a string for pattern-matching TaskManager prerender entries.
+  base::string16 MatchTaskManagerPrerender(const char* page_title);
+
+ protected:
+  // To be called from PrerenderTestUrlImpl. Sets up the appropraite prerenders,
+  // checking for the expected final status, navigates to the loader url, and
+  // waits for the load.
+  ScopedVector<TestPrerender> NavigateWithPrerenders(
+      const GURL& loader_url,
+      const std::vector<FinalStatus>& expected_final_status_queue,
+      int expected_number_of_loads);
+
  private:
   // Implement load of a url for a prerender test. prerender_url should be
   // loaded, and we should expect to see one prerenderer created, and exit, for
@@ -287,6 +399,8 @@ class PrerenderInProcessBrowserTest : virtual public InProcessBrowserTest {
   TestPrerenderContentsFactory* prerender_contents_factory_;
   Browser* explicitly_set_browser_;
   bool autostart_test_server_;
+  base::HistogramTester histogram_tester_;
+  std::unique_ptr<net::EmbeddedTestServer> https_src_server_;
 };
 
 // Makes |url| respond to requests with the contents of |file|, counting the