New issue
Advanced search Search tips

Issue 859546 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

NetworkService: Figure out what to do about Net.HttpResponseInfo.ConnectionInfo.* histograms

Project Member Reported by mmenke@chromium.org, Jul 2

Issue description

content/browser/loader/resource_loader records Net.HttpResponseInfo.ConnectionInfo.MainFrame and Net.HttpResponseInfo.ConnectionInfo.SubResource, which are only recorded when the network service is enabeld.  We should figure out if we can just remove the histograms, or if we still need them.  If the latter is the case, we'll need to figure out how to hook them up when the NetworkService is enabled.

Whatever the decision here, I don't believe hooking up these histograms should block running experiments on Windows canary.
 
 
Owner: b...@chromium.org
Status: Started (was: Untriaged)
Talking to Matt in person, we decided that first these histograms will be removed to make network service experiments less painful.  After that, since these histograms provide valuable information, similar histograms will soon be added elsewhere to record (almost) the same information.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 3

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

commit 8f657894251848a0025a2ae8aa408861e5819faa
Author: Bence Béky <bnc@chromium.org>
Date: Tue Jul 03 11:59:15 2018

Deprecate ConnectionInfo histograms.

ResourceLoader is not used by network service, therefore
Net.HttpResponseInfo.ConnectionInfo.MainFrame and
Net.HttpResponseInfo.ConnectionInfo.SubResource histograms will not be
recorded after network service launches.  This CL obsoletes these two
histograms, and a follow-up CL will add new histograms in the network
service code path.

Bug: 859546
Change-Id: I37e190c1ef060642c08f95314ce52428542a5a2d
Reviewed-on: https://chromium-review.googlesource.com/1122712
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572174}
[modify] https://crrev.com/8f657894251848a0025a2ae8aa408861e5819faa/content/browser/loader/resource_loader.cc
[modify] https://crrev.com/8f657894251848a0025a2ae8aa408861e5819faa/tools/metrics/histograms/histograms.xml

Labels: Proj-Servicification Hotlist-KnownIssue
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 31

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

commit 93662b94dbc5abbc0dbd5e89f76edf2eae7eb9fa
Author: Bence Béky <bnc@chromium.org>
Date: Tue Jul 31 16:13:07 2018

Add ConnectionInfo histograms for network requests.

As part of the network servicification effort, histograms were removed
from ResourceLoader in https://crrev.com/c/1122712, and are now added to
ResourceDispatcher in this CL.

Bug: 859546
Change-Id: I1c073098cdc532d067ab69ec98daa0aec1f9c910
Reviewed-on: https://chromium-review.googlesource.com/1145129
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579423}
[modify] https://crrev.com/93662b94dbc5abbc0dbd5e89f76edf2eae7eb9fa/chrome/browser/net/network_request_metrics_browsertest.cc
[modify] https://crrev.com/93662b94dbc5abbc0dbd5e89f76edf2eae7eb9fa/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/93662b94dbc5abbc0dbd5e89f76edf2eae7eb9fa/net/http/http_response_info.h
[modify] https://crrev.com/93662b94dbc5abbc0dbd5e89f76edf2eae7eb9fa/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/93662b94dbc5abbc0dbd5e89f76edf2eae7eb9fa/tools/metrics/histograms/histograms.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 1

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

commit 56e969f5eb58b485ae95eeafbfffe60344d1fa5b
Author: Bence Béky <bnc@chromium.org>
Date: Wed Aug 01 13:04:16 2018

Relax favicon load related test expectations.

I introduced some test expectations in https://crrev.com/c/1145129 which
I believe might be unnecessarily strict.  In particular,
Net.ConnectionInfo.MainFrame and Net.ConnectionInfo.SubResource might be
recorded in OnReceivedResponse() for favicon but RecordLoadHistograms()
not called yet by the time test execution arrives to the expectations.
This CL relaxes some expectations to prevent these tests from becoming
flaky.

Bug: 859546
Change-Id: Idbacc7d024176406c5eb1a4be6d190e168a7d432
Reviewed-on: https://chromium-review.googlesource.com/1156848
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579784}
[modify] https://crrev.com/56e969f5eb58b485ae95eeafbfffe60344d1fa5b/chrome/browser/net/network_request_metrics_browsertest.cc

Sign in to add a comment