Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(183)

Issue 2113363002: NQE: Add Transport RTT based GetECT algorithm (Closed)

Created:
4 years, 5 months ago by tbansal1
Modified:
4 years, 5 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

NQE: Add Transport RTT based GetECT algorithm In the next CL, GetECT API which computes Effective Connection Type (ECT) based on transport RTT and downlink throughput will be exposed to Cronet. BUG=625305 Committed: https://crrev.com/b7046a46eea6d3938ad2fc136d650003260cd47f Cr-Commit-Position: refs/heads/master@{#404497}

Patch Set 1 #

Total comments: 21

Patch Set 2 : Addressed kundaji comments #

Patch Set 3 : kundaji comments #

Patch Set 4 : Add TODO #

Total comments: 2

Patch Set 5 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+287 lines, -33 lines) Patch
M net/nqe/network_quality.h View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M net/nqe/network_quality_estimator.h View 1 2 3 4 6 chunks +32 lines, -6 lines 0 comments Download
M net/nqe/network_quality_estimator.cc View 1 2 3 4 7 chunks +106 lines, -25 lines 0 comments Download
M net/nqe/network_quality_estimator_unittest.cc View 1 2 4 chunks +139 lines, -2 lines 0 comments Download

Messages

Total messages: 31 (16 generated)
tbansal1
kundaji: ptal. Thanks.
4 years, 5 months ago (2016-07-01 22:51:00 UTC) #6
Not at Google. Contact bengr
https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", This is brittle because it allows adding an ...
4 years, 5 months ago (2016-07-06 19:04:07 UTC) #7
tbansal1
kundaji: ptal. Thanks. https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", On 2016/07/06 19:04:07, kundaji wrote: ...
4 years, 5 months ago (2016-07-06 23:17:33 UTC) #8
Not at Google. Contact bengr
lgtm https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", On 2016/07/06 23:17:32, tbansal1 wrote: > On ...
4 years, 5 months ago (2016-07-07 00:03:04 UTC) #9
tbansal1
bengr: ptal. Thanks. https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", On 2016/07/07 00:03:04, kundaji wrote: ...
4 years, 5 months ago (2016-07-07 16:47:32 UTC) #15
Not at Google. Contact bengr
https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", On 2016/07/07 16:47:32, tbansal1 wrote: > On 2016/07/07 ...
4 years, 5 months ago (2016-07-07 17:19:17 UTC) #16
tbansal1
https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", On 2016/07/07 17:19:17, kundaji wrote: > On 2016/07/07 ...
4 years, 5 months ago (2016-07-07 18:01:32 UTC) #17
Not at Google. Contact bengr
https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", On 2016/07/07 18:01:32, tbansal1 wrote: > On 2016/07/07 ...
4 years, 5 months ago (2016-07-07 19:10:49 UTC) #18
tbansal1
bengr: ptal. Thanks. https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2113363002/diff/20001/net/nqe/network_quality_estimator.cc#newcode238 net/nqe/network_quality_estimator.cc:238: {"TransportRTTOrDownstreamThroughput", On 2016/07/07 19:10:49, kundaji wrote: ...
4 years, 5 months ago (2016-07-07 22:47:26 UTC) #19
bengr
lgtm. Please update the description to spell out "effective connection type" so that readers know ...
4 years, 5 months ago (2016-07-08 17:10:50 UTC) #20
tbansal1
https://codereview.chromium.org/2113363002/diff/160001/net/nqe/network_quality_estimator_unittest.cc File net/nqe/network_quality_estimator_unittest.cc (right): https://codereview.chromium.org/2113363002/diff/160001/net/nqe/network_quality_estimator_unittest.cc#newcode662 net/nqe/network_quality_estimator_unittest.cc:662: for (Algorithms::const_iterator it_second = On 2016/07/08 17:10:50, bengr wrote: ...
4 years, 5 months ago (2016-07-08 18:11:16 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2113363002/180001
4 years, 5 months ago (2016-07-08 21:32:19 UTC) #26
commit-bot: I haz the power
Committed patchset #5 (id:180001)
4 years, 5 months ago (2016-07-08 21:38:04 UTC) #28
commit-bot: I haz the power
CQ bit was unchecked.
4 years, 5 months ago (2016-07-08 21:38:23 UTC) #29
commit-bot: I haz the power
4 years, 5 months ago (2016-07-08 21:40:29 UTC) #31
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/b7046a46eea6d3938ad2fc136d650003260cd47f
Cr-Commit-Position: refs/heads/master@{#404497}

Powered by Google App Engine
This is Rietveld 408576698