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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 1 week ago by Kyle Horimoto
Modified:
2 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] 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 26 (6 generated)
Kyle Horimoto
stevenjb: //chromeos/network changes hansberry: //chromeos/components/tether changes
2 months, 1 week ago (2017-04-18 21:06:26 UTC) #4
Kyle Horimoto
stevenjb: //chromeos/network changes hansberry: //chromeos/components/tether changes
2 months, 1 week 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: ...
2 months ago (2017-04-19 21:21:45 UTC) #6
Kyle Horimoto
Rebased and set upstream properly. PTAL.
2 months 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 ...
2 months 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: > ...
2 months 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 ...
2 months 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: ...
2 months 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 ...
2 months 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 ...
2 months 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: ...
2 months 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: ...
2 months 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 ...
2 months 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: > ...
2 months 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: ...
2 months ago (2017-04-20 21:28:45 UTC) #18
stevenjb
lgtm
2 months 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
2 months ago (2017-04-20 21:57:02 UTC) #21
Ryan Hansberry
lgtm
2 months 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
2 months ago (2017-04-20 23:20:10 UTC) #25
keishi
2 months 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....
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318