predictors: Use redirect data in prefetch.

chrome/browser/predictors/resource_prefetch_predictor.h

Index: chrome/browser/predictors/resource_prefetch_predictor.h
diff --git a/chrome/browser/predictors/resource_prefetch_predictor.h b/chrome/browser/predictors/resource_prefetch_predictor.h
index 079f8b0bca4188f36610ab2da850db0bd8fa7f8f..5834521c4972b3ec5f0b50ee3afd0fd61b2fef77 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor.h
+++ b/chrome/browser/predictors/resource_prefetch_predictor.h
@@ -243,10 +243,28 @@ class ResourcePrefetchPredictor
   // |urls| with resources that need to be prefetched.
   bool GetPrefetchData(const GURL& main_frame_url, std::vector<GURL>* urls);
 
+  // Returns true if the URL-based table contains PrefetchData that can be

[pasko, 2016/10/07 11:42:43]

not fully true, since the |urls| could be non-empty and hence it would return
true even if the table contains no relevant data.

We should put a DCHECK(urls->empty()) in the beginning of the implementation of
every function like this.

[alexilin, 2016/10/07 13:09:29]

Yeah, excellent observation!

Nevertheless, I think it's strange and non-intuitive invariant for this function
so I'll just check if any element was added to |urls|.

+  // used for a |main_frame_url| and fills |urls| with resources that need to be
+  // prefetched.
+  bool GetURLPrefetchData(const std::string& main_frame_url,
+                          std::vector<GURL>* urls);
+
+  // Returns true if the host-based table contains PrefetchData that can be used

[pasko, 2016/10/07 11:42:43]

It is preferable to write 'iff' in this case (iff == if and only if)

[alexilin, 2016/10/07 13:09:29]

Done.

+  // for a |main_frame_host| and fills |urls| with resources that need to be
+  // prefetched.
+  bool GetHostPrefetchData(const std::string& main_frame_host,
+                           std::vector<GURL>* urls);
+
   // Converts a PrefetchData into a list of URLs.
   void PopulatePrefetcherRequest(const PrefetchData& data,
                                  std::vector<GURL>* urls);
 
+  // Returns true if |redirect_data_map| contains confident redirect endpoint
+  // for |first_redirect| and assign it to the |final_redirect|.
+  bool GetFinalRedirect(const std::string& first_redirect,

[pasko, 2016/10/07 11:42:43]

this seems not to require any state of the object. In this case it should be
either static or should move to anonymous namespace in the .cc file.

Now, if we want to test it, we have two options:
1. public bool GetFinalRedirectForTesting(...) // calls .cc:GetFinalRedirect(),
more preferable by me
2. private static bool GetFinalRedirect(...) // with FRIEND_TEST_ALL_PREFIXES ..
which we have many.

[alexilin, 2016/10/07 13:09:29]

Implemented the second one because "we have many".
Done.

+                        RedirectDataMap* redirect_data_map,
+                        std::string* final_redirect);
+
  public:
   // Starts prefetching if it is enabled and prefetching data exists for the
   // NavigationID either at the URL or at the host level.