Chromium Code Reviews
Help | Chromium Project | 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 weeks ago by Kyle Horimoto
Modified:
1 week, 1 day 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
1 week, 3 days ago (2017-04-18 21:06:26 UTC) #4
Kyle Horimoto
stevenjb: //chromeos/network changes hansberry: //chromeos/components/tether changes
1 week, 3 days 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: ...
1 week, 2 days ago (2017-04-19 21:21:45 UTC) #6
Kyle Horimoto
Rebased and set upstream properly. PTAL.
1 week, 2 days 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 ...
1 week, 2 days 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: > ...
1 week, 2 days 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 ...
1 week, 1 day 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: ...
1 week, 1 day 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 ...
1 week, 1 day 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 ...
1 week, 1 day 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: ...
1 week, 1 day 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: ...
1 week, 1 day 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 ...
1 week, 1 day 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: > ...
1 week, 1 day 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: ...
1 week, 1 day ago (2017-04-20 21:28:45 UTC) #18
stevenjb
lgtm
1 week, 1 day 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
1 week, 1 day ago (2017-04-20 21:57:02 UTC) #21
Ryan Hansberry
lgtm
1 week, 1 day 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
1 week, 1 day ago (2017-04-20 23:20:10 UTC) #25
keishi
1 week, 1 day 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 cc6ac46