predictors: Move more methods from ResourcePrefetchPredictor into LoadingDataCollector.

chrome/browser/predictors/loading_data_collector.h

 // Copyright 2017 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_LOADING_DATA_COLLECTOR_H_
 #define CHROME_BROWSER_PREDICTORS_LOADING_DATA_COLLECTOR_H_
 
 #include <string>
 
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 #include "content/public/common/resource_type.h"
 
 namespace net {
 class URLRequest;
 }
 
 namespace predictors {
 
 // Records navigation events as reported by various observers to the database
 // and stats collection classes. All the non-static methods of this class need
 // to be called on the UI thread.
 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.
   static bool ShouldRecordRequest(net::URLRequest* request,
                                   content::ResourceType resource_type);
   static bool ShouldRecordResponse(net::URLRequest* response);
   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(
       const predictors::ResourcePrefetchPredictor::URLRequestSummary& request);
   void RecordURLResponse(
       const predictors::ResourcePrefetchPredictor::URLRequestSummary& response);
   void RecordURLRedirect(
       const predictors::ResourcePrefetchPredictor::URLRequestSummary& response);
 
   // Called when the main frame of a page completes loading.
   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);
 
  private:
   friend class ResourcePrefetchPredictorBrowserTest;
 
   FRIEND_TEST_ALL_PREFIXES(LoadingDataCollectorTest, HandledResourceTypes);
 
   // Returns true if the main page request is supported for prediction.
   static bool IsHandledMainPage(net::URLRequest* request);
 
   // Returns true if the subresource request is supported for prediction.
   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);
 
   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
 
 #endif  // CHROME_BROWSER_PREDICTORS_LOADING_DATA_COLLECTOR_H_