ARC: Implement SetWifiEnabledState API.

ARC: Implement SetWifiEnabledState API.

This change implements the SetWifiEnabledState API for the Host.

BUG=594723
BUG=b/27107139

Committed: https://crrev.com/9f654ac6a5d00c94ad1e612e3785fffd8e4df7c9
Cr-Commit-Position: refs/heads/master@{#381583}

[abhishekbh, 2016-03-14 23:23:59]

abhishekbh@google.com changed reviewers:
+ cernekee@google.com, dcheng@chromium.org

[abhishekbh, 2016-03-14 23:23:59]

PTAL

[abhishekbh, 2016-03-15 04:31:42]

abhishekbh@google.com changed reviewers:
+ stevenjb@chromium.org

[abhishekbh, 2016-03-15 18:17:53]

abhishekbh@google.com changed reviewers:
+ hidehiko@chromium.org

[dcheng, 2016-03-15 23:03:00]

mojom changes seem simple enough, but I'll wait for an ARC++ owner to review
before signing off

[stevenjb, 2016-03-15 23:12:14]

lgtm

[dcheng, 2016-03-15 23:19:43]

https://codereview.chromium.org/1798293002/diff/1/components/arc/net/arc_net_...
File components/arc/net/arc_net_host_impl.cc (right):

https://codereview.chromium.org/1798293002/diff/1/components/arc/net/arc_net_...
components/arc/net/arc_net_host_impl.cc:153:
chromeos::network_handler::ErrorCallback());
Out of curiosity... how are you supposed to know if it succeeded? The lack of
the error callback being called?

[stevenjb, 2016-03-15 23:31:31]

https://codereview.chromium.org/1798293002/diff/1/components/arc/net/arc_net_...
File components/arc/net/arc_net_host_impl.cc (right):

https://codereview.chromium.org/1798293002/diff/1/components/arc/net/arc_net_...
components/arc/net/arc_net_host_impl.cc:153:
chromeos::network_handler::ErrorCallback());
On 2016/03/15 23:19:43, dcheng wrote:
> Out of curiosity... how are you supposed to know if it succeeded? The lack of
> the error callback being called?

There's an email thread discussing this. Right now you don't know whether or not
it succeeded or failed. The error callback is so that known failures can be
logged, but in theory it could take ~10 seconds before a 'enabled' change
request succeeds or fails, so at the moment the caller needs to observe
technology state changes and decide how to handle that. (The current UI doesn't
do anything special, it just sends the request reflects the current state).

[dcheng, 2016-03-15 23:33:33]

lgtm

[abhishekbh, 2016-03-16 00:23:53]

Thanks for the approvals. The callback question has been discussed and for the
time being I am checking myself if modifying WiFi is legal or not and if it
isn't I return false. All other cases call the API.

[hidehiko, 2016-03-16 00:45:58]

https://codereview.chromium.org/1798293002/diff/20001/components/arc/common/n...
File components/arc/common/net.mojom (right):

https://codereview.chromium.org/1798293002/diff/20001/components/arc/common/n...
components/arc/common/net.mojom:52: [MinVersion=3] SetWifiEnabledState@4(bool
is_enabled) => (bool result);
As |result| is not actually if "setting is succeeded or not" as discussed (in
terms of setting wifi state error handling), could you add detailed comment
about it?

[abhishekbh, 2016-03-16 01:08:34]

On 2016/03/16 00:45:58, hidehiko wrote:
>
https://codereview.chromium.org/1798293002/diff/20001/components/arc/common/n...
> File components/arc/common/net.mojom (right):
> 
>
https://codereview.chromium.org/1798293002/diff/20001/components/arc/common/n...
> components/arc/common/net.mojom:52: [MinVersion=3] SetWifiEnabledState@4(bool
> is_enabled) => (bool result);
> As |result| is not actually if "setting is succeeded or not" as discussed (in
> terms of setting wifi state error handling), could you add detailed comment
> about it?

Yep, Done.

[hidehiko, 2016-03-16 01:47:56]

lgtm

https://codereview.chromium.org/1798293002/diff/40001/components/arc/common/n...
File components/arc/common/net.mojom (right):

https://codereview.chromium.org/1798293002/diff/40001/components/arc/common/n...
components/arc/common/net.mojom:52: // the state has been successfully set or
WiFi is already in the desired
How about mentioning about that the setting can fail in later phase, but the
error will be simply ignored.

https://codereview.chromium.org/1798293002/diff/40001/components/arc/common/n...
components/arc/common/net.mojom:53: // state. It is false if WiFi manipulation
is prohibited due to a policy or its
nit: wrap line.

[abhishekbh, 2016-03-16 22:46:09]

The CQ bit was checked by abhishekbh@google.com

[abhishekbh, 2016-03-16 22:46:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, dcheng@chromium.org, hidehiko@chromium.org
Link to the patchset: https://codereview.chromium.org/1798293002/#ps60001
(title: "Formatting changes in comments.")

[commit-bot: I haz the power, 2016-03-16 22:46:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1798293002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1798293002/60001

[commit-bot: I haz the power, 2016-03-16 23:22:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-16 23:26:38]

Description was changed from

==========
ARC: Implement SetWifiEnabledState API.

This change implements the SetWifiEnabledState API for the Host.

BUG=594723
BUG=b/27107139
==========

to

==========
ARC: Implement SetWifiEnabledState API.

This change implements the SetWifiEnabledState API for the Host.

BUG=594723
BUG=b/27107139

Committed: https://crrev.com/9f654ac6a5d00c94ad1e612e3785fffd8e4df7c9
Cr-Commit-Position: refs/heads/master@{#381583}
==========

[commit-bot: I haz the power, 2016-03-16 23:26:39]

Patchset 4 (id:??) landed as
https://crrev.com/9f654ac6a5d00c94ad1e612e3785fffd8e4df7c9
Cr-Commit-Position: refs/heads/master@{#381583}