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

Issue 2819993002: [CrOS Tether] Add the notion of a tether DeviceState. (Closed)

Created:
8 months ago by Kyle Horimoto
Modified:
7 months, 3 weeks 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] Add the notion of a tether DeviceState. This CL: (1) Adds a tether DeviceState to NetworkStateHandler during initialization. (2) Adds SetTetherTechnologyState() and SetTetherScanState() functions which can be called from the tether component. (3) Updates the SetTechnologyEnabled(), SetProhibitedTechnologies(), and FormattedHardwareAddressForType() functions to deal properly with the tether DeviceState. (4) Adds a new TetherDeviceStateManager class to the tether component which will handle setting the appropriate technology state. The class is mostly unimplemented for now. BUG=672263 Review-Url: https://codereview.chromium.org/2819993002 Cr-Commit-Position: refs/heads/master@{#466185} Committed: https://chromium.googlesource.com/chromium/src/+/5df3c973662600cb69c4d5e6ac8ae0d1870d10dc

Patch Set 1 #

Patch Set 2 : Ready for review. #

Patch Set 3 : Enable tethering in tethering-related tests. #

Patch Set 4 : Rebased. #

Patch Set 5 : Rebased. #

Patch Set 6 : Rebased. #

Patch Set 7 : Rebased. #

Patch Set 8 : Rebased. #

Total comments: 26

Patch Set 9 : hansberry@ comments. #

Total comments: 18

Patch Set 10 : stevenjb@ comments. #

Total comments: 12

Patch Set 11 : stevenjb@ comments. #

