Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(445)

Issue 152893002: [DeviceLight API] Content Side

Created:
6 years, 10 months ago by riju_
Modified:
6 years, 8 months ago
Reviewers:
timvolodine
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

[WIP] [DeviceLight API] corresponding Blink side change : https://codereview.chromium.org/143823004/ Implement the content/renderer and content/browser part of the Device Light API. Mostly added the content side plumbing like event pump As of now I am trying to follow DeviceMotion/DeviceOrientation Add the actual data fetchers for Android. We are utilizing the same infrastructure/ plumbing done by Device Motion and Orientation work

Patch Set 1 #

Patch Set 2 : Fix the Android Data Fetcher for Light #

Patch Set 3 : Layout Test preparation #

Patch Set 4 : Add filter message Rebase #

Patch Set 5 : rebase #

Patch Set 6 : rebase (java file changes) #

Patch Set 7 : rebase (java file changes) #

Patch Set 8 : DeviceLightMessageFilter init #

Patch Set 9 : Style/Linting nits #

Patch Set 10 : remove switch #

Patch Set 11 : Fix in Java file, stop() #

Unified diffs Side-by-side diffs Delta from patch set Stats (+489 lines, -53 lines) Patch
A content/browser/device_light/device_light_message_filter.h View 1 chunk +38 lines, -0 lines 0 comments Download
A content/browser/device_light/device_light_message_filter.cc View 1 2 3 4 5 6 7 1 chunk +66 lines, -0 lines 0 comments Download
M content/browser/device_orientation/data_fetcher_shared_memory.h View 2 chunks +2 lines, -0 lines 0 comments Download
M content/browser/device_orientation/data_fetcher_shared_memory_android.cc View 1 2 3 3 chunks +9 lines, -0 lines 0 comments Download
M content/browser/device_orientation/data_fetcher_shared_memory_base.h View 1 2 3 4 5 6 7 8 2 chunks +1 line, -2 lines 0 comments Download
M content/browser/device_orientation/data_fetcher_shared_memory_base.cc View 1 2 3 4 5 6 7 8 5 chunks +6 lines, -3 lines 0 comments Download
M content/browser/device_orientation/data_fetcher_shared_memory_base_unittest.cc View 1 2 3 4 5 6 7 8 3 chunks +0 lines, -3 lines 0 comments Download
M content/browser/device_orientation/data_fetcher_shared_memory_default.cc View 1 2 3 4 5 6 7 8 4 chunks +17 lines, -3 lines 0 comments Download
M content/browser/device_orientation/data_fetcher_shared_memory_win.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/device_orientation/device_inertial_sensor_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 7 chunks +21 lines, -4 lines 0 comments Download
M content/browser/device_orientation/device_inertial_sensor_service.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/device_orientation/device_inertial_sensor_service.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +10 lines, -3 lines 0 comments Download
M content/browser/device_orientation/device_motion_message_filter.h View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -3 lines 0 comments Download
M content/browser/device_orientation/inertial_sensor_consts.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/device_orientation/sensor_manager_android.h View 1 2 3 4 5 6 7 8 6 chunks +20 lines, -7 lines 0 comments Download
M content/browser/device_orientation/sensor_manager_android.cc View 1 2 3 4 5 6 7 8 3 chunks +58 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -0 lines 0 comments Download
M content/common/content_message_generator.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
A content/common/device_light/device_light_hardware_buffer.h View 1 chunk +17 lines, -0 lines 0 comments Download
A + content/common/device_light/device_light_messages.h View 1 2 3 4 5 6 7 8 2 chunks +13 lines, -8 lines 0 comments Download
M content/content_browser.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M content/content_common.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M content/content_renderer.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java View 1 2 3 4 5 6 7 8 9 10 9 chunks +48 lines, -0 lines 0 comments Download
M content/public/test/layouttest_support.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +4 lines, -0 lines 0 comments Download
A + content/renderer/device_light/device_light_event_pump.h View 2 chunks +14 lines, -15 lines 0 comments Download
A content/renderer/device_light/device_light_event_pump.cc View 1 chunk +62 lines, -0 lines 0 comments Download
M content/renderer/renderer_webkitplatformsupport_impl.h View 1 2 3 4 5 6 7 8 9 10 4 chunks +7 lines, -0 lines 0 comments Download
M content/renderer/renderer_webkitplatformsupport_impl.cc View 1 2 3 4 5 6 7 8 9 10 4 chunks +30 lines, -0 lines 0 comments Download
M content/shell/renderer/test_runner/WebTestDelegate.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M content/shell/renderer/test_runner/test_runner.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M content/shell/renderer/test_runner/test_runner.cc View 1 2 3 4 5 6 7 8 9 10 4 chunks +16 lines, -0 lines 0 comments Download
M content/shell/renderer/webkit_test_runner.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M content/shell/renderer/webkit_test_runner.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +4 lines, -0 lines 0 comments Download
M content/test/layouttest_support.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -0 lines 0 comments Download
M ipc/ipc_message_start.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
riju_
6 years, 9 months ago (2014-03-04 17:32:21 UTC) #1
riju_
Hi Tim, Please have a look. I have piggy backed on the Device Motion/ Orientation ...
6 years, 9 months ago (2014-03-19 13:29:34 UTC) #2
timvolodine
On 2014/03/19 13:29:34, riju_ wrote: > Hi Tim, > Please have a look. > I ...
6 years, 9 months ago (2014-03-21 15:28:13 UTC) #3
riju_
On 2014/03/21 15:28:13, timvolodine wrote: > On 2014/03/19 13:29:34, riju_ wrote: > > Hi Tim, ...
6 years, 9 months ago (2014-03-21 16:10:55 UTC) #4
timvolodine
6 years, 9 months ago (2014-03-21 16:51:05 UTC) #5
On 2014/03/21 16:10:55, riju_ wrote:
> On 2014/03/21 15:28:13, timvolodine wrote:
> > On 2014/03/19 13:29:34, riju_ wrote:
> > > Hi Tim, 
> > > Please have a look. 
> > > I have piggy backed on the Device Motion/ Orientation plumbing/ infra
work.
> > > 
> > > We should wait for the Blink side to get in first.
> > > https://codereview.chromium.org/143823004/
> > 
> > Great to see you could reuse the existing infrastructure for device
> > motion/orientation.
> > I have a few comments / question:
> > - we should split the content/renderer + IPC part and content/browser + java
> > part into separate patches for easier review.
> > - on the browser-side the ambient light sensor code is added to Device
> > Motion/Orientation specific classes. We should think about how to
> > rename/refactor this. As it is now it's not submittable.
> > - how often do you expect the ambient light sensor data to change? Device
> > Motion/Orientation was specifically designed with high frequency data in
> mind..
> 
> Tim : Thanks for having a look. 
> This big CL was just to request for comments, and me having a working demo on
> Nexus 5 before i present to reviewers.
> -Yes, I will split this to 3 CLs as you suggested.
> -Re factoring: Yes. I will try to extract the common parts (like we did for
the
> Blink part)
re re-factoring/renaming: probably most of the changes will need to happen in
content/browser, you seems to have a separate pump already in
content/renderer/device_light. Maybe we should put the whole thing in
content/renderer/device_sensors/

> -Ambient light should not be needing 20Hz (kDefaultPumpDelayMillis = 50) we
have
> in device_sensor_event_pump.cc. 
> I don't have a exact value in mind, maybe 1000ms is a good value.

This is actually important, as for infrequent updates like 1s the alternative
would be to use IPC (as Geolocation does for example) avoiding shared memory.
This is probably not critical for Ambient Light data, but if the pump is set to
1s there will be a worst case propagation delay of 1s for the sensor data to
reach Blink.
I am currently proposing to add a control IPC message like SharedMemoryChanged
to allow for 'immediate' propagation to solve this issue.

> 
> Cheers,
> Riju.

Powered by Google App Engine
This is Rietveld 408576698