New issue
Advanced search Search tips

Issue 638308 link

Starred by 0 users
Status: Fixed
Owner:
tbansal@chromium.org
Closed: Jul 2017
Cc:
bengr@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 705691


Sign in to add a comment

NQE should allow Cronet embedders to specify the variation params

Project Member Reported by tbansal@chromium.org, Aug 16 2016 
Currently, different configuration parameters of Network Quality Estimator (NQE) can be overridden using variation params which are provided to NQE in its constructor. Examples of configuration parameters include: RTT and bandwidth thresholds for different Effective Connection Type (ECT) values.

We should expose a way for Cronet embedders to specify their own values of variation params, so they can control NQE's behavior (e.g., use different thresholds for ECTs).
Project Member

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

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

commit 7a718ef951d41df73fdf7e00725c919498bf6394
Author: tbansal <tbansal@chromium.org>
Date: Mon Oct 31 21:33:46 2016

NQE: Separate out params to a different file

There is no functionality change. This CL only moves the
params and the related functions to a separate file.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.android:android_cronet

BUG= 638308 

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

[modify] https://crrev.com/7a718ef951d41df73fdf7e00725c919498bf6394/net/net.gypi
[modify] https://crrev.com/7a718ef951d41df73fdf7e00725c919498bf6394/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/7a718ef951d41df73fdf7e00725c919498bf6394/net/nqe/network_quality_estimator.h
[add] https://crrev.com/7a718ef951d41df73fdf7e00725c919498bf6394/net/nqe/network_quality_estimator_params.cc
[add] https://crrev.com/7a718ef951d41df73fdf7e00725c919498bf6394/net/nqe/network_quality_estimator_params.h

Comment 2 by tbansal@chromium.org, Feb 13 2017

Components: Internals>Network>NetworkQuality

Comment 3 by tbansal@chromium.org, Feb 14 2017

Labels: -nqe

Comment 4 by tbansal@chromium.org, Mar 23 2017

Components: -Internals>Network

Comment 5 by bengr@chromium.org, Apr 30 2017 

status?

Comment 6 by bengr@chromium.org, Apr 30 2017

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, May 4 2017 

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

commit 35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281
Author: tbansal <tbansal@chromium.org>
Date: Thu May 04 16:23:33 2017

NQE: Move params from the estimator class to the params class

Move the network quality estimator (NQE) config params from
n_q_e.cc to n_q_e_params class. This is a refactor change with no
functional change. Long term, this would make it easier for net
embedders (Chrome and Cronet embedders) to configure NQE.

BUG= 638308 
TBR=bengr@chromium.org

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

[modify] https://crrev.com/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281/net/BUILD.gn
[modify] https://crrev.com/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281/net/nqe/network_quality_estimator_params.cc
[modify] https://crrev.com/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281/net/nqe/network_quality_estimator_params.h
[add] https://crrev.com/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281/net/nqe/network_quality_estimator_params_unittest.cc
[modify] https://crrev.com/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281/net/nqe/network_quality_estimator_unittest.cc

Comment 8 by tbansal@chromium.org, May 8 2017

Blockedon: 705691

Comment 10 by tbansal@chromium.org, May 24 2017

Labels: M-61

Comment 11 by bengr@chromium.org, May 26 2017

Labels: -Pri-3 Pri-2
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 8 2017 

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

commit 4a4305a5f603f83ce7eafad9866565af013815c5
Author: tbansal <tbansal@chromium.org>
Date: Thu Jun 08 05:03:19 2017

Pass parsed network quality estimator params when constructing NQE

This CL changes the way how Network Quality Estimator (NQE) is
constructed. Currently, NQE configuration parameters are passed as
key-value pair by Chrome/Cronet in the NQE's constructor.

This CL requires Chrome/Cronet to parse the config params themselves,
store them in a class, and pass an instance of that class in the
constructor of NQE.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG= 638308 

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

[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/chrome/browser/android/net/external_estimate_provider_android_unittest.cc
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/chrome/browser/io_thread.cc
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/components/cronet/android/cronet_url_request_context_adapter.cc
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/network_quality_estimator_params.cc
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/network_quality_estimator_params.h
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/network_quality_estimator_test_util.cc
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/network_quality_estimator_unittest.cc
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/throughput_analyzer.h
[modify] https://crrev.com/4a4305a5f603f83ce7eafad9866565af013815c5/net/nqe/throughput_analyzer_unittest.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 4 2017 

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

commit e11aa36c4a842d64337c224824963527d5338847
Author: tbansal <tbansal@chromium.org>
Date: Tue Jul 04 15:59:36 2017

Add functionality for embedders to configure NQE

Embedders can specify Network Quality Estimator's (NQE)
configuration params via experimental options JSON string.
These params are passed to NQE's constructor. This way
the embedder can configure NQE similar to how Chrome
currently does it using field trial params.

The CL also makes |experimental_options| member in the
UrlRequestContextContextConfig as a private member since
|effective_experimental_options| captures similar information,
and it is confusing to have 2 public members which capture similar
information.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.android:android_cronet

BUG= 638308 

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

[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/components/cronet/android/cronet_url_request_context_adapter.cc
[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/components/cronet/android/cronet_url_request_context_adapter.h
[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/components/cronet/android/test/cronet_url_request_context_config_test.cc
[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/components/cronet/android/test/javatests/src/org/chromium/net/NQETest.java
[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/components/cronet/url_request_context_config.cc
[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/components/cronet/url_request_context_config.h
[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/net/nqe/network_quality_estimator_params.cc
[modify] https://crrev.com/e11aa36c4a842d64337c224824963527d5338847/net/nqe/network_quality_estimator_params.h

Comment 14 by tbansal@chromium.org, Jul 27 2017

Status: Fixed (was: Started)
►

Sign in to add a comment