Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2415083002: Move Device Sensors client files from Blink to //device/sensors client lib (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 2 weeks ago by blundell
Modified:
3 months, 1 week ago
CC:
blink-reviews, blink-reviews-api_chromium.org, blundell+watchlist_chromium.org, chromium-reviews, darin-cc_chromium.org, dglazkov+blink, droger+watchlist_chromium.org, einbinder+watch-test-runner_chromium.org, jochen+watch_chromium.org, mlamouri+watch-blink_chromium.org, mlamouri+watch-content_chromium.org, mlamouri+watch-sensors_chromium.org, mlamouri+watch-test-runner_chromium.org, Peter Beverloo, riju_, sdefresne+watchlist_chromium.org, timvolodine
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move Device Sensors client files from Blink to //device/sensors client lib Currently, several Device Sensors files are part of the WebKit public API. These files define POD structs that the Device Sensors implementation and its clients use to consistently interpret shared memory. As part of servicificying Device Sensors and enabling it to be used by clients other than Blink, this CL moves those files into the Device Sensors client library. It also updates naming conventions etc. to match their new location. Once concern in making this move is that Blink's modules implementations cannot in general depend on Chromium code due to performance concerns. In this particular case, the dependence is both necessary (for the interpretation of shared memory) and safe (since the structs are POD and must remain so). To avoid unwanted dependencies creeping in in the future, the moved files are isolated in their own target within //device/sensors/public/cpp, with a clear explanation in the BUILD.gn file of the constraints on this target. BUG=612322 TBR=jam Review-Url: https://codereview.chromium.org/2415083002 Cr-Commit-Position: refs/heads/master@{#458372} Committed: https://chromium.googlesource.com/chromium/src/+/241fad6fdac74b820fa21e29652bbae49239840e

Patch Set 1 : Revert licenses change for similarity #

Total comments: 1

Patch Set 2 : update licenses/header guards #

Patch Set 3 : Rebase #

Patch Set 4 : Fix gn check #

Total comments: 5

Patch Set 5 : Rebase #

Patch Set 6 : Really rebase #

Total comments: 2

Patch Set 7 : Rebase #

Patch Set 8 : Isolate moved files in their own target #

