New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 680983 link

Starred by 1 user
Status: Started
Owner:
█ bmcquade@chromium.org
Last visit > 30 days ago
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

page load metrics: data attribution sometimes incorrect for main frame resources

Project Member Reported by bmcquade@chromium.org, Jan 13 2017 
page_load_metrics recently added support for tracking bytes used per page load:
https://codereview.chromium.org/2560043004

The change assumes that the callback to inform page_load_metrics that a request has completed, which is invoked via ChromeResourceDispatcherHostDelegate::RequestComplete, can only be invoked after a navigation has committed.

This assumption appears to be valid for non main frame requests. However, main frame requests can complete either before or after commit.

Thus, we can no longer assume that all RequestComplete invocations should be routed to the currently committed load.

Instead, it is necessary to determine which of the currently tracked provisional loads or committed load is associated with a completed main frame request.

See https://docs.google.com/document/d/1dZP0si0IKEMeVKGsohJ3fFWiKqh3nZBtuTFVkAPSXYM/edit for more details.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 17 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/67906e1cd7937494e078325ead8bdb95093bbf29

commit 67906e1cd7937494e078325ead8bdb95093bbf29
Author: bmcquade <bmcquade@chromium.org>
Date: Tue Jan 17 22:06:06 2017

Associate a main resource request with its PageLoadTracker.

A main resource request may complete either before or after commit.
For example, if the main resource response fits in a single TCP window,
it will likely complete before commit, whereas if the response is large,
it will likely complete after commit.

This change special cases main resource PageLoadTracker attribution, by
finding the provisional load or committed load with a GlobalRequestID that
matches the given URLRequest.

See
https://docs.google.com/document/d/1dZP0si0IKEMeVKGsohJ3fFWiKqh3nZBtuTFVkAPSXYM/edit
for more details.

BUG=680983

Review-Url: https://codereview.chromium.org/2624283004
Cr-Commit-Position: refs/heads/master@{#444159}

[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/page_load_metrics/metrics_navigation_throttle.cc
[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/page_load_metrics/metrics_navigation_throttle.h
[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc
[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/page_load_metrics/metrics_web_contents_observer.h
[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/page_load_metrics/page_load_tracker.cc
[modify] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/browser/page_load_metrics/page_load_tracker.h
[add] https://crrev.com/67906e1cd7937494e078325ead8bdb95093bbf29/chrome/test/data/page_load_metrics/large.html

Comment 2 by bmcquade@chromium.org, Jan 17 2017 

This should be fixed by the recent patch. I'll watch histograms over the coming days and close this out if things look ok.
►

Sign in to add a comment