Send an event to the page load metrics to track resource starting.

chrome/browser/page_load_metrics/page_load_metrics_observer.h

Index: chrome/browser/page_load_metrics/page_load_metrics_observer.h
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_observer.h b/chrome/browser/page_load_metrics/page_load_metrics_observer.h
index 11a98896bd91bf72d28f0f68b05251f2ef5229f3..6607825064812c58a3ce2699c6df90e16a54b0be 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_observer.h
+++ b/chrome/browser/page_load_metrics/page_load_metrics_observer.h
@@ -195,16 +195,18 @@ struct PageLoadExtraInfo {
   const PageLoadMetadata child_frame_metadata;
 };
 
-// Container for various information about a request within a page load.
-struct ExtraRequestInfo {
-  ExtraRequestInfo(bool was_cached,
-                   int64_t raw_body_bytes,
-                   bool data_reduction_proxy_used,
-                   int64_t original_network_content_length);
+// Container for various information about a completed request within a page
+// load.
+struct ExtraRequestCompleteInfo {
+  ExtraRequestCompleteInfo(bool was_cached,
+                           int64_t raw_body_bytes,
+                           bool data_reduction_proxy_used,
+                           int64_t original_network_content_length,
+                           content::ResourceType found_type);

[Bryan McQuade, 2017/04/17 23:32:14]

what does 'found' mean in found_type? Any reason not to call this resource_type?

[Pete Williamson, 2017/04/18 18:17:27]

the intent is that the resource type can be determined in two ways, either from
the filename (which is what we do for ExtraRequestStartInfo), or from the
headers, which are considered more reliable, but could in theory be different
from the filename.  By 'found', I meant what we discovered in the headers.

Changed to detected_resource_type

 
-  ExtraRequestInfo(const ExtraRequestInfo& other);
+  ExtraRequestCompleteInfo(const ExtraRequestCompleteInfo& other);
 
-  ~ExtraRequestInfo();
+  ~ExtraRequestCompleteInfo();
 
   // True if the resource was loaded from cache.
   const bool was_cached;
@@ -218,6 +220,21 @@ struct ExtraRequestInfo {
   // The number of body (not header) bytes that the data reduction proxy saw
   // before it compressed the requests.
   const int64_t original_network_content_length;
+
+  // The type of the request as gleaned from the ResourceRequestInfo.
+  const content::ResourceType resource_type;
+};
+
+// Container for various information about a started request within a page load.
+struct ExtraRequestStartInfo {
+  explicit ExtraRequestStartInfo(content::ResourceType type);
+
+  ExtraRequestStartInfo(const ExtraRequestStartInfo& other);
+
+  ~ExtraRequestStartInfo();
+
+  // The type of the request as gleaned from the ResourceRequestInfo.

[Bryan McQuade, 2017/04/17 23:32:14]

if we're already reporting this here, it feels redundant to also report it in
the complete struct, unless the value in the complete struct has been updated in
some way to take additional info into account.

[RyanSturm, 2017/04/17 23:37:32]

I think it would either need to be GRID or type to determine a completion % by
type, which is the end goal I believe.

[Pete Williamson, 2017/04/18 18:17:27]

I'm trying to allow for the possibility that we discovered the resource type
changed while parsing the headers.  According to Ryan, that does happen
sometimes.

Also, for each resource type, we need to know how many starts it has so far to
compare against how many completions.  The signal is that, for instance, at a
given time, 5 of 7 images have completed, we need this to count the "7" part. 
We include it again in the completion event to count the '5' part, and to make
corrections if the headers have a different type than the file extension.

[RyanSturm, 2017/04/18 18:37:13]

AFAIK, there are basically four ways to determine resource type:
content::ResourceType, which roughly maps to the DOM element or action that
caused the resource to be downloaded (<img> tag for instance); extension type if
present can be useful (img.jpeg); MIME-type is a header that is the server
specifying what the type is and can generally be pretty useful if present and
accurate (image/webp); finally, how the resource is used in blink can give
something close to the ground-truth (this is pretty fuzzy as it can be used more
than once, and it is determined later).

I'm not sure what is best and haven't done an accuracy estimation for it in the
wild. I would say using content::ResourceType is sufficient in most places, and
the complete/started types will match up. XHRs are really the largest area for
the type to be unknown (they are treated as their own type), and a lot of times
MIME-type determination can be a good fallback for XHR types if the headers have
come in.

If you are training a model anyway, I'd just treat XHRs as XHRs and use
content::ResourceType as you are doing here. In terms of tracking pending vs
complete, you could keep a map of GlobalRequestID to resource type for pending
requests, but I think just keep counts of resource types is simpler as it seems
you are doing.

+  const content::ResourceType resource_type;
 };
 
 // Interface for PageLoadMetrics observers. All instances of this class are
@@ -375,8 +392,13 @@ class PageLoadMetricsObserver {
       const FailedProvisionalLoadInfo& failed_provisional_load_info,
       const PageLoadExtraInfo& extra_info) {}
 
+  // Called whenever a request load begins.
+  virtual void OnStartedResource(
+      const ExtraRequestStartInfo& extra_request_start_info) {}
+
   // Called whenever a request is loaded for this page load.
-  virtual void OnLoadedResource(const ExtraRequestInfo& extra_request_info) {}
+  virtual void OnLoadedResource(
+      const ExtraRequestCompleteInfo& extra_request_complete_info) {}
 };
 
 }  // namespace page_load_metrics