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

Issue 651448 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Avoid logging abort metrics for internal aborts such as 204s and downloads

Project Member Reported by bmcquade@chromium.org, Sep 29 2016

Issue description

The current page load metrics implementation logs aborts for internally generated aborts such as 204s and downloads.

We should ignore any aborts that did not commit that have HTTP response headers, since these are cases where Chrome determined that the response should not be committed due to being e.g. a 204 or a download, rather than a real user-initiated abort.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 5 2016

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

commit 7226898b5ec5e5d5eb11ef58606e5ca84c435919
Author: bmcquade <bmcquade@chromium.org>
Date: Wed Oct 05 12:05:51 2016

Prevent tracking metrics for cases such as 204s and downloads.

More generally, ignore tracking metrics for navigations that result
in receiving a successful HTTP response but that don't commit.

The PageLoad UMA implementation currently tracks 204s and downloads as
aborts, as their net error status reported on NavigationHandle is
ERR_ABORTED. In reality these are just loads that terminated without
committing, so they should instead be ignored.

Note that the new tests fail when using browser-side navigation, and
are currently disabled there. See  crbug.com/652767  for details.

BUG= 651448 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/chrome/browser/page_load_metrics/page_load_metrics_observer.h
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/content/public/browser/navigation_handle.h
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter

Status: Fixed (was: Started)
This is fixed, though  bug 652767  means that behavior is incorrect when browser side navigation is enabled.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7226898b5ec5e5d5eb11ef58606e5ca84c435919

commit 7226898b5ec5e5d5eb11ef58606e5ca84c435919
Author: bmcquade <bmcquade@chromium.org>
Date: Wed Oct 05 12:05:51 2016

Prevent tracking metrics for cases such as 204s and downloads.

More generally, ignore tracking metrics for navigations that result
in receiving a successful HTTP response but that don't commit.

The PageLoad UMA implementation currently tracks 204s and downloads as
aborts, as their net error status reported on NavigationHandle is
ERR_ABORTED. In reality these are just loads that terminated without
committing, so they should instead be ignored.

Note that the new tests fail when using browser-side navigation, and
are currently disabled there. See  crbug.com/652767  for details.

BUG= 651448 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/chrome/browser/page_load_metrics/page_load_metrics_observer.h
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/content/public/browser/navigation_handle.h
[modify] https://crrev.com/7226898b5ec5e5d5eb11ef58606e5ca84c435919/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter

Comment 4 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment