Port device_sensors to be hosted in Device Service

Port device_sensors to be hosted in Device Service

This patch moves device_sensors to be hosted by the Device Service.
Before this patch, device_sensors were registered in RenderProcessHostImpl, the
function DeviceSensorHost::Create() which makes StrongBinding is run on IO
Thread(if Android platform) or UI thread(if other platforms).

In this patch, we add a member |io_task_runner_| in DeviceService and use
PostTask() to make sure the StrongBindings still happen in same threads as
before.

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2692993006
Cr-Commit-Position: refs/heads/master@{#459015}
Committed: https://chromium.googlesource.com/chromium/src/+/57f4387cd58afa6949c3fa158271d49690ca8d81

[ke.he, 2017-02-15 03:38:18]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 03:39:07]

Dry run: 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-02-15 03:44:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 03:44:45]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[ke.he, 2017-02-15 07:45:13]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 07:45:26]

Dry run: 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-02-15 07:47:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 07:47:02]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ke.he, 2017-02-15 08:14:29]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 08:14:41]

Dry run: 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-02-15 08:17:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 08:17:03]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[ke.he, 2017-02-15 08:52:39]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 08:52:56]

Dry run: 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-02-15 08:55:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 08:55:36]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ke.he, 2017-02-15 09:07:18]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 09:07:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-02-15 09:08:05]

Description was changed from

==========
Port device_sensors to be hosted in Device Service

BUG=686706
==========

to

==========
Port device_sensors to be hosted in Device Service

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
==========

[commit-bot: I haz the power, 2017-02-15 09:50:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 09:50:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ke.he, 2017-02-15 12:10:29]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 12:10:45]

Dry run: 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-02-15 12:38:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 12:38:58]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[ke.he, 2017-02-15 12:49:23]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 12:49:57]

Dry run: 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-02-15 14:29:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 14:29:17]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-02-15 14:40:44]

ke.he@intel.com changed reviewers:
+ blundell@chromium.org, rockot@chromium.org

[ke.he, 2017-02-15 14:40:45]

Hi, Ken, Could you PTAL on this? Thanks!

[ke.he, 2017-02-17 13:46:34]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 13:46:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-02-17 15:05:49]

Description was changed from

==========
Port device_sensors to be hosted in Device Service

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
==========

to

==========
Port device_sensors to be hosted in Device Service

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[ke.he, 2017-02-17 15:07:12]

Patchset #8 (id:140001) has been deleted

[blundell, 2017-02-20 15:45:31]

https://codereview.chromium.org/2692993006/diff/120001/services/device/device...
File services/device/device_service.cc (right):

https://codereview.chromium.org/2692993006/diff/120001/services/device/device...
services/device/device_service.cc:84: io_task_runner_->PostTask(FROM_HERE,
base::Bind(&DeviceMotionHost::Create,
Could you add an explanation in the CL description of how this new threading /
interface registration structure preserves the behavior that was present in
//content?

[ke.he, 2017-02-21 02:37:38]

Description was changed from

==========
Port device_sensors to be hosted in Device Service

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Port device_sensors to be hosted in Device Service

This patch moves device_sensors to be hosted by the Device Service.
Before this patch, device_sensors were registered in RenderProcessHostImpl, the
function DeviceSensorHost::Create() which makes StrongBinding is run on IO
Thread(if Android platform) or UI thread(if other platforms).

In this patch, we add a member |io_task_runner_| in DeviceService and use
PostTask() to make sure the StrongBindings still happen in same threads as
before.

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[ke.he, 2017-02-21 02:38:33]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 02:38:49]

Dry run: 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-02-21 04:34:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-21 04:34:27]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[ke.he, 2017-02-21 08:52:29]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 08:53:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-02-21 09:34:04]

