Rework of ResourcePrefetchPredictor browser tests infrastructure.

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 91e3d93786e17c3e8a87925d0a4301fed1fa73b3..0f142962828c4b7f76792eb5a15fdca86e6025ad 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
+++ b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
@@ -107,6 +107,14 @@ std::vector<URLRequestSummary> GetUniqueSubresources(
   return subresources;
 }
 
+// Fill a NavigationID with "empty" data that does not trigger
+// the is_valid DCHECK(). Allows comparing.
+void EmptyNavigationId(NavigationID* navigation_id) {
+  navigation_id->render_process_id = 0;
+  navigation_id->render_frame_id = 0;
+  navigation_id->main_frame_url = GURL("http://127.0.0.1");
+}
+
 }  // namespace
 
 // Helper class to track and allow waiting for ResourcePrefetchPredictor events.
@@ -119,10 +127,12 @@ class ResourcePrefetchPredictorTestObserver : public TestObserver {
   explicit ResourcePrefetchPredictorTestObserver(
       ResourcePrefetchPredictor* predictor,
       const size_t expected_url_visit_count,
-      const PageRequestSummary& expected_summary)
+      const PageRequestSummary& expected_summary,
+      bool match_navigation_id)

[ahemery, 2016/12/07 14:13:02]

New test observer parameter. If true, when comparing result with reference we
match the navigation_id's

       : TestObserver(predictor),
         url_visit_count_(expected_url_visit_count),
-        summary_(expected_summary) {}
+        summary_(expected_summary),
+        match_navigation_id_(match_navigation_id) {}
 
   // TestObserver:
   void OnNavigationLearned(size_t url_visit_count,
@@ -132,10 +142,20 @@ class ResourcePrefetchPredictorTestObserver : public TestObserver {
     EXPECT_EQ(summary.initial_url, summary_.initial_url);
     // Duplicate resources can be observed in a single navigation but
     // ResourcePrefetchPredictor only cares about the first occurrence of each.
-    std::vector<ResourcePrefetchPredictor::URLRequestSummary> subresources =
-        GetUniqueSubresources(summary);
-    EXPECT_THAT(subresources, testing::UnorderedElementsAreArray(
-                                  summary_.subresource_requests));
+    std::vector<ResourcePrefetchPredictor::URLRequestSummary>
+        subresources_results = GetUniqueSubresources(summary);
+    std::vector<ResourcePrefetchPredictor::URLRequestSummary> subresources_ref(
+        summary_.subresource_requests);
+    if (!match_navigation_id_) {

[ahemery, 2016/12/07 14:13:02]

If we are not matching the navigation_id we empty all the id's. This is because
we cannot pass a custom comparison function to the GTEST matcher. Note the copy
of refs to avoid messing with the constness.

+      for (auto& subresource : subresources_ref) {
+        EmptyNavigationId(&subresource.navigation_id);
+      }
+      for (auto& subresource : subresources_results) {
+        EmptyNavigationId(&subresource.navigation_id);
+      }
+    }
+    EXPECT_THAT(subresources_results,
+                testing::UnorderedElementsAreArray(subresources_ref));
     run_loop_.Quit();
   }
 
@@ -145,6 +165,7 @@ class ResourcePrefetchPredictorTestObserver : public TestObserver {
   base::RunLoop run_loop_;
   size_t url_visit_count_;
   PageRequestSummary summary_;
+  bool match_navigation_id_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourcePrefetchPredictorTestObserver);
 };
@@ -173,7 +194,9 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {
     EnsurePredictorInitialized();
   }
 
-  void NavigateToURLAndCheckSubresources(const GURL& main_frame_url) {
+  void NavigateToURLAndCheckSubresources(
+      const GURL& main_frame_url,
+      WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB) {
     GURL endpoint_url = GetRedirectEndpoint(main_frame_url);
     std::vector<URLRequestSummary> url_request_summaries;
     for (const auto& kv : resources_) {
@@ -182,11 +205,23 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {
       url_request_summaries.push_back(
           GetURLRequestSummaryForResource(endpoint_url, kv.second));
     }
+
+    bool match_navigation_id =
+        (disposition == WindowOpenDisposition::CURRENT_TAB);
+    int wait_flags = ui_test_utils::BROWSER_TEST_NONE;
+    if (disposition == WindowOpenDisposition::NEW_FOREGROUND_TAB)
+      wait_flags = ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB;
+    if (disposition == WindowOpenDisposition::NEW_POPUP)
+      wait_flags = ui_test_utils::BROWSER_TEST_WAIT_FOR_BROWSER;
+
     ResourcePrefetchPredictorTestObserver observer(
         predictor_, UpdateAndGetVisitCount(main_frame_url),
         CreatePageRequestSummary(endpoint_url.spec(), main_frame_url.spec(),
-                                 url_request_summaries));
-    ui_test_utils::NavigateToURL(browser(), main_frame_url);
+                                 url_request_summaries),
+        match_navigation_id);
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser(), main_frame_url, disposition,
+        ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION | wait_flags);

[alexilin, 2016/12/07 16:57:11]

From offline discussion: we don't have to wait for loading completed in this
call because we have our own observer.
So we can just pass ui_test_utils::BROWSER_TEST_NONE flag.

     observer.Wait();
   }
 
@@ -489,4 +524,26 @@ IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
   NavigateToURLAndCheckSubresources(GetURL(kHtmlIframePath));
 }
 
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest, LearningInPopup) {

[ahemery, 2016/12/07 14:13:02]

New tests. Do the same as simple learning but in a popup or new tab

+  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);
+  AddResource(GetURL(kFontPath), content::RESOURCE_TYPE_FONT_RESOURCE,
+              net::HIGHEST);
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath),
+                                    WindowOpenDisposition::NEW_POPUP);
+}
+
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest, LearningInNewTab) {
+  AddResource(GetURL(kFontPath), content::RESOURCE_TYPE_FONT_RESOURCE,
+              net::HIGHEST);
+  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(kHtmlSubresourcesPath),
+                                    WindowOpenDisposition::NEW_FOREGROUND_TAB);
+}
+
 }  // namespace predictors