Windows: Ambient Light API

Windows: Ambient Light API

TEST= http://jsfiddle.net/73v8rmex/3/ (manual)

BUG=336424
Committed: https://crrev.com/a6d0c54aa3c008eb25d759bcb06422c34aa72040
Cr-Commit-Position: refs/heads/master@{#319060}

[riju_, 2015-02-04 15:31:37]

Patchset #2 (id:20001) has been deleted

[riju_, 2015-02-04 15:36:25]

Patchset #2 (id:40001) has been deleted

[riju_, 2015-02-27 11:50:27]

rijubrata.bhaumik@intel.com changed reviewers:
+ timvolodine@chromium.org

[riju_, 2015-02-27 11:50:27]

Hi Tim, please take a look.

[timvolodine, 2015-03-02 17:39:58]

thanks riju!
is there a test url or so to verify this functionality? if so maybe add that as
a TEST=url (manual) line in description.

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
File content/browser/device_sensors/data_fetcher_shared_memory_win.cc (right):

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:21: static bool
SetLightBuffer(content::DeviceLightHardwareBuffer* buffer,
no need for static

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:28: return
true;
do you need to return anything at all? the result doesn't seem to be used
anywhere in the code..

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:248:
GetSensorValue(SENSOR_DATA_TYPE_LIGHT_LEVEL_LUX, new_data, &lux, &has_lux);
has_lux does not seem be used after the call. should anything special happen in
case has_lux == false?

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:318: case
CONSUMER_TYPE_LIGHT:
alignment as with above cases

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:325: if
(sensor_light_available)
what's the value of the buffer? should it be initialized to -1?

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:349:
SetLightBuffer(light_buffer_, -1);
also light_buffer = nullptr?

[riju_, 2015-03-03 09:53:55]

Thanks Tim !

Just added a simple test page at http://jsfiddle.net/73v8rmex/3/
If i need to demo to anybody, i use this
http://www.francesco.iovine.name/mdn/xmas-tales/public_html/ because it looks
good.

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
File content/browser/device_sensors/data_fetcher_shared_memory_win.cc (right):

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:21: static bool
SetLightBuffer(content::DeviceLightHardwareBuffer* buffer,
On 2015/03/02 17:39:58, timvolodine wrote:
> no need for static

Done.

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:28: return
true;
On 2015/03/02 17:39:58, timvolodine wrote:
> do you need to return anything at all? the result doesn't seem to be used
> anywhere in the code..

Done.

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:248:
GetSensorValue(SENSOR_DATA_TYPE_LIGHT_LEVEL_LUX, new_data, &lux, &has_lux);
On 2015/03/02 17:39:58, timvolodine wrote:
> has_lux does not seem be used after the call. should anything special happen
in
> case has_lux == false?

Done. returning early if valid no lux value is found.

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:318: case
CONSUMER_TYPE_LIGHT:
On 2015/03/02 17:39:58, timvolodine wrote:
> alignment as with above cases

Done.

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:325: if
(sensor_light_available)
On 2015/03/02 17:39:58, timvolodine wrote:
> what's the value of the buffer? should it be initialized to -1?

Done.

https://codereview.chromium.org/754963006/diff/80001/content/browser/device_s...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:349:
SetLightBuffer(light_buffer_, -1);
On 2015/03/02 17:39:58, timvolodine wrote:
> also light_buffer = nullptr?

Done.

[timvolodine, 2015-03-04 12:06:37]

thanks, lgtm

https://codereview.chromium.org/754963006/diff/100001/content/browser/device_...
File content/browser/device_sensors/data_fetcher_shared_memory_win.cc (right):

https://codereview.chromium.org/754963006/diff/100001/content/browser/device_...
content/browser/device_sensors/data_fetcher_shared_memory_win.cc:21: void
SetLightBuffer(content::DeviceLightHardwareBuffer* buffer, double lux) {
nit: DCHECK(buffer)?

[riju_, 2015-03-04 12:34:23]

The CQ bit was checked by rijubrata.bhaumik@intel.com

[riju_, 2015-03-04 12:34:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
timvolodine@chromium.org
Link to the patchset: https://codereview.chromium.org/754963006/#ps120001
(title: "add dcheck")

[commit-bot: I haz the power, 2015-03-04 12:34:46]

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

[commit-bot: I haz the power, 2015-03-04 13:31:18]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-03-04 13:32:08]

Patchset 5 (id:??) landed as
https://crrev.com/a6d0c54aa3c008eb25d759bcb06422c34aa72040
Cr-Commit-Position: refs/heads/master@{#319060}