After CL description updated, a new buildbot "linux_site_isolation" was
triggered and failed
(layout-test:
http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html")

Its Javascript code did call the testRunner.setMockDeviceMotion(). While my
change is to replace "InterfaceRegister and InterfaceProvider" mojo-pair into
"Device-Service and Device-Service-connector" mojo-pair, we didn't change
anything about the setMockDeviceMotion(), there is no Motion and Orientaion mojo
implementations in Javascript code either.

So strange the layouttest is broken:(

https://codereview.chromium.org/2692993006/diff/120001/services/device/device...
File services/device/device_service.cc (right):

https://codereview.chromium.org/2692993006/diff/120001/services/device/device...
services/device/device_service.cc:84: io_task_runner_->PostTask(FROM_HERE,
base::Bind(&DeviceMotionHost::Create,
On 2017/02/20 15:45:31, blundell wrote:
> Could you add an explanation in the CL description of how this new threading /
> interface registration structure preserves the behavior that was present in
> //content?

Done.

[commit-bot: I haz the power, 2017-02-21 10:48:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-21 10:48:12]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[leonhsl(Using Gerrit), 2017-02-22 03:02:13]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 03:04:38]

Dry run: 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-02-22 07:34:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 07:34:58]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[blundell, 2017-02-22 16:30:24]

https://codereview.chromium.org/2692993006/diff/160001/content/renderer/devic...
File content/renderer/device_sensors/device_sensor_event_pump.h (left):

https://codereview.chromium.org/2692993006/diff/160001/content/renderer/devic...
content/renderer/device_sensors/device_sensor_event_pump.h:27:
mojo::InterfaceRequest<MojoInterface> request(&mojo_interface_);
The problem is here. You're now not binding |mojo_interface_| if lines 38-41
aren't executed (which they aren't in layout tests). However, |mojo_interface_|
is later being dereferenced, which causes the renderer to crash. An easy fix is
to just preserve the behavior of binding |mojo_interface_| unconditionally, then
either sending the other end of the pipe to the impl or not.

[ke.he, 2017-02-23 03:11:10]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 03:11:44]

Dry run: 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-02-23 04:24:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 04:24:32]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-02-23 05:49:10]

On 2017/02/22 16:30:24, blundell wrote:
>
https://codereview.chromium.org/2692993006/diff/160001/content/renderer/devic...
> File content/renderer/device_sensors/device_sensor_event_pump.h (left):
> 
>
https://codereview.chromium.org/2692993006/diff/160001/content/renderer/devic...
> content/renderer/device_sensors/device_sensor_event_pump.h:27:
> mojo::InterfaceRequest<MojoInterface> request(&mojo_interface_);
> The problem is here. You're now not binding |mojo_interface_| if lines 38-41
> aren't executed (which they aren't in layout tests). However,
|mojo_interface_|
> is later being dereferenced, which causes the renderer to crash. An easy fix
is
> to just preserve the behavior of binding |mojo_interface_| unconditionally,
then
> either sending the other end of the pipe to the impl or not.

Ohhh! I'm sorry I paid all my attention on the layouttest but didn't notice the
dereference!
Thank you! 
PTAL.

[blundell, 2017-02-23 14:31:15]

Hi,

The code looks fine but there's a problematic reference to DeviceSensorService
in //content:

https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.cc?q=D...

That call occurs after the Device Service is destroyed, so it's not a no-op to
move it into the Device Service destructor. Moreover, the DeviceSensorService
runs on the IO thread on platforms other than Android and isn't thread-safe.
I've seen this in other features as well and at this point I'm not sure how to
solve it. I'm going to send an email to services-dev@ about the issue. I'd like
to remove that dependency before/as part of this CL, so let's hold off to solve
that issue.

[blundell, 2017-03-03 20:25:03]

On 2017/02/23 14:31:15, blundell wrote:
> Hi,
> 
> The code looks fine but there's a problematic reference to DeviceSensorService
> in //content:
> 
>
https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.cc?q=D...
> 
> That call occurs after the Device Service is destroyed, so it's not a no-op to
> move it into the Device Service destructor. Moreover, the DeviceSensorService
> runs on the IO thread on platforms other than Android and isn't thread-safe.
> I've seen this in other features as well and at this point I'm not sure how to
> solve it. I'm going to send an email to services-dev@ about the issue. I'd
like
> to remove that dependency before/as part of this CL, so let's hold off to
solve
> that issue.

Hi Ke He,

Could you try out the approach described in this thread to solve this problem:

https://groups.google.com/a/chromium.org/forum/#!topic/services-dev/Y9FKZf9n1ls?
I would do that as a separate CL first and then return to this one once we solve
that problem. Please also file a separate bug for that issue (similar to
crbug.com/695008), and block crbug.com/686706 on it.

Thanks,

Colin

[ke.he, 2017-03-06 03:11:23]

On 2017/03/03 20:25:03, blundell wrote:
> On 2017/02/23 14:31:15, blundell wrote:
> > Hi,
> > 
> > The code looks fine but there's a problematic reference to
DeviceSensorService
> > in //content:
> > 
> >
>
https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.cc?q=D...
> > 
> > That call occurs after the Device Service is destroyed, so it's not a no-op
to
> > move it into the Device Service destructor. Moreover, the
DeviceSensorService
> > runs on the IO thread on platforms other than Android and isn't thread-safe.
> > I've seen this in other features as well and at this point I'm not sure how
to
> > solve it. I'm going to send an email to services-dev@ about the issue. I'd
> like
> > to remove that dependency before/as part of this CL, so let's hold off to
> solve
> > that issue.
> 
> Hi Ke He,
> 
> Could you try out the approach described in this thread to solve this problem:
> 
>
https://groups.google.com/a/chromium.org/forum/#!topic/services-dev/Y9FKZf9n1ls?
> I would do that as a separate CL first and then return to this one once we
solve
> that problem. Please also file a separate bug for that issue (similar to
> crbug.com/695008), and block crbug.com/686706 on it.
> 
> Thanks,
> 
> Colin

Sure, Thank you!

[ke.he, 2017-03-17 03:53:04]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 03:53:37]

Dry run: 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-03-17 04:51:03]

The CQ bit was unchecked by commit-bot@chromium.org

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

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-03-17 05:18:43]

Hi,Colin, PTAL. thanks:)

[blundell, 2017-03-17 12:40:13]

Hi Ke He,

Can we now remove all deps from //content to //device/sensors and
//device/sensors:java as part of this CL? If not, we still need to fix the
blockers first :).

[ke.he, 2017-03-17 16:10:46]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 16:11:19]

Dry run: 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-03-17 16:16:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 16:16:58]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ke.he, 2017-03-17 16:22:19]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 16:22:39]

Dry run: 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-03-17 16:27:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 16:27:42]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[ke.he, 2017-03-17 16:31:09]

Patchset #11 (id:220001) has been deleted

[ke.he, 2017-03-17 16:33:27]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 16:33:47]

Dry run: 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-03-17 18:13:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 18:13:37]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ke.he, 2017-03-17 20:48:27]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 20:49:17]

Dry run: 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-03-17 22:26:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 22:26:29]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ke.he, 2017-03-18 02:20:20]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-18 02:20:54]

Dry run: 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-03-18 04:00:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-18 04:00:27]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-03-18 05:02:28]

there are 4 deps from //content to //device/sensor:
1)The deps from //content/browser to //device/sensor is redundant, I have
deleted it.

2)The deps from //content/test to //device/sensor is because of
browsertest(there is a separate bug 694888 tracking it). we discussed on it
before, the solution might be : "the browsertest needs to be ported to a service
test that lives in the Device Service itself (i.e., the usage of the renderer
would be replaced by a test "device sensor client" service that the test can
connect to and drive.)"

3)The deps from //content/public/android to //device/sensor:java, I feel we
should do it after moving codes from //device/sensor to //service/device, then
just let //content/public/android depends on //service/device:java instead.

4)The deps from //content/renderer. It is a really blocker while it is
orthogonal with this CL, See bug 676328.

Any comments on the next step? Thanks very much:)

[blundell, 2017-03-20 08:17:54]

Thanks, Ke He!

On 2017/03/18 05:02:28, ke.he wrote:
> there are 4 deps from //content to //device/sensor:
> 1)The deps from //content/browser to //device/sensor is redundant, I have
> deleted it.

Awesome.

> 
> 2)The deps from //content/test to //device/sensor is because of
> browsertest(there is a separate bug 694888 tracking it). we discussed on it
> before, the solution might be : "the browsertest needs to be ported to a
service
> test that lives in the Device Service itself (i.e., the usage of the renderer
> would be replaced by a test "device sensor client" service that the test can
> connect to and drive.)"

Yeah, we can do this as a followup.

> 
> 3)The deps from //content/public/android to //device/sensor:java, I feel we
> should do it after moving codes from //device/sensor to //service/device, then
> just let //content/public/android depends on //service/device:java instead.
>

The key question is: Does //content actually use the Device Sensors Java code,
or is the dep only for packaging that code? If it's the latter, I agree with
you. If there's an actual code dependency we need to solve that first.
 
> 4)The deps from //content/renderer. It is a really blocker while it is
> orthogonal with this CL, See bug 676328.
>

Agreed, that's a blocker to Blink Onion Soup but not servicification.

> Any comments on the next step? Thanks very much:)

[ke.he, 2017-03-21 07:24:46]

>>  3)The deps from //content/public/android to //device/sensor:java, I feel we
>>  should do it after moving codes from //device/sensor to //service/device,
then
>>  just let //content/public/android depends on //service/device:java instead.

> The key question is: Does //content actually use the Device Sensors Java code,
> or is the dep only for packaging that code? If it's the latter, I agree with
> you. If there's an actual code dependency we need to solve that first.
 
Thank you Colin!
The deps is only for packaging the class binary into final apk. There is no
reference to DeviceSensor.java from //content/*.java. To verify this I removed
the "device/sensor:java" deps out of "//content/public/android:content_java" and
run "ninja -C out/android_build content_java". It turns out the content_java.jar
can still be generated successfully. so I think content_java doesn't actually
use the Device Sensor Java code. 
And,the device/sensor:javatest is in the same situation. It is also just for
final apk packaging.

While I find a real deps(sorry I missed it last time) the //content/app/BUILD.gn
depends on //device/sensor. In file content/app/android/library_loader_hooks.cc:
EnsureJniRegistered(JNIEnv* env)
{  ...
   if (!device::android::RegisterDeviceSensorJni(env))
   ...
}
It is easy to decouple this, just move the RegisterDeviceSensorJni() into
services/device/android/register_jni.cc, very similar as
RegisterTimeZoneMonitorJni().
I think we should do it when moving codes from //device/sensor to
//service/device.

Maybe we can land this CL(if everything of this CL is correct of course) before
start the next step? Thanks very much!

[blundell, 2017-03-21 10:58:31]

lgtm, thanks for the investigations and explanations!

[ke.he, 2017-03-21 11:31:48]

On 2017/03/21 10:58:31, blundell wrote:
> lgtm, thanks for the investigations and explanations!

Hi, Colin, thank you!
By the way, Can I start to move the files from //device/sensors into
//service/device now? Or we have to resolve below blocker first? I'm not sure
whether it is allowed to temporarily introducing the deps from
//content/renderer to //service/device.
>> 4)The deps from //content/renderer. It is a really blocker while it is
>> orthogonal with this CL, See bug 676328.
>>
>Agreed, that's a blocker to Blink Onion Soup but not servicification.

Hi, Jam@, Could you PTAL on the changes in //content? Thanks!

[ke.he, 2017-03-21 11:32:08]

ke.he@intel.com changed reviewers:
+ jam@chromium.org

[ke.he, 2017-03-21 11:33:36]

Hi, Jam@, Could you PTAL on the changes in //content? Thanks!

[blundell, 2017-03-21 11:52:48]

On 2017/03/21 11:31:48, ke.he wrote:
> On 2017/03/21 10:58:31, blundell wrote:
> > lgtm, thanks for the investigations and explanations!
> 
> Hi, Colin, thank you!
> By the way, Can I start to move the files from //device/sensors into
> //service/device now? Or we have to resolve below blocker first? I'm not sure
> whether it is allowed to temporarily introducing the deps from
> //content/renderer to //service/device.

The Blink Onion Soup issue is completely orthogonal to servicification; feel
free to go ahead there :).

> >> 4)The deps from //content/renderer. It is a really blocker while it is
> >> orthogonal with this CL, See bug 676328.
> >>
> >Agreed, that's a blocker to Blink Onion Soup but not servicification.
> 
> Hi, Jam@, Could you PTAL on the changes in //content? Thanks!

[jam, 2017-03-21 23:58:43]

lgtm

[ke.he, 2017-03-22 00:53:58]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-03-22 00:54:41]

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-03-22 01:10:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 01:10:31]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ke.he, 2017-03-22 02:08:54]

ke.he@intel.com changed reviewers:
+ tsepez@chromium.org

[ke.he, 2017-03-22 02:08:55]

Hi, Tom, Could you PTAL on
"content/public/app/mojo/content_browser_manifest.json" and
"content/public/app/mojo/content_renderer_manifest.json" for the security
review?
Thanks!

[Tom Sepez, 2017-03-22 17:13:14]

lgtm

[ke.he, 2017-03-23 02:12:45]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-03-23 02:13:09]

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-03-23 02:16:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 02:16:49]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ke.he, 2017-03-23 02:43:33]

The CQ bit was checked by ke.he@intel.com

[ke.he, 2017-03-23 02:43:34]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2692993006/#ps280001
(title: "code rebase")

[commit-bot: I haz the power, 2017-03-23 02:43:57]

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-03-23 03:00:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 03:00:54]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[ke.he, 2017-03-23 03:10:39]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-03-23 03:10:52]

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-03-23 03:27:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 03:27:03]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[ke.he, 2017-03-23 05:20:59]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-03-23 05:21:16]

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-03-23 05:34:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 05:34:19]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[ke.he, 2017-03-23 06:13:33]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-03-23 06:13:42]

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-03-23 06:31:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 06:31:23]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[ke.he, 2017-03-23 07:26:53]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-03-23 07:27:09]

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-03-23 07:40:13]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1490254013839160,
"parent_rev": "15710b44fb12849ae9f165bafb934287a349cde6", "commit_rev":
"57f4387cd58afa6949c3fa158271d49690ca8d81"}

[commit-bot: I haz the power, 2017-03-23 07:41:47]

Description was changed from

==========
Port device_sensors to be hosted in Device Service

This patch moves device_sensors to be hosted by the Device Service.
Before this patch, device_sensors were registered in RenderProcessHostImpl, the
function DeviceSensorHost::Create() which makes StrongBinding is run on IO
Thread(if Android platform) or UI thread(if other platforms).

In this patch, we add a member |io_task_runner_| in DeviceService and use
PostTask() to make sure the StrongBindings still happen in same threads as
before.

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Port device_sensors to be hosted in Device Service

This patch moves device_sensors to be hosted by the Device Service.
Before this patch, device_sensors were registered in RenderProcessHostImpl, the
function DeviceSensorHost::Create() which makes StrongBinding is run on IO
Thread(if Android platform) or UI thread(if other platforms).

In this patch, we add a member |io_task_runner_| in DeviceService and use
PostTask() to make sure the StrongBindings still happen in same threads as
before.

TODO: The mojom and cpp library are still in //device/sensors, will move them
into //services/device later (when move //device/sensors/ implementations into
//service).

BUG=686706
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2692993006
Cr-Commit-Position: refs/heads/master@{#459015}
Committed:
https://chromium.googlesource.com/chromium/src/+/57f4387cd58afa6949c3fa158271...
==========

[commit-bot: I haz the power, 2017-03-23 07:41:49]

Committed patchset #13 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/57f4387cd58afa6949c3fa158271...