predictors: Use redirect data in prefetch.

chrome/browser/predictors/resource_prefetch_predictor.h

 // Copyright 2014 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.
 
 #ifndef CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_H_
 #define CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_H_
 
 #include <stddef.h>
 
 #include <map>
@@ skipping 206 matching lines @@
   static bool IsHandledSubresource(net::URLRequest* request,
                                    content::ResourceType resource_type);
 
   // Returns true if the subresource has a supported type.
   static bool IsHandledResourceType(content::ResourceType resource_type,
                                     const std::string& mime_type);
 
   // 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;
 
   // Functions called on different network events pertaining to the loading of
   // main frame resource or sub resources.
   void OnMainFrameRequest(const URLRequestSummary& request);
   void OnMainFrameResponse(const URLRequestSummary& response);
   void OnMainFrameRedirect(const URLRequestSummary& response);
   void OnSubresourceResponse(const URLRequestSummary& response);
 
   // Called when onload completes for a navigation. We treat this point as the
   // "completion" of the navigation. The resources requested by the page up to
   // 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:
   // Starts prefetching if it is enabled and prefetching data exists for the
   // NavigationID either at the URL or at the host level.
   void StartPrefetching(const GURL& main_frame_url);
 
   // Stops prefetching that may be in progress corresponding to |navigation_id|.
   void StopPrefetching(const GURL& main_frame_url);
 
@@ skipping 101 matching lines @@
 
   ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
       history_service_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourcePrefetchPredictor);
 };
 
 }  // namespace predictors
 
 #endif  // CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_H_