Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(61)

Issue 1306423004: [NetInfo] Browser changes to support connection.downlinkMax (Closed)

Created:
3 years, 9 months ago by jkarlin
Modified:
3 years, 9 months ago
CC:
adamk, cbentzel+watch_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, igrigorik, jam, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[NetInfo] Browser changes to support connection.downlinkMax Patch 1) https://crrev.com/1308943005 Patch 2) https://crrev.com/1306423004 (this) Patch 3) https://crrev.com/1309903006 Also depends on https://codereview.chromium.org/1306653003/ BUG=412741 Committed: https://crrev.com/300a1d28b57bc0d49603c97828f1833f577bd46a Cr-Commit-Position: refs/heads/master@{#349739}

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : clean up #

Total comments: 8

Patch Set 4 : Rebase #

Patch Set 5 : Rebase #

Patch Set 6 : Fixes #

Patch Set 7 : Fixes #

Total comments: 4

Patch Set 8 : Rebase #

Patch Set 9 : No millibits for you! #

Total comments: 4

Patch Set 10 : Rebase #

Patch Set 11 : Rebase #

Patch Set 12 : Address comments from PS9 #

Patch Set 13 : Minor browser test fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+164 lines, -111 lines) Patch
M chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +3 lines, -1 line 0 comments Download
M content/browser/net/browser_online_state_observer.h View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -3 lines 0 comments Download
M content/browser/net/browser_online_state_observer.cc View 1 2 3 4 5 6 7 8 9 1 chunk +6 lines, -4 lines 0 comments Download
M content/browser/net_info_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +38 lines, -7 lines 0 comments Download
M content/browser/renderer_host/render_view_host_impl.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -3 lines 0 comments Download
M content/common/view_messages.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -2 lines 0 comments Download
M content/public/common/common_param_traits_macros.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M content/public/common/web_preferences.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M content/public/common/web_preferences.cc View 1 2 3 4 5 1 chunk +4 lines, -1 line 0 comments Download
M content/renderer/render_thread_impl.h View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M content/renderer/render_thread_impl.cc View 1 2 3 2 chunks +7 lines, -5 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M content/test/data/net_info.html View 1 chunk +4 lines, -0 lines 0 comments Download
M net/base/network_change_notifier.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +6 lines, -5 lines 0 comments Download
M net/base/network_change_notifier.cc View 1 2 3 4 5 6 7 8 9 2 chunks +75 lines, -75 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 44 (19 generated)
jkarlin
Paul: PTAL as this CL is largely about using the NetworkChangeNotifier.
3 years, 9 months ago (2015-09-08 15:11:50 UTC) #9
jkarlin
(It helps to add the reviewer before sending the message). Paul, PTAL as this is ...
3 years, 9 months ago (2015-09-08 15:12:34 UTC) #11
jkarlin
pauljensen@: Gentle ping
3 years, 9 months ago (2015-09-09 16:16:19 UTC) #12
pauljensen
On 2015/09/09 16:16:19, jkarlin wrote: > pauljensen@: Gentle ping I was on bug triage duty ...
3 years, 9 months ago (2015-09-09 16:21:23 UTC) #13
jkarlin
pauljensen@: Understood, thanks. Just trying to get things in by feature freeze. adamk@: PTAL, thanks!
3 years, 9 months ago (2015-09-09 17:06:32 UTC) #15
pauljensen
https://codereview.chromium.org/1306423004/diff/170001/content/browser/net_info_browsertest.cc File content/browser/net_info_browsertest.cc (right): https://codereview.chromium.org/1306423004/diff/170001/content/browser/net_info_browsertest.cc#newcode61 content/browser/net_info_browsertest.cc:61: } Now that you unprotected NCN::GetMaxBandwidthForConnectionSubType(), this function seems ...
3 years, 9 months ago (2015-09-11 14:25:37 UTC) #16
jkarlin
Thanks. https://codereview.chromium.org/1306423004/diff/170001/content/browser/net_info_browsertest.cc File content/browser/net_info_browsertest.cc (right): https://codereview.chromium.org/1306423004/diff/170001/content/browser/net_info_browsertest.cc#newcode61 content/browser/net_info_browsertest.cc:61: } On 2015/09/11 14:25:37, pauljensen wrote: > Now ...
3 years, 9 months ago (2015-09-11 20:15:21 UTC) #17
pauljensen
lgtm https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h#newcode246 net/base/network_change_notifier.h:246: // Returns a theoretical upper limit (in mbps) ...
3 years, 9 months ago (2015-09-15 17:39:24 UTC) #18
jkarlin
Thanks. adamk PTAL! https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h#newcode246 net/base/network_change_notifier.h:246: // Returns a theoretical upper limit ...
3 years, 9 months ago (2015-09-15 17:51:49 UTC) #19
pauljensen
https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h#newcode246 net/base/network_change_notifier.h:246: // Returns a theoretical upper limit (in mbps) on ...
3 years, 9 months ago (2015-09-15 17:58:32 UTC) #20
jkarlin
https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306423004/diff/250001/net/base/network_change_notifier.h#newcode246 net/base/network_change_notifier.h:246: // Returns a theoretical upper limit (in mbps) on ...
3 years, 9 months ago (2015-09-15 18:07:19 UTC) #21
pauljensen
lgtm
3 years, 9 months ago (2015-09-15 18:08:30 UTC) #22
adamk
I'm happy to take a look but I'm not an OWNER in any of this ...
3 years, 9 months ago (2015-09-15 18:48:24 UTC) #23
jkarlin
-r adamk by request jochen@chromium.org: Please review content/ changes for OWNERS which are primarily plumbing ...
3 years, 9 months ago (2015-09-15 18:55:08 UTC) #25
jochen (gone - plz use gerrit)
content lgtm https://codereview.chromium.org/1306423004/diff/290001/content/browser/net_info_browsertest.cc File content/browser/net_info_browsertest.cc (right): https://codereview.chromium.org/1306423004/diff/290001/content/browser/net_info_browsertest.cc#newcode18 content/browser/net_info_browsertest.cc:18: using net::NetworkChangeNotifier; please no using statements https://codereview.chromium.org/1306423004/diff/290001/net/base/network_change_notifier.h ...
3 years, 9 months ago (2015-09-16 11:26:18 UTC) #26
jkarlin
Thanks! https://codereview.chromium.org/1306423004/diff/290001/content/browser/net_info_browsertest.cc File content/browser/net_info_browsertest.cc (right): https://codereview.chromium.org/1306423004/diff/290001/content/browser/net_info_browsertest.cc#newcode18 content/browser/net_info_browsertest.cc:18: using net::NetworkChangeNotifier; On 2015/09/16 11:26:18, jochen wrote: > ...
3 years, 9 months ago (2015-09-16 12:09:36 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1306423004/350001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1306423004/350001
3 years, 9 months ago (2015-09-18 15:18:14 UTC) #30
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/101528)
3 years, 9 months ago (2015-09-18 15:27:48 UTC) #32
jkarlin
tsepez@chromium.org: Please review changes in view_messages.h and common_param_traits_macros.h. Thank you!
3 years, 9 months ago (2015-09-18 15:34:54 UTC) #34
Tom Sepez
Messages LGTM.
3 years, 9 months ago (2015-09-18 15:47:33 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1306423004/350001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1306423004/350001
3 years, 9 months ago (2015-09-18 15:49:43 UTC) #37
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/105079)
3 years, 9 months ago (2015-09-18 17:17:50 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1306423004/370001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1306423004/370001
3 years, 9 months ago (2015-09-18 18:18:04 UTC) #42
commit-bot: I haz the power
Committed patchset #13 (id:370001)
3 years, 9 months ago (2015-09-18 19:32:59 UTC) #43
commit-bot: I haz the power
3 years, 9 months ago (2015-09-18 19:34:01 UTC) #44
Message was sent while issue was closed.
Patchset 13 (id:??) landed as
https://crrev.com/300a1d28b57bc0d49603c97828f1833f577bd46a
Cr-Commit-Position: refs/heads/master@{#349739}

Powered by Google App Engine
This is Rietveld 408576698