Patch Set 9 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+299 lines, -353 lines) Patch
M content/public/test/layouttest_support.h View 1 2 3 4 5 6 3 chunks +9 lines, -6 lines 0 comments Download
M content/renderer/BUILD.gn View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/device_sensors/device_motion_event_pump.h View 2 chunks +2 lines, -2 lines 0 comments Download
M content/renderer/device_sensors/device_motion_event_pump.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M content/renderer/device_sensors/device_motion_event_pump_unittest.cc View 1 2 3 4 5 6 5 chunks +6 lines, -8 lines 0 comments Download
M content/renderer/device_sensors/device_orientation_event_pump.h View 3 chunks +4 lines, -4 lines 0 comments Download
M content/renderer/device_sensors/device_orientation_event_pump.cc View 3 chunks +4 lines, -4 lines 0 comments Download
M content/renderer/device_sensors/device_orientation_event_pump_unittest.cc View 1 2 3 4 5 6 7 chunks +8 lines, -10 lines 0 comments Download
M content/renderer/renderer_blink_platform_impl.h View 1 2 3 4 5 6 3 chunks +9 lines, -7 lines 0 comments Download
M content/renderer/renderer_blink_platform_impl.cc View 1 2 3 4 5 6 7 8 3 chunks +5 lines, -5 lines 0 comments Download
M content/shell/BUILD.gn View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/shell/renderer/layout_test/blink_test_runner.h View 1 2 3 4 5 6 2 chunks +4 lines, -5 lines 0 comments Download
M content/shell/renderer/layout_test/blink_test_runner.cc View 1 2 3 4 5 6 2 chunks +4 lines, -5 lines 0 comments Download
M content/shell/test_runner/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M content/shell/test_runner/DEPS View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M content/shell/test_runner/test_runner.cc View 1 2 3 4 5 6 4 chunks +4 lines, -4 lines 0 comments Download
M content/shell/test_runner/test_runner_for_specific_view.cc View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M content/shell/test_runner/web_test_delegate.h View 1 2 3 4 5 6 3 chunks +7 lines, -4 lines 0 comments Download
M content/test/BUILD.gn View 1 2 3 4 5 6 7 8 3 chunks +3 lines, -2 lines 0 comments Download
M content/test/DEPS View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M content/test/layouttest_support.cc View 1 2 3 4 5 6 3 chunks +6 lines, -6 lines 0 comments Download
M device/BUILD.gn View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M device/generic_sensor/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M device/sensors/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M device/sensors/public/cpp/BUILD.gn View 1 2 3 4 5 6 7 2 chunks +30 lines, -2 lines 0 comments Download
D device/sensors/public/cpp/DEPS View 1 2 3 4 5 6 7 1 chunk +0 lines, -3 lines 0 comments Download
M device/sensors/public/cpp/device_motion_hardware_buffer.h View 1 2 3 4 5 6 1 chunk +2 lines, -3 lines 0 comments Download
M device/sensors/public/cpp/device_orientation_hardware_buffer.h View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 0 comments Download
A device/sensors/public/cpp/motion_data.h View 1 1 chunk +54 lines, -0 lines 0 comments Download
A device/sensors/public/cpp/motion_data.cc View 1 1 chunk +21 lines, -0 lines 0 comments Download
A device/sensors/public/cpp/orientation_data.h View 1 1 chunk +38 lines, -0 lines 0 comments Download
A device/sensors/public/cpp/orientation_data.cc View 1 1 chunk +19 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DEPS View 1 2 3 4 5 6 7 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceMotionData.h View 1 2 3 4 5 6 2 chunks +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceMotionData.cpp View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceMotionDispatcher.h View 2 chunks +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceMotionDispatcher.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceOrientationData.h View 1 2 3 4 5 6 2 chunks +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceOrientationData.cpp View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceOrientationDispatcher.h View 2 chunks +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/device_orientation/DeviceOrientationDispatcher.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/BUILD.gn View 1 2 3 4 5 6 1 chunk +0 lines, -2 lines 0 comments Download
D third_party/WebKit/Source/platform/exported/WebDeviceMotionData.cpp View 1 chunk +0 lines, -45 lines 0 comments Download
D third_party/WebKit/Source/platform/exported/WebDeviceOrientationData.cpp View 1 chunk +0 lines, -45 lines 0 comments Download
M third_party/WebKit/public/BUILD.gn View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -2 lines 0 comments Download
D third_party/WebKit/public/platform/modules/device_orientation/WebDeviceMotionData.h View 1 chunk +0 lines, -82 lines 0 comments Download
M third_party/WebKit/public/platform/modules/device_orientation/WebDeviceMotionListener.h View 1 chunk +5 lines, -3 lines 0 comments Download
D third_party/WebKit/public/platform/modules/device_orientation/WebDeviceOrientationData.h View 1 chunk +0 lines, -66 lines 0 comments Download
M third_party/WebKit/public/platform/modules/device_orientation/WebDeviceOrientationListener.h View 1 chunk +5 lines, -3 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 68 (42 generated)
blundell
Hi Tim, PS#1 shows the diff in the moved files (in the final patchset they ...
8 months, 2 weeks ago (2016-10-14 07:32:52 UTC) #16
blundell
ping :)
8 months, 1 week ago (2016-10-21 20:04:08 UTC) #17
timvolodine
two comments/questions below, otherwise looks fine to me. https://codereview.chromium.org/2415083002/diff/100001/device/sensors/public/cpp/motion_data.h File device/sensors/public/cpp/motion_data.h (right): https://codereview.chromium.org/2415083002/diff/100001/device/sensors/public/cpp/motion_data.h#newcode1 device/sensors/public/cpp/motion_data.h:1: // ...
8 months ago (2016-10-24 16:23:43 UTC) #18
blundell
https://codereview.chromium.org/2415083002/diff/100001/device/sensors/public/cpp/motion_data.h File device/sensors/public/cpp/motion_data.h (right): https://codereview.chromium.org/2415083002/diff/100001/device/sensors/public/cpp/motion_data.h#newcode1 device/sensors/public/cpp/motion_data.h:1: // Copyright 2013 The Chromium Authors. All rights reserved. ...
8 months ago (2016-10-24 16:25:59 UTC) #19
timvolodine
one more comment below, non-blocking so lgtm, thanks! https://codereview.chromium.org/2415083002/diff/100001/device/sensors/public/cpp/orientation_data.h File device/sensors/public/cpp/orientation_data.h (right): https://codereview.chromium.org/2415083002/diff/100001/device/sensors/public/cpp/orientation_data.h#newcode14 device/sensors/public/cpp/orientation_data.h:14: OrientationData(); ...
8 months ago (2016-10-24 18:13:20 UTC) #20
blundell
On 2016/10/24 18:13:20, timvolodine wrote: > one more comment below, non-blocking so lgtm, thanks! > ...
8 months ago (2016-10-26 15:34:52 UTC) #21
blundell
esprehn@: Could you review the removal of files from //third_party/WebKit/Source/platform/exported?
8 months ago (2016-10-26 15:37:18 UTC) #23
haraken
LGTM
8 months ago (2016-10-26 15:49:39 UTC) #24
blundell
Thanks, Kentaro! TBR=jam for the changes in //content outside of //content/*/device_sensors. These are just mechanical ...
8 months ago (2016-10-26 16:00:58 UTC) #26
commit-bot: I haz the power
This CL has an open dependency (Issue 2410123002 Patch 80001). Please resolve the dependency and ...
8 months ago (2016-10-26 16:01:54 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2415083002/140001
8 months ago (2016-10-26 16:18:46 UTC) #33
jam
https://codereview.chromium.org/2415083002/diff/140001/device/sensors/public/cpp/motion_data.h File device/sensors/public/cpp/motion_data.h (right): https://codereview.chromium.org/2415083002/diff/140001/device/sensors/public/cpp/motion_data.h#newcode18 device/sensors/public/cpp/motion_data.h:18: double accelerationX; this file should use chromium naming convention, ...
8 months ago (2016-10-26 17:13:41 UTC) #35
jam
oh and lgtm with these changes
8 months ago (2016-10-26 17:46:20 UTC) #36
esprehn
This is pulling in headers from device/sensors/public/cpp and not generated mojo ones, which isn't allowed ...
7 months, 3 weeks ago (2016-11-04 23:44:59 UTC) #38
blundell
On 2016/11/04 23:44:59, esprehn wrote: > This is pulling in headers from device/sensors/public/cpp and not ...
7 months, 3 weeks ago (2016-11-07 14:29:28 UTC) #39
blundell
On 2016/11/07 14:29:28, blundell wrote: > On 2016/11/04 23:44:59, esprehn wrote: > > This is ...
7 months, 3 weeks ago (2016-11-07 14:30:03 UTC) #40
blundell
Hi, Elliott, per the discussion on platform-architecture-dev@, I've isolated the moved files in their own ...
3 months, 1 week ago (2017-03-16 16:43:18 UTC) #44
jam
On 2017/03/16 16:43:18, blundell wrote: > Hi, > > Elliott, per the discussion on platform-architecture-dev@, ...
3 months, 1 week ago (2017-03-20 14:44:00 UTC) #49
blundell
Elliott: ping. On 2017/03/16 16:43:18, blundell wrote: > Hi, > > Elliott, per the discussion ...
3 months, 1 week ago (2017-03-20 15:21:57 UTC) #50
dglazkov
lgtm
3 months, 1 week ago (2017-03-20 22:44:32 UTC) #52
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2415083002/180001
3 months, 1 week ago (2017-03-21 08:18:02 UTC) #56
commit-bot: I haz the power
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/174068) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
3 months, 1 week ago (2017-03-21 08:20:38 UTC) #58
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2415083002/200001
3 months, 1 week ago (2017-03-21 08:43:06 UTC) #61
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/254260)
3 months, 1 week ago (2017-03-21 10:18:16 UTC) #63
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2415083002/200001
3 months, 1 week ago (2017-03-21 10:23:45 UTC) #65
commit-bot: I haz the power
3 months, 1 week ago (2017-03-21 11:15:27 UTC) #68
Message was sent while issue was closed.
Committed patchset #9 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/241fad6fdac74b820fa21e29652b...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318