Move //content/browser/device_sensor/ into device/sensors

Move //content/browser/device_sensor/ into device/sensors

This patch moves device_sensor related files out of //content/browser and into
//device/sensors. In next patch we will port it into Device Service.

The file DeviceSensors.java and DeviceSensorsTest.java are also moved into
//device. The device_sensor_browsertest.cc is still in
//content/browser/device_sensor/

BUG=612322
Review-Url: https://codereview.chromium.org/2646093002
Cr-Commit-Position: refs/heads/master@{#447770}
Committed: https://chromium.googlesource.com/chromium/src/+/48ce2ae57a453067012e318d07a44bafe6c752b1

[ke.he, 2017-01-20 08:33:15]

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

[commit-bot: I haz the power, 2017-01-20 08:33:35]

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-01-20 08:37:50]

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

[commit-bot: I haz the power, 2017-01-20 08:37:50]

Dry run: Try jobs failed on following builders:
  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_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  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-01-22 09:58:44]

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

[commit-bot: I haz the power, 2017-01-22 09:58:50]

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-01-22 10:06:35]

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

[commit-bot: I haz the power, 2017-01-22 10:06:37]

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-01-23 09:43:47]

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

[commit-bot: I haz the power, 2017-01-23 09:43:59]

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-01-23 09:48:08]

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

[commit-bot: I haz the power, 2017-01-23 09:48:08]

Dry run: Try jobs failed on following builders:
  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_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ke.he, 2017-01-23 13:53:28]

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

[commit-bot: I haz the power, 2017-01-23 13:53:43]

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-01-23 13:54:37]

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

[commit-bot: I haz the power, 2017-01-23 13:54:38]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[ke.he, 2017-01-24 06:29:30]

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

[commit-bot: I haz the power, 2017-01-24 06:29:51]

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-01-24 06:36:52]

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

[commit-bot: I haz the power, 2017-01-24 06:36:53]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[ke.he, 2017-01-25 14:40:53]

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

[commit-bot: I haz the power, 2017-01-25 14:41:03]

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-01-25 14:48:34]

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

[commit-bot: I haz the power, 2017-01-25 14:48:37]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[ke.he, 2017-01-26 01:00:18]

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

[commit-bot: I haz the power, 2017-01-26 01:01:14]

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-01-26 01:43:11]

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

[commit-bot: I haz the power, 2017-01-26 01:43:12]

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

[ke.he, 2017-01-26 02:25:20]

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

[commit-bot: I haz the power, 2017-01-26 02:26: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-01-26 03:08:01]

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

[commit-bot: I haz the power, 2017-01-26 03:08:02]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[ke.he, 2017-01-26 07:58:57]

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

[commit-bot: I haz the power, 2017-01-26 07:59:12]

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-01-26 10:30:37]

Patchset #11 (id:200001) has been deleted

[ke.he, 2017-01-26 11:14:20]

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

[commit-bot: I haz the power, 2017-01-26 11:14: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-01-26 12:45:39]

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

[commit-bot: I haz the power, 2017-01-26 12:45:40]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-01-26 13:02:20]

Description was changed from

==========
Move //content/browser/device_sensor/ into device/sensors

Only browser_test files are left alone.

BUG=612322
==========

to

==========
Move //content/browser/device_sensor/ into device/sensors

This patch moves device_sensor related files out of //content/browser and into
//device/sensors. In next patch we will port it into Device Service.

The device_sensor_browsertest.cc is still in //content/browser/device_sensor/

BUG=612322
==========

[ke.he, 2017-01-26 13:04:09]

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

[commit-bot: I haz the power, 2017-01-26 13:04:20]

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-01-26 13:06:31]

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

[ke.he, 2017-01-26 13:06:33]

Hi, Colin, PTAL on this, Thanks:)

[timvolodine, 2017-01-26 14:01:14]

timvolodine@chromium.org changed reviewers:
+ timvolodine@chromium.org

[timvolodine, 2017-01-26 14:01:16]

shouldn't the //content/browser/device_sensor/ go into device/sensors/browser?

also, is the reformatting in most files due to git cl format?

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/DEPS
File device/sensors/DEPS (right):

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/DEPS#ne...
device/sensors/DEPS:4: "+ui/gfx",
is this needed?

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
File
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java
(right):

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java:73:
public static final Set<Integer> DEVICE_ORIENTATION_SENSORS_A =
why does this have to be public? also below and the VisibleForTesting fields and
methods..

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java:370:
values[1] = Math.asin(matrixR[7]); // beta (-pi/2, pi/2)
the alignment here was actually done on purpose for better readability, so
please keep it

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/device_...
File device/sensors/device_sensor_export.h (right):

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/device_...
device/sensors/device_sensor_export.h:1: // Copyright (c) 2016 The Chromium
Authors. All rights reserved.
don't think we use "(c)" anymore and 2016 should probably be 2017?

[commit-bot: I haz the power, 2017-01-26 14:26:11]

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

[commit-bot: I haz the power, 2017-01-26 14:26:13]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-01-26 15:33:22]

Hi, timvolodine@, thank you!
Yes the format was changed by "git cl format" command.

I moved those files into device/sensors instead of device/sensors/browser, the
reason is: in my understanding, the final purpose is to move the device_sensor
into device service. Although they are run in browser process and currently the
device service is also run in browser process, but in long term it might be much
flexible to decide which process the device service would be in. And, I also
found other modules in device don't have the browser folder either. what do you
think?

I'll update the CL based on your comments. Thanks again:)

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/DEPS
File device/sensors/DEPS (right):

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/DEPS#ne...
device/sensors/DEPS:4: "+ui/gfx",
On 2017/01/26 14:01:15, timvolodine wrote:
> is this needed?

Seems not needed. I'll have a double check tomorrow.
So the gfx deps in device/sensors/Build.gn might be removed too.
Thanks pointing out this:)

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
File
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java
(right):

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java:73:
public static final Set<Integer> DEVICE_ORIENTATION_SENSORS_A =
On 2017/01/26 14:01:15, timvolodine wrote:
> why does this have to be public? also below and the VisibleForTesting fields
and
> methods..

With this patch, the package name of this class is changed from
"org.chromium.content.browser" to "org.chromium.device.sensors".
While the DeviceSensorsTest.java are still in org.chromium.content.browser
package. So in this cross package situation I have to
make DeviceSensors to be public to let DeviceSensorsTest be able to find it.

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java:370:
values[1] = Math.asin(matrixR[7]); // beta (-pi/2, pi/2)
On 2017/01/26 14:01:15, timvolodine wrote:
> the alignment here was actually done on purpose for better readability, so
> please keep it

The format is changed by "git cl format", I'll keep this part as original, and
let "git cl format" to format other parts,
do you think it's ok?

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/device_...
File device/sensors/device_sensor_export.h (right):

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/device_...
device/sensors/device_sensor_export.h:1: // Copyright (c) 2016 The Chromium
Authors. All rights reserved.
On 2017/01/26 14:01:15, timvolodine wrote:
> don't think we use "(c)" anymore and 2016 should probably be 2017?

Yes. Thanks for pointing out this.

[ke.he, 2017-01-27 06:14:46]

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

[commit-bot: I haz the power, 2017-01-27 06:15:02]

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-01-27 07:08:58]

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

[commit-bot: I haz the power, 2017-01-27 07:09:00]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-01-27 07:22:01]

timvolodine@, CL updated, PTAL.
In sensor_manager_chromeos.cc it depends on ui/gfx.
Thanks!

[blundell, 2017-01-27 14:25:13]

Tim: It is not necessary to put the code under a browser/ subdir in //device
because, as Ke He notes, it will no longer be browser process-specific code as
such.

Ke He: Could you separate the formatting changes from the code move? i.e., do
them as a separate CL either before or after (using clang format directly).
Thanks!

[blundell, 2017-01-27 14:25:14]

Tim: It is not necessary to put the code under a browser/ subdir in //device
because, as Ke He notes, it will no longer be browser process-specific code as
such.

Ke He: Could you separate the formatting changes from the code move? i.e., do
them as a separate CL either before or after (using clang format directly).
Thanks!

[ke.he, 2017-01-28 03:00:13]

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

[commit-bot: I haz the power, 2017-01-28 03:00:33]

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-01-28 03:06:36]

Hi, Colin, Hi, Tim,
I have reverted the format changes. I'll submit another CL for formatting as
Colin suggested.
CL updated, PTAL. Thanks!

[commit-bot: I haz the power, 2017-01-28 03:57:14]

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

[commit-bot: I haz the power, 2017-01-28 03:57:15]

Dry run: This issue passed the CQ dry run.

[blundell, 2017-01-30 14:06:05]

lgtm, thanks!

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
File device/sensors/data_fetcher_shared_memory_mac.cc (right):

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
device/sensors/data_fetcher_shared_memory_mac.cc:20: void
FetchLight(device::AmbientLightSensor* sensor,
nit: just put the tests in the device namespace.

[ke.he, 2017-01-30 15:08:53]

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

[ke.he, 2017-01-30 15:08:56]

Hi, Colin, thanks!
Hi, Tim, Could you PTAL on the whole CL.
jam@, Could you PTAL on the changes in //content? Thanks all!

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
File device/sensors/data_fetcher_shared_memory_mac.cc (right):

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
device/sensors/data_fetcher_shared_memory_mac.cc:20: void
FetchLight(device::AmbientLightSensor* sensor,
On 2017/01/30 14:06:04, blundell wrote:
> nit: just put the tests in the device namespace.

The fetchLight() function(s) here is not for test. Do you mean to move them into
device namespace?

[blundell, 2017-01-30 15:12:35]

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
File device/sensors/data_fetcher_shared_memory_mac.cc (right):

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
device/sensors/data_fetcher_shared_memory_mac.cc:20: void
FetchLight(device::AmbientLightSensor* sensor,
On 2017/01/30 15:08:56, ke.he wrote:
> On 2017/01/30 14:06:04, blundell wrote:
> > nit: just put the tests in the device namespace.
> 
> The fetchLight() function(s) here is not for test. Do you mean to move them
into
> device namespace?

Oops, I misread. Yes, just move the namespace device { opener up before line 16.

[ke.he, 2017-01-30 15:24:37]

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

[commit-bot: I haz the power, 2017-01-30 15:24:55]

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-01-30 15:26:22]

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
File device/sensors/data_fetcher_shared_memory_mac.cc (right):

https://codereview.chromium.org/2646093002/diff/300001/device/sensors/data_fe...
device/sensors/data_fetcher_shared_memory_mac.cc:20: void
FetchLight(device::AmbientLightSensor* sensor,
On 2017/01/30 15:12:35, blundell wrote:
> On 2017/01/30 15:08:56, ke.he wrote:
> > On 2017/01/30 14:06:04, blundell wrote:
> > > nit: just put the tests in the device namespace.
> > 
> > The fetchLight() function(s) here is not for test. Do you mean to move them
> into
> > device namespace?
> 
> Oops, I misread. Yes, just move the namespace device { opener up before line
16.

Thanks, Done:)

[commit-bot: I haz the power, 2017-01-30 17:15:18]

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

[commit-bot: I haz the power, 2017-01-30 17:15:21]

Dry run: This issue passed the CQ dry run.

[jam, 2017-01-31 06:37:45]

content other than device_sensor directories lgtm (for those subdirs I defer to
Tim

[timvolodine, 2017-01-31 07:13:15]

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
File
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java
(right):

https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java:73:
public static final Set<Integer> DEVICE_ORIENTATION_SENSORS_A =
On 2017/01/26 15:33:22, ke.he wrote:
> On 2017/01/26 14:01:15, timvolodine wrote:
> > why does this have to be public? also below and the VisibleForTesting fields
> and
> > methods..
> 
> With this patch, the package name of this class is changed from
> "org.chromium.content.browser" to "org.chromium.device.sensors".
> While the DeviceSensorsTest.java are still in org.chromium.content.browser
> package. So in this cross package situation I have to
> make DeviceSensors to be public to let DeviceSensorsTest be able to find it.

I assume this is a temporary situation? Is it possible to also move the test to
the new package to avoid publics and the cross package divide?

[timvolodine, 2017-01-31 07:18:58]

On 2017/01/27 14:25:14, blundell wrote:
> Tim: It is not necessary to put the code under a browser/ subdir in //device
> because, as Ke He notes, it will no longer be browser process-specific code as
> such.
>

I was wondering how this would look like as we also have code in common/ and
renderer/ for device_sensors, so there is a logical separation of what is meant
to run where.. I was assuming those pieces would also move at some point, but
not sure if that's the current plan though..
 
> Ke He: Could you separate the formatting changes from the code move? i.e., do
> them as a separate CL either before or after (using clang format directly).
> Thanks!

[blundell, 2017-01-31 11:45:48]

On 2017/01/31 07:13:15, timvolodine wrote:
>
https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
> File
> device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java
> (right):
> 
>
https://codereview.chromium.org/2646093002/diff/240001/device/sensors/android...
>
device/sensors/android/java/src/org/chromium/device/sensors/DeviceSensors.java:73:
> public static final Set<Integer> DEVICE_ORIENTATION_SENSORS_A =
> On 2017/01/26 15:33:22, ke.he wrote:
> > On 2017/01/26 14:01:15, timvolodine wrote:
> > > why does this have to be public? also below and the VisibleForTesting
fields
> > and
> > > methods..
> > 
> > With this patch, the package name of this class is changed from
> > "org.chromium.content.browser" to "org.chromium.device.sensors".
> > While the DeviceSensorsTest.java are still in org.chromium.content.browser
> > package. So in this cross package situation I have to
> > make DeviceSensors to be public to let DeviceSensorsTest be able to find it.
> 
> I assume this is a temporary situation? Is it possible to also move the test
to
> the new package to avoid publics and the cross package divide?

+1, it looks like the test doesn't have any dependencies on //content so
hopefully this is doable in this patch.

[blundell, 2017-01-31 11:48:12]

On 2017/01/31 07:18:58, timvolodine wrote:
> On 2017/01/27 14:25:14, blundell wrote:
> > Tim: It is not necessary to put the code under a browser/ subdir in //device
> > because, as Ke He notes, it will no longer be browser process-specific code
as
> > such.
> >
> 
> I was wondering how this would look like as we also have code in common/ and
> renderer/ for device_sensors, so there is a logical separation of what is
meant
> to run where.. I was assuming those pieces would also move at some point, but
> not sure if that's the current plan though..

Ah, a great question. The renderer code will move into Blink per Blink Onion
Soup (so it will become the Blink client of the device sensors feature provided
by the Device Service). What code in //content/common are you speaking of?

>  
> > Ke He: Could you separate the formatting changes from the code move? i.e.,
do
> > them as a separate CL either before or after (using clang format directly).
> > Thanks!

[ke.he, 2017-02-01 12:54:37]

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

[commit-bot: I haz the power, 2017-02-01 12:54:53]

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-01 13:50:42]

Colin, Tim: DeviceSensorsTest.java has been moved into device. PTAL. Thanks:)

[commit-bot: I haz the power, 2017-02-01 13:56:35]

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

[commit-bot: I haz the power, 2017-02-01 13:56:36]

Dry run: This issue passed the CQ dry run.

[timvolodine, 2017-02-02 02:09:35]

On 2017/01/31 11:48:12, blundell wrote:
> On 2017/01/31 07:18:58, timvolodine wrote:
> > On 2017/01/27 14:25:14, blundell wrote:
> > > Tim: It is not necessary to put the code under a browser/ subdir in
//device
> > > because, as Ke He notes, it will no longer be browser process-specific
code
> as
> > > such.
> > >
> > 
> > I was wondering how this would look like as we also have code in common/ and
> > renderer/ for device_sensors, so there is a logical separation of what is
> meant
> > to run where.. I was assuming those pieces would also move at some point,
but
> > not sure if that's the current plan though..
> 
> Ah, a great question. The renderer code will move into Blink per Blink Onion
> Soup (so it will become the Blink client of the device sensors feature
provided
> by the Device Service). What code in //content/common are you speaking of?
>

I see, that makes sense. Looks like you already moved the common/ part some time
ago so no further concerns here. :)

[timvolodine, 2017-02-02 02:11:33]

On 2017/02/01 13:50:42, ke.he wrote:
> Colin, Tim: DeviceSensorsTest.java has been moved into device. PTAL. Thanks:)

great! thanks Ke He. lgtm.

[timvolodine, 2017-02-02 02:14:38]

On 2017/02/02 02:11:33, timvolodine wrote:
> On 2017/02/01 13:50:42, ke.he wrote:
> > Colin, Tim: DeviceSensorsTest.java has been moved into device. PTAL.
Thanks:)
> 
> great! thanks Ke He. lgtm.

hmm the original DeviceSensorsTest.java appears to be marked as "M" (instead of
"D") is there any reason for that?

also, maybe add to description that the corresponding java files i.e.
DeviceSensors.java and DeviceSensorsTest.java have also been moved.

[blundell, 2017-02-02 08:35:59]

still lgtm

On 2017/02/02 02:14:38, timvolodine wrote:
> On 2017/02/02 02:11:33, timvolodine wrote:
> > On 2017/02/01 13:50:42, ke.he wrote:
> > > Colin, Tim: DeviceSensorsTest.java has been moved into device. PTAL.
> Thanks:)
> > 
> > great! thanks Ke He. lgtm.
> 
> hmm the original DeviceSensorsTest.java appears to be marked as "M" (instead
of
> "D") is there any reason for that?
> 

This is just something that the UI does from time to time for whatever reason.
I've noticed this on other CLs.

> also, maybe add to description that the corresponding java files i.e.
> DeviceSensors.java and DeviceSensorsTest.java have also been moved.

[ke.he, 2017-02-02 12:36:31]

Description was changed from

==========
Move //content/browser/device_sensor/ into device/sensors

This patch moves device_sensor related files out of //content/browser and into
//device/sensors. In next patch we will port it into Device Service.

The device_sensor_browsertest.cc is still in //content/browser/device_sensor/

