[CrOS Tether] Create TetherDisconnector, which disconnects from active tethering sessions.

[CrOS Tether] Create TetherDisconnector, which disconnects from active tethering sessions.

This CL also creates NetworkConnectionHandlerTetherDelegate, which controls both tether connections and disconnections when requested in NetworkConnectionHandler.

BUG=672263
Review-Url: https://codereview.chromium.org/2857853005
Cr-Commit-Position: refs/heads/master@{#469256}
Committed: https://chromium.googlesource.com/chromium/src/+/8d47055fe57c19eee560dfd25f5109845b1ad7fc

[Kyle Horimoto, 2017-05-04 00:46:07]

khorimoto@chromium.org changed reviewers:
+ hansberry@chromium.org

[Kyle Horimoto, 2017-05-04 00:46:52]

Description was changed from

==========
[CrOS Tether] Create TetherDisconnector, which disconnects from active tethering
sessions.

This CL also creates NetworkConnectionHandlerTetherDelegate, which controls both
tether connections and disconnections when requested in
NetworkConnectionHandler.

NOTE: This CL is a work in progress. Do not review yet.

BUG=672263
==========

to

==========
[CrOS Tether] Create TetherDisconnector, which disconnects from active tethering
sessions.

This CL also creates NetworkConnectionHandlerTetherDelegate, which controls both
tether connections and disconnections when requested in
NetworkConnectionHandler.

BUG=672263
==========

[Ryan Hansberry, 2017-05-04 01:23:01]

https://codereview.chromium.org/2857853005/diff/80001/chromeos/components/tet...
File chromeos/components/tether/tether_disconnector.h (right):

https://codereview.chromium.org/2857853005/diff/80001/chromeos/components/tet...
chromeos/components/tether/tether_disconnector.h:96:
std::unordered_map<std::string, DisconnectCallbacks>
Why do we need these maps? I can't foresee why we need multiple
DisconnectOperations running if we're only connected to one WiFi network.

[Kyle Horimoto, 2017-05-04 01:42:38]

https://codereview.chromium.org/2857853005/diff/80001/chromeos/components/tet...
File chromeos/components/tether/tether_disconnector.h (right):

https://codereview.chromium.org/2857853005/diff/80001/chromeos/components/tet...
chromeos/components/tether/tether_disconnector.h:96:
std::unordered_map<std::string, DisconnectCallbacks>
On 2017/05/04 01:23:00, Ryan Hansberry wrote:
> Why do we need these maps? I can't foresee why we need multiple
> DisconnectOperations running if we're only connected to one WiFi network.

Done.

[Ryan Hansberry, 2017-05-04 01:56:28]

lgtm with nits

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
File chromeos/components/tether/tether_disconnector.cc (right):

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
chromeos/components/tether/tether_disconnector.cc:90:
DisconnectActiveWifiConnection(tether_network_guid, active_wifi_network_guid,
I think it would be safe to ensure disconnect_tethering_operation_ is null here
-- in case the user could spam a Disconnect button, for example.

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
File chromeos/components/tether/tether_disconnector_unittest.cc (right):

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
chromeos/components/tether/tether_disconnector_unittest.cc:296:
result.swap(result_);
Please use a more descriptive name.

[Kyle Horimoto, 2017-05-04 02:00:14]

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
File chromeos/components/tether/tether_disconnector.cc (right):

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
chromeos/components/tether/tether_disconnector.cc:90:
DisconnectActiveWifiConnection(tether_network_guid, active_wifi_network_guid,
On 2017/05/04 01:56:28, Ryan Hansberry wrote:
> I think it would be safe to ensure disconnect_tethering_operation_ is null
here
> -- in case the user could spam a Disconnect button, for example.

That won't be possible. Once the active host is set to disconnected below, any
subsequent attempt to disconnect will be rejected by
NetworkConnectionHandlerImpl before the call reaches here.

Added a DCHECK() to confirm this.

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
File chromeos/components/tether/tether_disconnector_unittest.cc (right):

https://codereview.chromium.org/2857853005/diff/100001/chromeos/components/te...
chromeos/components/tether/tether_disconnector_unittest.cc:296:
result.swap(result_);
On 2017/05/04 01:56:28, Ryan Hansberry wrote:
> Please use a more descriptive name.

Done.

[Kyle Horimoto, 2017-05-04 02:00:18]

The CQ bit was checked by khorimoto@chromium.org

[Kyle Horimoto, 2017-05-04 02:00:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
hansberry@chromium.org
Link to the patchset: https://codereview.chromium.org/2857853005/#ps120001
(title: "hansberry@ comments.")

[commit-bot: I haz the power, 2017-05-04 02:00:45]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 03:10:09]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493863218011580,
"parent_rev": "8c6cff65cbe641a213c46a086fd7efb939b0e355", "commit_rev":
"8d47055fe57c19eee560dfd25f5109845b1ad7fc"}

[commit-bot: I haz the power, 2017-05-04 03:10:50]

Description was changed from

==========
[CrOS Tether] Create TetherDisconnector, which disconnects from active tethering
sessions.

This CL also creates NetworkConnectionHandlerTetherDelegate, which controls both
tether connections and disconnections when requested in
NetworkConnectionHandler.

BUG=672263
==========

to

==========
[CrOS Tether] Create TetherDisconnector, which disconnects from active tethering
sessions.

This CL also creates NetworkConnectionHandlerTetherDelegate, which controls both
tether connections and disconnections when requested in
NetworkConnectionHandler.

BUG=672263

Review-Url: https://codereview.chromium.org/2857853005
Cr-Commit-Position: refs/heads/master@{#469256}
Committed:
https://chromium.googlesource.com/chromium/src/+/8d47055fe57c19eee560dfd25f51...
==========

[commit-bot: I haz the power, 2017-05-04 03:10:51]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/8d47055fe57c19eee560dfd25f51...