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 225 matching lines @@
 
   // 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.
   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.
   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);
 
  private:
   // Starts initialization by posting a task to the DB thread to read the
@@ skipping 99 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_