BUG=612322
==========

to

==========
Move //content/browser/device_sensor/ into device/sensors

This patch moves device_sensor related files out of //content/browser and into
//device/sensors. In next patch we will port it into Device Service.

The file DeviceSensors.java and DeviceSensorsTest.java are also moved into
//device. The device_sensor_browsertest.cc is still in
//content/browser/device_sensor/

BUG=612322
==========

[ke.he, 2017-02-02 12:41:05]

On 2017/02/02 08:35:59, blundell wrote:
> still lgtm
> 
> On 2017/02/02 02:14:38, timvolodine wrote:
> > On 2017/02/02 02:11:33, timvolodine wrote:
> > > On 2017/02/01 13:50:42, ke.he wrote:
> > > > Colin, Tim: DeviceSensorsTest.java has been moved into device. PTAL.
> > Thanks:)
> > > 
> > > great! thanks Ke He. lgtm.
> > 
> > hmm the original DeviceSensorsTest.java appears to be marked as "M" (instead
> of
> > "D") is there any reason for that?
> > 
> 
> This is just something that the UI does from time to time for whatever reason.
> I've noticed this on other CLs.
> 
> > also, maybe add to description that the corresponding java files i.e.
> > DeviceSensors.java and DeviceSensorsTest.java have also been moved.

Description added too. thanks all:)

[ke.he, 2017-02-02 12:41:54]

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

[ke.he, 2017-02-02 12:41:55]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2646093002/#ps340001
(title: "move devicesensortest.java into //device")

[commit-bot: I haz the power, 2017-02-02 12:42: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-02-02 12:55:25]

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

[commit-bot: I haz the power, 2017-02-02 12:55:27]

Failed to apply patch for content/browser/BUILD.gn:
While running git apply --index -p1;
  error: patch failed: content/browser/BUILD.gn:76
  error: content/browser/BUILD.gn: patch does not apply

