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..be57fcc95ec60c33d65241b22912d2c980087677 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor.h
+++ b/chrome/browser/predictors/resource_prefetch_predictor.h
@@ -224,6 +224,12 @@ class ResourcePrefetchPredictor
   // Returns true if the request (should have a response in it) is "no-store".
   static bool IsNoStore(const net::URLRequest* request);
 
+  // Returns true if |redirect_data_map| contains confident redirect endpoint

[pasko, 2016/10/07 13:44:27]

nit: iff

[alexilin, 2016/10/07 14:29:05]

Done.

+  // for |first_redirect| and assign it to the |final_redirect|.

[pasko, 2016/10/07 13:44:27]

nit: s/assign/assigns/

[pasko, 2016/10/07 13:44:27]

nit: first_redirect is not a good name, because a redirect may refer to two URLs
and other data like request/response headers, and this is just a string.
Suggestion: |entry_point|.

[alexilin, 2016/10/07 14:29:05]

Done.

+  static bool GetFinalRedirect(const std::string& first_redirect,
+                               RedirectDataMap* redirect_data_map,

[pasko, 2016/10/07 13:44:27]

if the object (RedirectDataMap) is not modified by the function, it should be
passed as const reference (or by value).

This would be according to the style guide:

https://google.github.io/styleguide/cppguide.html#Function_Parameter_Ordering

https://google.github.io/styleguide/cppguide.html#Reference_Arguments

We do not always follow it (historical reasons?), but for new code it is
strongly recommended.

[alexilin, 2016/10/07 14:29:06]

Ok, thanks! I know this rule but I was confused by surrounding code and by the
fact that obtaining const reference from unique_ptr looks ugly.
(A few moments later) Ah, I forgot about unique_ptr::operator*. Not so ugly now.

+                               std::string* final_redirect);

[pasko, 2016/10/07 13:44:27]

nit: |redirect_endpoint| sounds more intuitive

(feel free to discard/object if it does not work for you this way)

[alexilin, 2016/10/07 14:29:05]

Done.

+
   // KeyedService methods override.
   void Shutdown() override;
 
@@ -239,12 +245,16 @@ class ResourcePrefetchPredictor
   // this point are the only ones considered for prefetching.
   void OnNavigationComplete(const NavigationID& nav_id_without_timing_info);
 
-  // Returns true if there is PrefetchData that can be used for a URL and fills
-  // |urls| with resources that need to be prefetched.
+  // Returns true iff there is PrefetchData that can be used for a
+  // |main_frame_url| and fills |urls| with resources that need to be
+  // prefetched.
   bool GetPrefetchData(const GURL& main_frame_url, std::vector<GURL>* urls);
 
-  // Converts a PrefetchData into a list of URLs.
-  void PopulatePrefetcherRequest(const PrefetchData& data,
+  // Returns true iff the |data_map| contains PrefetchData that can be used
+  // for a |main_frame_key| and fills |urls| with resources that need to be
+  // prefetched.
+  bool PopulatePrefetcherRequest(const std::string& main_frame_key,
+                                 PrefetchDataMap* data_map,

[pasko, 2016/10/07 13:44:27]

const reference here too

[alexilin, 2016/10/07 14:29:05]

Done.

                                  std::vector<GURL>* urls);
 
  public: