predictors: Move more methods from ResourcePrefetchPredictor into LoadingDataCollector.

chrome/browser/predictors/loading_data_collector.h

Index: chrome/browser/predictors/loading_data_collector.h
diff --git a/chrome/browser/predictors/loading_data_collector.h b/chrome/browser/predictors/loading_data_collector.h
index 65274eef91284820edce6fbc3a8a1dbd26af836e..fb60642032b57b1e371421ff306c51db69d796df 100644
--- a/chrome/browser/predictors/loading_data_collector.h
+++ b/chrome/browser/predictors/loading_data_collector.h
@@ -23,7 +23,9 @@ class LoadingDataCollector
     : public base::SupportsWeakPtr<LoadingDataCollector> {
  public:
   explicit LoadingDataCollector(
-      predictors::ResourcePrefetchPredictor* predictor);
+      predictors::ResourcePrefetchPredictor* predictor,
+      predictors::LoadingStatsCollector* stats_collector,
+      const LoadingPredictorConfig& config);
   ~LoadingDataCollector();
 
   // Thread safe.
@@ -33,7 +35,7 @@ class LoadingDataCollector
   static bool ShouldRecordRedirect(net::URLRequest* response);
 
   // 'LoadingPredictorObserver' and 'ResourcePrefetchPredictorTabHelper' call
-  // the below functions to inform the predictor of main frame and resource
+  // the below functions to inform the collector of main frame and resource
   // requests. Should only be called if the corresponding Should* functions
   // return true.
   void RecordURLRequest(
@@ -47,7 +49,7 @@ class LoadingDataCollector
   void RecordMainFrameLoadComplete(const NavigationID& navigation_id);
 
   // Called after the main frame's first contentful paint.
-  void RecordFirstContentfulPaint(
+  virtual void RecordFirstContentfulPaint(

[alexilin, 2017/06/14 11:41:54]

Why virtual?

[trevordixon, 2017/06/16 05:11:28]

So a MockLoadingDataCollector can override it. Does it need to be virtual in
that case?

[alexilin, 2017/06/16 09:31:57]

Yes, it has to be virtual to be overriden by mock class. You just haven't added
unittests to this CL yet so I decided to clarify.

       const NavigationID& navigation_id,
       const base::TimeTicks& first_contentful_paint);
 
@@ -69,7 +71,36 @@ class LoadingDataCollector
 
   static void SetAllowPortInUrlsForTesting(bool state);
 
+  // Functions called on different network events pertaining to the loading of
+  // main frame resource or sub resources.
+  void OnMainFrameRequest(
+      const predictors::ResourcePrefetchPredictor::URLRequestSummary& request);
+  void OnMainFrameRedirect(
+      const predictors::ResourcePrefetchPredictor::URLRequestSummary& response);
+  void OnSubresourceResponse(
+      const predictors::ResourcePrefetchPredictor::URLRequestSummary& response);
+  void OnSubresourceRedirect(
+      const predictors::ResourcePrefetchPredictor::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.

[alexilin, 2017/06/14 11:41:54]

s/for prefetching//

[trevordixon, 2017/06/16 05:11:28]

Done.

+  void OnNavigationComplete(const NavigationID& nav_id_without_timing_info);
+
+  // Cleanup inflight_navigations_ and call a cleanup for stats_collector_.
+  void CleanupAbandonedNavigations(const NavigationID& navigation_id);
+
+  typedef std::map<

[alexilin, 2017/06/14 11:41:54]

This formatting looks strange. Did you run `git cl format`?

[trevordixon, 2017/06/16 05:11:28]

Yes, this is what it did!

[alexilin, 2017/06/16 09:31:57]

Ok, it's much better after stripping namespaces!

+      NavigationID,
+      std::unique_ptr<
+          predictors::ResourcePrefetchPredictor::PageRequestSummary>>
+      NavigationMap;
+
   predictors::ResourcePrefetchPredictor* const predictor_;
+  predictors::LoadingStatsCollector* const stats_collector_;
+  const LoadingPredictorConfig config_;
+
+  NavigationMap inflight_navigations_;
 };
 
 }  // namespace predictors