Patch:       content/browser/BUILD.gn
Index: content/browser/BUILD.gn
diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn
index
826786abb707a986b1f58782d42d0b5e4a14d56e..d704a912c0be9610a33801c01d315348277e9f62
100644
--- a/content/browser/BUILD.gn
+++ b/content/browser/BUILD.gn
@@ -76,7 +76,7 @@ source_set("browser") {
     "//device/nfc:mojo_bindings",
     "//device/power_save_blocker",
     "//device/screen_orientation/public/interfaces",
-    "//device/sensors/public/cpp",
+    "//device/sensors",
     "//device/vibration",
     "//device/wake_lock",
     "//gin",
@@ -432,25 +432,6 @@ source_set("browser") {
     # needed on all platforms.
     "compositor/surface_utils.cc",
     "compositor/surface_utils.h",
-    "device_sensors/ambient_light_mac.cc",
-    "device_sensors/ambient_light_mac.h",
-    "device_sensors/data_fetcher_shared_memory.h",
-    "device_sensors/data_fetcher_shared_memory_android.cc",
-    "device_sensors/data_fetcher_shared_memory_base.cc",
-    "device_sensors/data_fetcher_shared_memory_base.h",
-    "device_sensors/data_fetcher_shared_memory_chromeos.cc",
-    "device_sensors/data_fetcher_shared_memory_default.cc",
-    "device_sensors/data_fetcher_shared_memory_mac.cc",
-    "device_sensors/data_fetcher_shared_memory_win.cc",
-    "device_sensors/device_sensor_host.cc",
-    "device_sensors/device_sensor_host.h",
-    "device_sensors/device_sensor_service.cc",
-    "device_sensors/device_sensor_service.h",
-    "device_sensors/device_sensors_consts.h",
-    "device_sensors/sensor_manager_android.cc",
-    "device_sensors/sensor_manager_android.h",
-    "device_sensors/sensor_manager_chromeos.cc",
-    "device_sensors/sensor_manager_chromeos.h",
     "devtools/browser_devtools_agent_host.cc",
     "devtools/browser_devtools_agent_host.h",
     "devtools/devtools_agent_host_impl.cc",
@@ -1554,7 +1535,6 @@ source_set("browser") {
   }
 
   if (is_win) {
-    sources -= [ "device_sensors/data_fetcher_shared_memory_default.cc" ]
     defines += [
       # This prevents the inclusion of atlhost.h which paired
       # with the windows 8 sdk it does the wrong thing.
@@ -1571,7 +1551,6 @@ source_set("browser") {
       "dxguid.lib",
       "imm32.lib",
       "oleacc.lib",
-      "sensorsapi.lib",
       "portabledeviceguids.lib",
     ]
   }
@@ -1795,7 +1774,6 @@ source_set("browser") {
     ]
     sources -= [
       "browser_ipc_logging.cc",
-      "device_sensors/data_fetcher_shared_memory_default.cc",
       "media/session/audio_focus_delegate_default.cc",
       "media/session/audio_focus_manager.cc",
       "media/session/audio_focus_manager.h",
@@ -1841,12 +1819,10 @@ source_set("browser") {
   }
 
   if (is_mac) {
-    sources -= [ "device_sensors/data_fetcher_shared_memory_default.cc" ]
     deps += [
       "//media",
       "//sandbox/mac:sandbox",
       "//third_party/mozilla",
-      "//third_party/sudden_motion_sensor",
       "//ui/accelerated_widget_mac",
     ]
     libs += [
@@ -1857,7 +1833,6 @@ source_set("browser") {
   }
 
   if (is_chromeos) {
-    sources -= [ "device_sensors/data_fetcher_shared_memory_default.cc" ]
     deps += [
       "//chromeos",
       "//chromeos:power_manager_proto",

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

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

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

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

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

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-02 13:11:14]

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

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

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-02 14:11:48]

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

[ke.he, 2017-02-02 14:11:50]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
timvolodine@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2646093002/#ps380001
(title: "gn format & code rebase")

[commit-bot: I haz the power, 2017-02-02 14:12:03]

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-02 15:31:37]

CQ is committing da patch.

Bot data: {"patchset_id": 380001, "attempt_start_ts": 1486044708928380,
"parent_rev": "5eded4b55c92335c96702d53aa319a360c86f556", "commit_rev":
"48ce2ae57a453067012e318d07a44bafe6c752b1"}

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

Description was changed from

==========
Move //content/browser/device_sensor/ into device/sensors

This patch moves device_sensor related files out of //content/browser and into
//device/sensors. In next patch we will port it into Device Service.

The file DeviceSensors.java and DeviceSensorsTest.java are also moved into
//device. The device_sensor_browsertest.cc is still in
//content/browser/device_sensor/

BUG=612322
==========

to

==========
Move //content/browser/device_sensor/ into device/sensors

This patch moves device_sensor related files out of //content/browser and into
//device/sensors. In next patch we will port it into Device Service.

The file DeviceSensors.java and DeviceSensorsTest.java are also moved into
//device. The device_sensor_browsertest.cc is still in
//content/browser/device_sensor/

BUG=612322

Review-Url: https://codereview.chromium.org/2646093002
Cr-Commit-Position: refs/heads/master@{#447770}
Committed:
https://chromium.googlesource.com/chromium/src/+/48ce2ae57a453067012e318d07a4...
==========

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

Committed patchset #19 (id:380001) as
https://chromium.googlesource.com/chromium/src/+/48ce2ae57a453067012e318d07a4...

[ortuno, 2017-02-03 20:51:48]

ortuno@chromium.org changed reviewers:
+ ortuno@chromium.org

[ortuno, 2017-02-03 20:51:50]

I might be wrong but I think this causes all of blink to be compiled when
compiling device_unittest. Is that intended?

[ke.he, 2017-02-04 05:57:06]

On 2017/02/03 20:51:50, ortuno wrote:
> I might be wrong but I think this causes all of blink to be compiled when
> compiling device_unittest. Is that intended?

ortuno@, thanks for pointing out this. I'll have a check.
I found it's my mistake that I missed to request the owner of the file
//device/BUILD.gn to have a review.
Really sorry for this.

[blundell, 2017-02-06 14:56:27]

On 2017/02/04 05:57:06, ke.he wrote:
> On 2017/02/03 20:51:50, ortuno wrote:
> > I might be wrong but I think this causes all of blink to be compiled when
> > compiling device_unittest. Is that intended?
> 
> ortuno@, thanks for pointing out this. I'll have a check.
> I found it's my mistake that I missed to request the owner of the file
> //device/BUILD.gn to have a review.
> Really sorry for this.

Good catch! Yes, this added dependency looks like it can just be removed, as
none of the new test code depends on Blink (and if it did, presumably it would
just be on the headers, so the existing dependency on blink_headers should be
enough). Ke He, can you take care of this? Thanks!

[ke.he, 2017-02-07 08:04:04]

On 2017/02/06 14:56:27, blundell wrote:
> On 2017/02/04 05:57:06, ke.he wrote:
> > On 2017/02/03 20:51:50, ortuno wrote:
> > > I might be wrong but I think this causes all of blink to be compiled when
> > > compiling device_unittest. Is that intended?
> > 
> > ortuno@, thanks for pointing out this. I'll have a check.
> > I found it's my mistake that I missed to request the owner of the file
> > //device/BUILD.gn to have a review.
> > Really sorry for this.
> 
> Good catch! Yes, this added dependency looks like it can just be removed, as
> none of the new test code depends on Blink (and if it did, presumably it would
> just be on the headers, so the existing dependency on blink_headers should be
> enough). Ke He, can you take care of this? Thanks!

Hi, ortuno@ and Colin:
Sorry to response late.
I found the device_unittests Does have dependency on Blink. The reason is:
The file "data_fetcher_shared_memory_base_unittest.cc" was moved from //content
into //device.
In that unittest file it has a member |motion_buffer_| which is type of
"DeviceMotionHardwareBuffer".
So it has dependency on "WebDeviceMotionData.cpp" which is included in target
"//third_party/WebKit/Source/platform:platform".
And, I found the original target content_unittests had dependency on the blink
too. So we have to
keep the blink dependency here.
Thanks very much!

[blundell, 2017-02-07 15:38:57]

OK, that seems correct to me after all. Can you add a TODO to remove this
dependency when https://bugs.chromium.org/p/chromium/issues/detail?id=686709 is
fixed?