New issue
Advanced search Search tips

Issue 882482 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Network servicification of NQE in network metrics provider

Project Member Reported by tbansal@chromium.org, Sep 10

Issue description

Network quality is attached to the UMA and UKM metrics report during the time of upload. Currently, the network quality is queried using Network Quality Estimator (NQE). We need to transition it to using network quality tracker.
https://cs.chromium.org/chromium/src/components/metrics/net/network_metrics_provider.h
 
Labels: Proj-Servicification-Stable
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 24

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

commit a21361157100fef16d7417972a2bd68af7b5318c
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Sep 24 16:44:41 2018

Network servicification of NQE in network metrics provider

Use Network Quality Tracker (NQT) in network metrics provider
instead of the network quality estimator (NQE).

NQT works when network servicification is enabled or disabled.
NQE works only when network servicification is disabled.

Note that NQT is to be used on UI thread, while NQE was on
IO thread, which makes the code a bit simpler.

Change-Id: If95a8882d578ef6d00ea1a51e4bc5a861b3a47a2
Bug:  882482 
Reviewed-on: https://chromium-review.googlesource.com/1215371
Reviewed-by: Robert Kaplow (sloooow) <rkaplow@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593566}
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/chrome/browser/metrics/network_quality_estimator_provider_impl.cc
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/chrome/browser/metrics/network_quality_estimator_provider_impl.h
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/chrome/browser/metrics/ukm_browsertest.cc
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/components/metrics/BUILD.gn
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/a21361157100fef16d7417972a2bd68af7b5318c/components/metrics/net/network_metrics_provider_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 25

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

commit b52ecff78ef4422671a94b5527f4394e2daeea09
Author: agrieve <agrieve@chromium.org>
Date: Tue Sep 25 03:01:49 2018

Revert "Network servicification of NQE in network metrics provider"

This reverts commit a21361157100fef16d7417972a2bd68af7b5318c.

Reason for revert: Broke Cronet bot (see bug)

Original change's description:
> Network servicification of NQE in network metrics provider
> 
> Use Network Quality Tracker (NQT) in network metrics provider
> instead of the network quality estimator (NQE).
> 
> NQT works when network servicification is enabled or disabled.
> NQE works only when network servicification is disabled.
> 
> Note that NQT is to be used on UI thread, while NQE was on
> IO thread, which makes the code a bit simpler.
> 
> Change-Id: If95a8882d578ef6d00ea1a51e4bc5a861b3a47a2
> Bug:  882482 
> Reviewed-on: https://chromium-review.googlesource.com/1215371
> Reviewed-by: Robert Kaplow (sloooow) <rkaplow@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593566}

TBR=rkaplow@chromium.org,tbansal@chromium.org,robertogden@chromium.org,ryansturm@chromium.org

Change-Id: I339700eb7875d805413eddf3b5405243ad02a5c7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  882482 , 888612
Reviewed-on: https://chromium-review.googlesource.com/1242198
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593811}
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/chrome/browser/metrics/network_quality_estimator_provider_impl.cc
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/chrome/browser/metrics/network_quality_estimator_provider_impl.h
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/chrome/browser/metrics/ukm_browsertest.cc
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/components/metrics/BUILD.gn
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/b52ecff78ef4422671a94b5527f4394e2daeea09/components/metrics/net/network_metrics_provider_unittest.cc

Status: Assigned (was: Fixed)
Re-opening since the CL got reverted.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 22

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

commit 1fe79f73e53c182ac913473d2ddc9ad0ed8fae97
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Oct 22 19:18:01 2018

Network servicification of NQE in network metrics provider

Use Network Quality Tracker (NQT) in network metrics provider
instead of the network quality estimator (NQE).

NQT works when network servicification is enabled or disabled.
NQE works only when network servicification is disabled.

Note that NQT is to be used on UI thread, while NQE was on
IO thread, which makes the code a bit simpler.

Bug:  882482 
Change-Id: Ic5d9c3080ce74992efe1ddaa069fbbb04a255fe4
Reviewed-on: https://chromium-review.googlesource.com/c/1286969
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Robert Kaplow (sloooow) <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601675}
[modify] https://crrev.com/1fe79f73e53c182ac913473d2ddc9ad0ed8fae97/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/1fe79f73e53c182ac913473d2ddc9ad0ed8fae97/chrome/browser/metrics/network_quality_estimator_provider_impl.cc
[modify] https://crrev.com/1fe79f73e53c182ac913473d2ddc9ad0ed8fae97/chrome/browser/metrics/network_quality_estimator_provider_impl.h
[modify] https://crrev.com/1fe79f73e53c182ac913473d2ddc9ad0ed8fae97/chrome/browser/metrics/ukm_browsertest.cc
[modify] https://crrev.com/1fe79f73e53c182ac913473d2ddc9ad0ed8fae97/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/1fe79f73e53c182ac913473d2ddc9ad0ed8fae97/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/1fe79f73e53c182ac913473d2ddc9ad0ed8fae97/components/metrics/net/network_metrics_provider_unittest.cc

Labels: -M-71 M-72
Status: Fixed (was: Assigned)

Sign in to add a comment