Patch Set 12 : stevenjb@ comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+398 lines, -17 lines) Patch
M chromeos/components/tether/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M chromeos/components/tether/active_host_network_state_updater_unittest.cc View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M chromeos/components/tether/host_scanner_unittest.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M chromeos/components/tether/initializer.h View 2 chunks +2 lines, -0 lines 0 comments Download
M chromeos/components/tether/initializer.cc View 1 2 3 4 5 6 7 8 9 2 chunks +3 lines, -0 lines 0 comments Download
M chromeos/components/tether/tether_connector_unittest.cc View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A chromeos/components/tether/tether_device_state_manager.h View 1 chunk +48 lines, -0 lines 0 comments Download
A chromeos/components/tether/tether_device_state_manager.cc View 1 chunk +38 lines, -0 lines 0 comments Download
M chromeos/network/device_state.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chromeos/network/managed_state.h View 1 chunk +1 line, -0 lines 0 comments Download
M chromeos/network/network_connect_unittest.cc View 1 2 3 chunks +9 lines, -0 lines 0 comments Download
M chromeos/network/network_connection_handler.h View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -0 lines 0 comments Download
M chromeos/network/network_connection_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M chromeos/network/network_state_handler.h View 1 2 3 4 5 6 7 8 9 10 5 chunks +25 lines, -2 lines 0 comments Download
M chromeos/network/network_state_handler.cc View 1 2 3 4 5 6 7 8 9 10 16 chunks +146 lines, -11 lines 0 comments Download
M chromeos/network/network_state_handler_unittest.cc View 1 2 3 4 5 6 7 8 9 14 chunks +104 lines, -4 lines 0 comments Download
M chromeos/network/tether_constants.h View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M chromeos/network/tether_constants.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 26 (6 generated)
Kyle Horimoto
stevenjb: //chromeos/network changes hansberry: //chromeos/components/tether changes
7 months, 3 weeks ago (2017-04-18 21:06:26 UTC) #4
Kyle Horimoto
stevenjb: //chromeos/network changes hansberry: //chromeos/components/tether changes
7 months, 3 weeks ago (2017-04-18 21:06:27 UTC) #5
stevenjb
This looks like it includes changes from another CL? To upload just the differences, use: ...
7 months, 3 weeks ago (2017-04-19 21:21:45 UTC) #6
Kyle Horimoto
Rebased and set upstream properly. PTAL.
7 months, 3 weeks ago (2017-04-19 23:11:33 UTC) #7
Ryan Hansberry
https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/device_state.cc File chromeos/network/device_state.cc (right): https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/device_state.cc#newcode12 chromeos/network/device_state.cc:12: #include "chromeos/network/tether_constants.h" Is this include necessary? https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/managed_state.h File chromeos/network/managed_state.h ...
7 months, 3 weeks ago (2017-04-20 01:38:36 UTC) #8
Kyle Horimoto
https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/device_state.cc File chromeos/network/device_state.cc (right): https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/device_state.cc#newcode12 chromeos/network/device_state.cc:12: #include "chromeos/network/tether_constants.h" On 2017/04/20 01:38:35, Ryan Hansberry wrote: > ...
7 months, 3 weeks ago (2017-04-20 02:16:08 UTC) #9
stevenjb
https://codereview.chromium.org/2819993002/diff/160001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819993002/diff/160001/chromeos/network/network_state_handler.cc#newcode159 chromeos/network/network_state_handler.cc:159: enabled ? TECHNOLOGY_ENABLED : TECHNOLOGY_AVAILABLE; We need to ensure ...
7 months, 3 weeks ago (2017-04-20 16:49:43 UTC) #10
Ryan Hansberry
https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc#newcode376 chromeos/network/network_state_handler.cc:376: network = GetNetworkStateFromGuid(network->tether_guid()); On 2017/04/20 02:16:08, Kyle Horimoto wrote: ...
7 months, 3 weeks ago (2017-04-20 16:53:54 UTC) #11
stevenjb
https://codereview.chromium.org/2819993002/diff/160001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819993002/diff/160001/chromeos/network/network_state_handler.cc#newcode179 chromeos/network/network_state_handler.cc:179: tether_technology_state_ = technology_state; So, to go with my comments ...
7 months, 3 weeks ago (2017-04-20 18:13:45 UTC) #12
stevenjb
https://codereview.chromium.org/2819993002/diff/160001/chromeos/network/network_state_handler.h File chromeos/network/network_state_handler.h (right): https://codereview.chromium.org/2819993002/diff/160001/chromeos/network/network_state_handler.h#newcode504 chromeos/network/network_state_handler.h:504: TechnologyState::TECHNOLOGY_PROHIBITED; This should be initialized to UNAVAILABLE which should ...
7 months, 3 weeks ago (2017-04-20 18:29:07 UTC) #13
Kyle Horimoto
https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc#newcode376 chromeos/network/network_state_handler.cc:376: network = GetNetworkStateFromGuid(network->tether_guid()); On 2017/04/20 16:53:54, Ryan Hansberry wrote: ...
7 months, 3 weeks ago (2017-04-20 19:38:46 UTC) #14
stevenjb
https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc#newcode376 chromeos/network/network_state_handler.cc:376: network = GetNetworkStateFromGuid(network->tether_guid()); On 2017/04/20 19:38:46, Kyle Horimoto wrote: ...
7 months, 3 weeks ago (2017-04-20 20:01:11 UTC) #15
Kyle Horimoto
https://codereview.chromium.org/2819993002/diff/180001/chromeos/network/network_connection_handler.cc File chromeos/network/network_connection_handler.cc (right): https://codereview.chromium.org/2819993002/diff/180001/chromeos/network/network_connection_handler.cc#newcode122 chromeos/network/network_connection_handler.cc:122: "enabled-or-disabled-when-not-available"; On 2017/04/20 20:01:10, stevenjb wrote: > nit: I ...
7 months, 3 weeks ago (2017-04-20 20:49:20 UTC) #16
Ryan Hansberry
https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc#newcode376 chromeos/network/network_state_handler.cc:376: network = GetNetworkStateFromGuid(network->tether_guid()); On 2017/04/20 20:01:10, stevenjb wrote: > ...
7 months, 3 weeks ago (2017-04-20 21:27:02 UTC) #17
Kyle Horimoto
https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc File chromeos/network/network_state_handler.cc (right): https://codereview.chromium.org/2819993002/diff/140001/chromeos/network/network_state_handler.cc#newcode376 chromeos/network/network_state_handler.cc:376: network = GetNetworkStateFromGuid(network->tether_guid()); On 2017/04/20 21:27:02, Ryan Hansberry wrote: ...
7 months, 3 weeks ago (2017-04-20 21:28:45 UTC) #18
stevenjb
lgtm
7 months, 3 weeks ago (2017-04-20 21:54:37 UTC) #19
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/2819993002/220001
7 months, 3 weeks ago (2017-04-20 21:57:02 UTC) #21
Ryan Hansberry
lgtm
7 months, 3 weeks ago (2017-04-20 22:29:21 UTC) #22
commit-bot: I haz the power
Committed patchset #12 (id:220001) as https://chromium.googlesource.com/chromium/src/+/5df3c973662600cb69c4d5e6ac8ae0d1870d10dc
7 months, 3 weeks ago (2017-04-20 23:20:10 UTC) #25
keishi
7 months, 3 weeks ago (2017-04-21 01:19:48 UTC) #26
Message was sent while issue was closed.
A revert of this CL (patchset #12 id:220001) has been created in
https://codereview.chromium.org/2836543002/ by keishi@chromium.org.

The reason for reverting is: NetworkStateHandlerTest.TetherTechnologyState
started failing on Linux ChromiumOS Tests (dbg)

https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20C...
https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20C....

Powered by Google App Engine
This is Rietveld 0eb02b776