Migrate HidServiceLinux and HidConnectionLinux to BrowserThread::UI.

Migrate HidServiceLinux and HidConnectionLinux to BrowserThread::UI.

This patch is a follow-up to https://crrev.com/e8fa00efd0965a7eb5816a
that moves the HidService and HidConnection implementations on Linux
from the browser's FILE thread to the UI thread. This is being done
because the HID APIs on platforms other than Linux are more natural to
use on the UI thread. The users of these objects are also usually on
the UI thread. (For example, the extension API bindings.)

BUG=422540
Committed: https://crrev.com/56868deb02c83ade521fa482e2d03b9319162ae9
Cr-Commit-Position: refs/heads/master@{#306729}

[Reilly Grant (use Gerrit), 2014-12-03 02:40:25]

Patchset #1 (id:1) has been deleted

[Reilly Grant (use Gerrit), 2014-12-03 06:17:47]

reillyg@chromium.org changed reviewers:
+ rockot@chromium.org

[Reilly Grant (use Gerrit), 2014-12-03 06:18:05]

PTAL

[Reilly Grant (use Gerrit), 2014-12-03 22:09:20]

The Chrome OS issue has been resolved. This patch is ready to review.

[Ken Rockot(use gerrit already), 2014-12-03 22:35:10]

lgtm

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_connectio...
File device/hid/hid_connection_linux.cc (right):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_connectio...
device/hid/hid_connection_linux.cc:35: public base::NonThreadSafe {
nit: I think you can compose a base::ThreadChecker in this case, which would be
preferred according to the conditions outlined in
base/threading/thread_checker.h

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service.cc
File device/hid/hid_service.cc (left):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service.c...
device/hid/hid_service.cc:74: DCHECK(thread_checker_.CalledOnValidThread());
Why do we want to remove this one?

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
File device/hid/hid_service_linux.cc (right):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
device/hid/hid_service_linux.cc:125: if (str_property != NULL) {
nit: could you nullptr these while you're here?

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
File device/hid/hid_service_linux.h (right):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
device/hid/hid_service_linux.h:37: // These functions implement the process of
locating, requesting access to and
nit: I would maybe indent this with the inner class scope rather than the
preprocessor - though now that I say that, I'm guessing clang_format is
responsible for this. Ugh.

[Reilly Grant (use Gerrit), 2014-12-03 23:07:52]

Patchset #3 (id:60001) has been deleted

[Reilly Grant (use Gerrit), 2014-12-03 23:10:28]

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_connectio...
File device/hid/hid_connection_linux.cc (right):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_connectio...
device/hid/hid_connection_linux.cc:35: public base::NonThreadSafe {
On 2014/12/03 22:35:09, Ken Rockot wrote:
> nit: I think you can compose a base::ThreadChecker in this case, which would
be
> preferred according to the conditions outlined in
> base/threading/thread_checker.h

Done.

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service.cc
File device/hid/hid_service.cc (left):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service.c...
device/hid/hid_service.cc:74: DCHECK(thread_checker_.CalledOnValidThread());
On 2014/12/03 22:35:10, Ken Rockot wrote:
> Why do we want to remove this one?

I mistakenly recalled that base::ThreadChecker has this DCHECK in its own
destructor. This is only the case for base::NonThreadSafe. The DCHECK in the
HidService constructor is truly redundant.

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
File device/hid/hid_service_linux.cc (right):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
device/hid/hid_service_linux.cc:125: if (str_property != NULL) {
On 2014/12/03 22:35:10, Ken Rockot wrote:
> nit: could you nullptr these while you're here?

Technically this is a C API and so NULL is correct. Maybe?

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
File device/hid/hid_service_linux.h (right):

https://codereview.chromium.org/771393002/diff/40001/device/hid/hid_service_l...
device/hid/hid_service_linux.h:37: // These functions implement the process of
locating, requesting access to and
On 2014/12/03 22:35:10, Ken Rockot wrote:
> nit: I would maybe indent this with the inner class scope rather than the
> preprocessor - though now that I say that, I'm guessing clang_format is
> responsible for this. Ugh.

Done.

[Reilly Grant (use Gerrit), 2014-12-03 23:23:07]

reillyg@chromium.org changed reviewers:
+ thestig@chromium.org

[Reilly Grant (use Gerrit), 2014-12-03 23:23:08]

thestig, please take a look at the chrome_device_client.cc update.

[Lei Zhang, 2014-12-03 23:29:39]

lgtm

[Reilly Grant (use Gerrit), 2014-12-03 23:38:16]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2014-12-03 23:39:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/771393002/80001

[commit-bot: I haz the power, 2014-12-04 00:26:37]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2014-12-04 00:27:54]

Patchset 3 (id:??) landed as
https://crrev.com/56868deb02c83ade521fa482e2d03b9319162ae9
Cr-Commit-Position: refs/heads/master@{#306729}