[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

After the bootstrapping attempt is successful/failed, or the user has logged into the system,
power off the Bluetooth adapter only if the Bluetooth adapter was off before and there is no
connected Bluetooth type HID device.

BUG=chrome-os-partner:52828
Committed: https://crrev.com/b6391c22b6339579ade0295e5995c66da3f9a239
Cr-Commit-Position: refs/heads/master@{#391731}

[xdai1, 2016-05-02 22:43:31]

Patchset #3 (id:40001) has been deleted

[xdai1, 2016-05-02 22:44:27]

Description was changed from

==========
[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

After the bootstrapping attempt is successful/failed, or the user has logged
into the system, power off the Bluetooth adapter only if the Bluetooth adapter
was off before and there is no connected Bluetooth type HID device.

BUG=
==========

to

==========
[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

After the bootstrapping attempt is successful/failed, or the user has logged
into the system, 
power off the Bluetooth adapter only if the Bluetooth adapter was off before and
there is no 
connected Bluetooth type HID device.

BUG=chrome-os-partner:52828
==========

[xdai1, 2016-05-02 22:48:20]

xdai@chromium.org changed reviewers:
+ achuith@chromium.org

[xdai1, 2016-05-02 22:48:20]

achuith@, could you help review this CL please? Thanks!

[achuithb, 2016-05-02 23:02:40]

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
File components/pairing/bluetooth_host_pairing_controller.cc (right):

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
components/pairing/bluetooth_host_pairing_controller.cc:171:
content::BrowserThread::FILE, FROM_HERE, base::Bind(&GetDevices),
BrowserThread::FILE is deprecated, please use BlockingPool instead.

Something like this:
https://code.google.com/p/chromium/codesearch#chromium/src/components/feedbac...

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
components/pairing/bluetooth_host_pairing_controller.cc:335: for (auto& iter :
devices) {
const auto& device

The iterator should be const, and device is a better name than iter

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
components/pairing/bluetooth_host_pairing_controller.cc:346: adapter_ = NULL;
nullptr here and everywhere in this file please

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
File components/pairing/bluetooth_host_pairing_controller.h (right):

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
components/pairing/bluetooth_host_pairing_controller.h:41: typedef
device::InputServiceLinux::InputDeviceInfo InputDeviceInfo;
We prefer using, please switch this and HostingPairingController::Observer.

https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/8dOAMzgR4ao

[xdai1, 2016-05-03 00:30:26]

xdai@chromium.org changed reviewers:
+ avi@chromium.org, rockot@chromium.org

[xdai1, 2016-05-03 00:30:27]

achuith@, I've addressed your comments. Please take another look, thanks! 

+rockot@ for:
    the dependency /device/hid added in components/pairing/DEPS 
    device/hid/fake_input_service_linux.cc
    device/hid/fake_input_service_linux.h
    device/hid/input_service_linux.h

+avi@ for the dependency /content/public added in components/pairing/DEPS .

Thanks!

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
File components/pairing/bluetooth_host_pairing_controller.cc (right):

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
components/pairing/bluetooth_host_pairing_controller.cc:171:
content::BrowserThread::FILE, FROM_HERE, base::Bind(&GetDevices),
On 2016/05/02 23:02:40, achuithb wrote:
> BrowserThread::FILE is deprecated, please use BlockingPool instead.
> 
> Something like this:
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/feedbac...

I'm not sure I should do that.. In
https://code.google.com/p/chromium/codesearch#chromium/src/device/hid/input_s...
it says all methods in InputServiceLinux should be called from the FILE thread.

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
components/pairing/bluetooth_host_pairing_controller.cc:335: for (auto& iter :
devices) {
On 2016/05/02 23:02:40, achuithb wrote:
> const auto& device
> 
> The iterator should be const, and device is a better name than iter

Done.

https://codereview.chromium.org/1942883003/diff/60001/components/pairing/blue...
components/pairing/bluetooth_host_pairing_controller.cc:346: adapter_ = NULL;
On 2016/05/02 23:02:40, achuithb wrote:
> nullptr here and everywhere in this file please

Done.

[Avi (use Gerrit), 2016-05-03 01:47:02]

mmmm

The point of components is that they don't depend on content.

Why don't we have threading in base/?

[Ken Rockot(use gerrit already), 2016-05-03 16:01:58]

device/hid lgtm.

fwiw re avi@'s comment, there are plenty of components which depend on content,
but in this case i agree that the dependency is totally unnecessary and should
definitely be avoided.

could you please construct the controller over a
scoped_refptr<base::SingleThreadTaskRunner> (to be the task runner which does
blocking/file tasks) and pass one in wherever it's constructed?

[xdai1, 2016-05-03 18:49:08]

rockot@, avi@, I removed the dependency on content/public as you suggested,
please take another look, thanks for your review!

[Avi (use Gerrit), 2016-05-04 00:03:08]

avi@chromium.org changed reviewers:
- avi@chromium.org

[Avi (use Gerrit), 2016-05-04 00:03:08]

I'm happy, but there's nothing left here that requires my review.

[Ken Rockot(use gerrit already), 2016-05-04 00:05:22]

lgtm

[xdai1, 2016-05-04 17:04:13]

On 2016/05/04 00:05:22, Ken Rockot wrote:
> lgtm

Thanks! achuith@, could you help take another look at this CL please?

[achuithb, 2016-05-04 22:05:04]

lgtm, please fix the nits below

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
File device/hid/fake_input_service_linux.cc (right):

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
device/hid/fake_input_service_linux.cc:31: for (const auto& device : devices_) {
Don't need {} here

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
File device/hid/fake_input_service_linux.h (left):

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
device/hid/fake_input_service_linux.h:25:
DISALLOW_COPY_AND_ASSIGN(FakeInputServiceLinux);
This isn't right - it needs to be private

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
File device/hid/fake_input_service_linux.h (right):

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
device/hid/fake_input_service_linux.h:16: 
get rid of this newline

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
device/hid/fake_input_service_linux.h:24: 
Could you add a private: here?

[xdai1, 2016-05-04 22:25:01]

Addressed! Thanks for your review.

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
File device/hid/fake_input_service_linux.cc (right):

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
device/hid/fake_input_service_linux.cc:31: for (const auto& device : devices_) {
On 2016/05/04 22:05:03, achuithb wrote:
> Don't need {} here

Done.

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
File device/hid/fake_input_service_linux.h (right):

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
device/hid/fake_input_service_linux.h:16: 
On 2016/05/04 22:05:04, achuithb wrote:
> get rid of this newline

Done.

https://codereview.chromium.org/1942883003/diff/100001/device/hid/fake_input_...
device/hid/fake_input_service_linux.h:24: 
On 2016/05/04 22:05:03, achuithb wrote:
> Could you add a private: here? 

Done.

[xdai1, 2016-05-04 23:11:03]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2016-05-04 23:11:03]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/1942883003/#ps120001
(title: "Address achuith@'s comments.")

[commit-bot: I haz the power, 2016-05-04 23:11:52]

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

[commit-bot: I haz the power, 2016-05-05 02:09:04]

Description was changed from

==========
[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

After the bootstrapping attempt is successful/failed, or the user has logged
into the system, 
power off the Bluetooth adapter only if the Bluetooth adapter was off before and
there is no 
connected Bluetooth type HID device.

BUG=chrome-os-partner:52828
==========

to

==========
[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

After the bootstrapping attempt is successful/failed, or the user has logged
into the system, 
power off the Bluetooth adapter only if the Bluetooth adapter was off before and
there is no 
connected Bluetooth type HID device.

BUG=chrome-os-partner:52828
==========

[commit-bot: I haz the power, 2016-05-05 02:09:05]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-05-05 02:11:57]

Description was changed from

==========
[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

After the bootstrapping attempt is successful/failed, or the user has logged
into the system, 
power off the Bluetooth adapter only if the Bluetooth adapter was off before and
there is no 
connected Bluetooth type HID device.

BUG=chrome-os-partner:52828
==========

to

==========
[Chrome OS Bootstrapping]: Do not always power off the Bluetooth adapter.

After the bootstrapping attempt is successful/failed, or the user has logged
into the system,
power off the Bluetooth adapter only if the Bluetooth adapter was off before and
there is no
connected Bluetooth type HID device.

BUG=chrome-os-partner:52828

Committed: https://crrev.com/b6391c22b6339579ade0295e5995c66da3f9a239
Cr-Commit-Position: refs/heads/master@{#391731}
==========

[commit-bot: I haz the power, 2016-05-05 02:11:58]

Patchset 6 (id:??) landed as
https://crrev.com/b6391c22b6339579ade0295e5995c66da3f9a239
Cr-Commit-Position: refs/heads/master@{#391731}