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

Issue 2819383002: [CrOS Tether] Update NetworkState to include tether properties and integrate into NetworkStateHandl… (Closed)

Created:
3 years, 8 months ago by Kyle Horimoto
Modified:
3 years, 8 months ago
CC:
chromium-reviews, jlklein+watch-tether_chromium.org, tengs+watch-tether_chromium.org, stevenjb+watch_chromium.org, hansberry+watch-tether_chromium.org, jhawkins+watch-tether_chromium.org, oshima+watch_chromium.org, lesliewatkins+watch-tether_chromium.org, khorimoto+watch-tether_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[CrOS Tether] Update NetworkState to include tether properties and integrate into NetworkStateHandler. This CL: (1) Adds tether properties (battery percentage, carrier, signal strength) to NetworkState. (2) Updates NetworkStateHandler::AddTetherNetworkState() to include parameters for these properties, and adds an UpdateTetherNetworkProperties() function to update these values at a later time. (3) Correctly returns tether networks for NetworkStateHandler's getter functions: ConnectedNetworkByType(), ConnectingNetworkByType(), FirstNetworkByType(), GetNetworkListByType() (4) Updates the tether component's HostScanner to pass tether properties to NetworkStateHandler when scan results come in. With this CL, the chrome.networkingPrivate API successfully returns tether networks! BUG=672263 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2819383002 Cr-Commit-Position: refs/heads/master@{#466085} Committed: https://chromium.googlesource.com/chromium/src/+/71c16b98c68a2b66290b656da91cd1992664655e

Patch Set 1 #

Patch Set 2 : Comments. #

Total comments: 1

Patch Set 3 : Fix comments, add style change, change a LOG to a DCHECK. #

Total comments: 31

Patch Set 4 : hansberry@ comments. #

Patch Set 5 : Update TODOs. #

Total comments: 20

Patch Set 6 : Rebased. #

Patch Set 7 : Rebased. #

Total comments: 8

Patch Set 8 : hansberry@ comments. #

Patch Set 9 : Made GetTetherNetworkList() public again. #

Patch Set 10 : Remove unused function. #

Patch Set 11 : stevenjb@ and hansberry@ comments. #

Total comments: 10

Patch Set 12 : stevenjb@ comments. #

Patch Set 13 : stevenjb@ comments. #

Total comments: 2

Patch Set 14 : stevenjb@ comments. #

Total comments: 2

Patch Set 15 : stevenjb@ comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+402 lines, -91 lines) Patch
M ash/system/network/network_list.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/resources/settings/internet_page/network_summary.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +3 lines, -1 line 0 comments Download
M chromeos/components/tether/active_host_network_state_updater_unittest.cc View 1 2 3 4 5 3 chunks +4 lines, -4 lines 0 comments Download
M chromeos/components/tether/host_scan_scheduler_unittest.cc View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M chromeos/components/tether/host_scanner.h View 3 chunks +4 lines, -1 line 0 comments Download
M chromeos/components/tether/host_scanner.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +53 lines, -7 lines 0 comments Download
M chromeos/components/tether/host_scanner_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 6 chunks +14 lines, -8 lines 0 comments Download
M chromeos/components/tether/initializer.cc View 1 chunk +1 line, -1 line 0 comments Download
M chromeos/components/tether/tether_connector_unittest.cc View 1 2 3 4 5 6 7 3 chunks +5 lines, -6 lines 0 comments Download
M chromeos/network/network_connect_unittest.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M chromeos/network/network_state.h View 1 2 3 4 5 6 7 8 9 10 5 chunks +17 lines, -0 lines 0 comments Download
M chromeos/network/network_state.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +10 lines, -0 lines 0 comments Download
M chromeos/network/network_state_handler.h View 1 2 3 4 5 6 7 9 10 4 chunks +32 lines, -20 lines 0 comments Download
M chromeos/network/network_state_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 12 chunks +122 lines, -17 lines 0 comments Download
M chromeos/network/network_state_handler_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 11 chunks +103 lines, -21 lines 0 comments Download
M chromeos/network/network_state_unittest.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +26 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (6 generated)
Kyle Horimoto
stevenjb: For //chromeos/network changes. hansberry: For //chromeos/components/tether changes. https://codereview.chromium.org/2819383002/diff/20001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/20001/chromeos/network/network_state_handler.cc#newcode504 chromeos/network/network_state_handler.cc:504: // ...
3 years, 8 months ago (2017-04-18 01:16:06 UTC) #2
Ryan Hansberry
https://codereview.chromium.org/2819383002/diff/40001/chromeos/components/tether/host_scanner.cc File chromeos/components/tether/host_scanner.cc (right): https://codereview.chromium.org/2819383002/diff/40001/chromeos/components/tether/host_scanner.cc#newcode88 chromeos/components/tether/host_scanner.cc:88: // invoked, which will add a bunch of duplicate ...
3 years, 8 months ago (2017-04-18 16:08:56 UTC) #3
Kyle Horimoto
https://codereview.chromium.org/2819383002/diff/40001/chromeos/components/tether/host_scanner.cc File chromeos/components/tether/host_scanner.cc (right): https://codereview.chromium.org/2819383002/diff/40001/chromeos/components/tether/host_scanner.cc#newcode88 chromeos/components/tether/host_scanner.cc:88: // invoked, which will add a bunch of duplicate ...
3 years, 8 months ago (2017-04-18 17:15:23 UTC) #4
Ryan Hansberry
lgtm with nits https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc#newcode325 chromeos/network/network_state_handler.cc:325: void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type, On 2017/04/18 ...
3 years, 8 months ago (2017-04-18 23:56:17 UTC) #6
Kyle Horimoto
https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc#newcode325 chromeos/network/network_state_handler.cc:325: void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type, On 2017/04/18 23:56:17, Ryan Hansberry ...
3 years, 8 months ago (2017-04-19 00:28:17 UTC) #7
Kyle Horimoto
https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc#newcode325 chromeos/network/network_state_handler.cc:325: void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type, On 2017/04/19 00:28:17, Kyle Horimoto ...
3 years, 8 months ago (2017-04-19 02:05:46 UTC) #8
stevenjb
Ugh, sorry, forgot to publish these. Some of the comments may be stale :( https://codereview.chromium.org/2819383002/diff/80001/chromeos/components/tether/host_scanner.cc ...
3 years, 8 months ago (2017-04-19 17:34:17 UTC) #9
Ryan Hansberry
https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc#newcode325 chromeos/network/network_state_handler.cc:325: void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type, On 2017/04/19 02:05:46, Kyle Horimoto ...
3 years, 8 months ago (2017-04-19 17:49:07 UTC) #10
Kyle Horimoto
https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/40001/chromeos/network/network_state_handler.cc#newcode325 chromeos/network/network_state_handler.cc:325: void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type, On 2017/04/19 17:49:07, Ryan Hansberry ...
3 years, 8 months ago (2017-04-19 19:23:29 UTC) #11
stevenjb
https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js File chrome/browser/resources/settings/internet_page/network_summary.js (right): https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js#newcode285 chrome/browser/resources/settings/internet_page/network_summary.js:285: Tether: [], We shouldn't add this here until we ...
3 years, 8 months ago (2017-04-19 21:16:03 UTC) #12
Kyle Horimoto
https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js File chrome/browser/resources/settings/internet_page/network_summary.js (right): https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js#newcode285 chrome/browser/resources/settings/internet_page/network_summary.js:285: Tether: [], On 2017/04/19 21:16:02, stevenjb wrote: > We ...
3 years, 8 months ago (2017-04-19 22:31:05 UTC) #13
stevenjb
https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js File chrome/browser/resources/settings/internet_page/network_summary.js (right): https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js#newcode285 chrome/browser/resources/settings/internet_page/network_summary.js:285: Tether: [], On 2017/04/19 22:31:04, Kyle Horimoto wrote: > ...
3 years, 8 months ago (2017-04-19 22:37:34 UTC) #14
stevenjb
https://codereview.chromium.org/2819383002/diff/240001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/240001/chromeos/network/network_state_handler.cc#newcode242 chromeos/network/network_state_handler.cc:242: if (connected_network && shill::kTypeEthernet == connected_network->type()) { nit: we ...
3 years, 8 months ago (2017-04-19 22:43:05 UTC) #15
Kyle Horimoto
https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js File chrome/browser/resources/settings/internet_page/network_summary.js (right): https://codereview.chromium.org/2819383002/diff/200001/chrome/browser/resources/settings/internet_page/network_summary.js#newcode285 chrome/browser/resources/settings/internet_page/network_summary.js:285: Tether: [], On 2017/04/19 22:37:34, stevenjb wrote: > On ...
3 years, 8 months ago (2017-04-19 23:08:59 UTC) #16
stevenjb
lgtm w/ nit https://codereview.chromium.org/2819383002/diff/260001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/260001/chromeos/network/network_state_handler.cc#newcode379 chromeos/network/network_state_handler.cc:379: if (shill::kTypeEthernet == network->type()) { nit: ...
3 years, 8 months ago (2017-04-20 16:13:51 UTC) #17
Kyle Horimoto
https://codereview.chromium.org/2819383002/diff/260001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819383002/diff/260001/chromeos/network/network_state_handler.cc#newcode379 chromeos/network/network_state_handler.cc:379: if (shill::kTypeEthernet == network->type()) { On 2017/04/20 16:13:51, stevenjb ...
3 years, 8 months ago (2017-04-20 17:16:40 UTC) #18
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/2819383002/280001
3 years, 8 months ago (2017-04-20 17:17:19 UTC) #21
commit-bot: I haz the power
3 years, 8 months ago (2017-04-20 18:48:25 UTC) #24
Message was sent while issue was closed.
Committed patchset #15 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/71c16b98c68a2b66290b656da91c...

Powered by Google App Engine
This is Rietveld 408576698