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..b7aeb341ac96a83bb80b154c8719acf61a0165c3 100644
--- a/chrome/browser/predictors/loading_data_collector.h
+++ b/chrome/browser/predictors/loading_data_collector.h
@@ -5,10 +5,17 @@
 #ifndef CHROME_BROWSER_PREDICTORS_LOADING_DATA_COLLECTOR_H_
 #define CHROME_BROWSER_PREDICTORS_LOADING_DATA_COLLECTOR_H_
 
+#include <map>
+#include <memory>
 #include <string>
+#include <vector>
 
-#include "chrome/browser/predictors/resource_prefetch_predictor.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/predictors/loading_predictor_config.h"
+#include "chrome/browser/predictors/resource_prefetch_common.h"
 #include "content/public/common/resource_type.h"
+#include "net/base/request_priority.h"
+#include "url/gurl.h"
 
 namespace net {
 class URLRequest;
@@ -16,6 +23,71 @@ class URLRequest;
 
 namespace predictors {
 
+class LoadingStatsCollector;
+class ResourcePrefetchPredictor;
+
+// Data collected for origin-based prediction, for a single origin during a
+// page load (see PageRequestSummary).
+struct OriginRequestSummary {
+  OriginRequestSummary();
+  OriginRequestSummary(const OriginRequestSummary& other);
+  ~OriginRequestSummary();
+
+  GURL origin;
+  bool always_access_network;
+  bool accessed_network;
+  int first_occurrence;
+};
+
+// Stores the data that we need to get from the URLRequest.
+struct URLRequestSummary {
+  URLRequestSummary();
+  URLRequestSummary(const URLRequestSummary& other);
+  ~URLRequestSummary();
+
+  NavigationID navigation_id;
+  GURL resource_url;
+  GURL request_url;  // URL after all redirects.
+  content::ResourceType resource_type;
+  net::RequestPriority priority;
+  base::TimeTicks response_time;
+  bool before_first_contentful_paint;
+
+  // Only for responses.
+  std::string mime_type;
+  bool was_cached;
+  GURL redirect_url;  // Empty unless request was redirected to a valid url.
+
+  bool has_validators;
+  bool always_revalidate;
+  bool is_no_store;
+  bool network_accessed;
+
+  // Initializes a |URLRequestSummary| from a |URLRequest| response.
+  // Returns true for success. Note: NavigationID is NOT initialized
+  // by this function.
+  static bool SummarizeResponse(const net::URLRequest& request,
+                                URLRequestSummary* summary);
+};
+
+// Stores the data learned from a single navigation.
+struct PageRequestSummary {
+  explicit PageRequestSummary(const GURL& main_frame_url);
+  PageRequestSummary(const PageRequestSummary& other);
+  ~PageRequestSummary();
+
+  GURL main_frame_url;
+  GURL initial_url;
+  base::TimeTicks first_contentful_paint;
+
+  // Stores all subresource requests within a single navigation, from initial
+  // main frame request to navigation completion.
+  std::vector<URLRequestSummary> subresource_requests;
+  // Map of origin -> OriginRequestSummary. Only one instance of each origin
+  // is kept per navigation, but the summary is updated several times.
+  std::map<GURL, OriginRequestSummary> origins;
+};
+
 // 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.
@@ -23,9 +95,23 @@ class LoadingDataCollector
     : public base::SupportsWeakPtr<LoadingDataCollector> {
  public:
   explicit LoadingDataCollector(
-      predictors::ResourcePrefetchPredictor* predictor);
+      predictors::ResourcePrefetchPredictor* predictor,
+      predictors::LoadingStatsCollector* stats_collector,
+      const LoadingPredictorConfig& config);
   ~LoadingDataCollector();
 
+  // Determines the resource type from the declared one, falling back to MIME
+  // type detection when it is not explicit.
+  static content::ResourceType GetResourceType(
+      content::ResourceType resource_type,
+      const std::string& mime_type);
+
+  // Determines the ResourceType from the mime type, defaulting to the
+  // |fallback| if the ResourceType could not be determined.
+  static content::ResourceType GetResourceTypeFromMimeType(
+      const std::string& mime_type,
+      content::ResourceType fallback);
+
   // Thread safe.
   static bool ShouldRecordRequest(net::URLRequest* request,
                                   content::ResourceType resource_type);
@@ -33,21 +119,20 @@ 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(
-      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 RecordURLRequest(const URLRequestSummary& request);
+  void RecordURLResponse(const URLRequestSummary& response);
+  void RecordURLRedirect(const URLRequestSummary& response);
+
+  // Called when the main frame of a page completes loading. 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.
   void RecordMainFrameLoadComplete(const NavigationID& navigation_id);
 
   // Called after the main frame's first contentful paint.
-  void RecordFirstContentfulPaint(
+  virtual void RecordFirstContentfulPaint(
       const NavigationID& navigation_id,
       const base::TimeTicks& first_contentful_paint);
 
@@ -69,7 +154,24 @@ class LoadingDataCollector
 
   static void SetAllowPortInUrlsForTesting(bool state);
 
-  predictors::ResourcePrefetchPredictor* const predictor_;
+  // Functions called on different network events pertaining to the loading of
+  // main frame resource or sub resources.
+  void OnMainFrameRequest(const URLRequestSummary& request);
+  void OnMainFrameRedirect(const URLRequestSummary& response);
+  void OnSubresourceResponse(const URLRequestSummary& response);
+  void OnSubresourceRedirect(const URLRequestSummary& response);
+
+  // Cleanup inflight_navigations_ and call a cleanup for stats_collector_.
+  void CleanupAbandonedNavigations(const NavigationID& navigation_id);
+
+  typedef std::map<NavigationID, std::unique_ptr<PageRequestSummary>>

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

nit:
using NavigationMap = std::map<NavigationID,
std::unique_ptr<PageRequestSummary>> - I'd prefer that we use C++11 style type
aliasing;
and put it in the beginning of the private session according to the style guide:
https://google.github.io/styleguide/cppguide.html#Declaration_Order

[trevordixon, 2017/06/16 10:53:19]

Done.

+      NavigationMap;
+
+  ResourcePrefetchPredictor* const predictor_;
+  LoadingStatsCollector* const stats_collector_;
+  const LoadingPredictorConfig config_;
+
+  NavigationMap inflight_navigations_;
 };
 
 }  // namespace predictors