[DeviceLight] Ensure to round lux value to avoid fingerprinting risk.

[DeviceLight] Ensure to round lux value to avoid fingerprinting risk.

Make sure to only expose rounded lux values to JavaScript in order
to decrease fingerprinting risk. This also ensures less frequent
updates in case the lux value is of high precision.

BUG=642731
TBR=mkwst@chromium.org

Review-Url: https://codereview.chromium.org/2772843002
Cr-Commit-Position: refs/heads/master@{#465865}
Committed: https://chromium.googlesource.com/chromium/src/+/4ce63dcc9eee16f803df5ef92d29a7c23bdb32c7

[timvolodine, 2017-03-23 16:36:18]

The CQ bit was checked by timvolodine@chromium.org to run a CQ dry run

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

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

[timvolodine, 2017-03-23 16:37:38]

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

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

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

[commit-bot: I haz the power, 2017-03-23 16:50:55]

Dry run: 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_...)

[palmer, 2017-03-23 19:00:18]

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

[palmer, 2017-03-23 19:00:19]

https://codereview.chromium.org/2772843002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/device_light/DeviceLightDispatcher.cpp
(right):

https://codereview.chromium.org/2772843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/device_light/DeviceLightDispatcher.cpp:18:
return isinf(lux) ? lux : std::round(lux);
Apologies in advance for the drive-by. :)

A rounded double is still a high-precision value, though. I'd really like to get
it lower if possible, like down to 4 bits of precision. (Or whatever small range
still suits the intended applications.)

[battre, 2017-03-24 15:04:18]

LGTM but exposing 4 bits or 0%, 10%, 20%, ... 90%, 100% would look much better
to me.

[timvolodine, 2017-03-27 11:47:12]

The CQ bit was checked by timvolodine@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 11:47:34]

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-27 12:13:30]

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

[commit-bot: I haz the power, 2017-03-27 12:13:30]

Dry run: 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_...)

[timvolodine, 2017-04-19 19:48:17]

The CQ bit was checked by timvolodine@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 19:49:04]

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-04-19 20:00:19]

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

[commit-bot: I haz the power, 2017-04-19 20:00:21]

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

[timvolodine, 2017-04-19 22:51:50]

The CQ bit was checked by timvolodine@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 22:52:40]

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-04-20 00:34:32]

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

[commit-bot: I haz the power, 2017-04-20 00:34:33]

Dry run: This issue passed the CQ dry run.

[timvolodine, 2017-04-20 01:34:30]

Yes agree with the comments that further reduced precision might be better,
however this is not as straightforward due to the original spec
(https://www.w3.org/TR/2015/WD-ambient-light-20150903/#idl-def-DeviceLightEvent)
mentioning explicitly using lux value and also "infinity" value in case no light
information is available.
Added a todo, will land this for now as it is a strict improvement.

https://codereview.chromium.org/2772843002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/device_light/DeviceLightDispatcher.cpp
(right):

https://codereview.chromium.org/2772843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/device_light/DeviceLightDispatcher.cpp:18:
return isinf(lux) ? lux : std::round(lux);
On 2017/03/23 19:00:19, palmer wrote:
> Apologies in advance for the drive-by. :)
> 
> A rounded double is still a high-precision value, though. I'd really like to
get
> it lower if possible, like down to 4 bits of precision. (Or whatever small
range
> still suits the intended applications.)

Acknowledged.

[timvolodine, 2017-04-20 01:34:35]

The CQ bit was checked by timvolodine@chromium.org

[timvolodine, 2017-04-20 01:34:35]

The patchset sent to the CQ was uploaded after l-g-t-m from battre@chromium.org
Link to the patchset: https://codereview.chromium.org/2772843002/#ps60001
(title: "fix compile")

[commit-bot: I haz the power, 2017-04-20 01:35:17]

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-04-20 01:44:50]

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

[commit-bot: I haz the power, 2017-04-20 01:44:54]

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...)

[timvolodine, 2017-04-20 02:14:17]

Description was changed from

==========
[DeviceLight] Ensure to round lux value to avoid fingerprinting risk.

Make sure to only expose rounded lux values to JavaScript in order
to decrease fingerprinting risk. This also ensures less frequent
updates in case the lux value is of high precision.

BUG=642731
==========

to

==========
[DeviceLight] Ensure to round lux value to avoid fingerprinting risk.

Make sure to only expose rounded lux values to JavaScript in order
to decrease fingerprinting risk. This also ensures less frequent
updates in case the lux value is of high precision.

BUG=642731
TBR=mkwst@chromium.org
==========

[timvolodine, 2017-04-20 02:14:18]

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

[timvolodine, 2017-04-20 02:14:22]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2017-04-20 02:14: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-04-20 02:20:26]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492654462821000,
"parent_rev": "6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7", "commit_rev":
"4ce63dcc9eee16f803df5ef92d29a7c23bdb32c7"}

[commit-bot: I haz the power, 2017-04-20 02:20:44]

Description was changed from

==========
[DeviceLight] Ensure to round lux value to avoid fingerprinting risk.

Make sure to only expose rounded lux values to JavaScript in order
to decrease fingerprinting risk. This also ensures less frequent
updates in case the lux value is of high precision.

BUG=642731
TBR=mkwst@chromium.org
==========

to

==========
[DeviceLight] Ensure to round lux value to avoid fingerprinting risk.

Make sure to only expose rounded lux values to JavaScript in order
to decrease fingerprinting risk. This also ensures less frequent
updates in case the lux value is of high precision.

BUG=642731
TBR=mkwst@chromium.org

Review-Url: https://codereview.chromium.org/2772843002
Cr-Commit-Position: refs/heads/master@{#465865}
Committed:
https://chromium.googlesource.com/chromium/src/+/4ce63dcc9eee16f803df5ef92d29...
==========

[commit-bot: I haz the power, 2017-04-20 02:20:47]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/4ce63dcc9eee16f803df5ef92d29...

[battre, 2017-04-20 09:00:27]

On 2017/04/20 01:34:30, timvolodine (OOO) wrote:
> Yes agree with the comments that further reduced precision might be better,
> however this is not as straightforward due to the original spec
>
(https://www.w3.org/TR/2015/WD-ambient-light-20150903/#idl-def-DeviceLightEvent)
> mentioning explicitly using lux value and also "infinity" value in case no
light
> information is available.
> Added a todo, will land this for now as it is a strict improvement.

According to https://en.wikipedia.org/wiki/Daylight, bright sunlight is up to
120,000 lux (~1 lux during cloudy days). At the same time, night illuminance go
down to 0.01 lux or a quarter moon. So this means the this CL added little
privacy protection during the day and broke the API for dark rooms. Or am I
missing something?

I think that we need a logarithmic scale that we divide into N steps.

[palmer, 2017-04-20 19:21:55]

> According to https://en.wikipedia.org/wiki/Daylight, bright sunlight is up to
120,000 lux (~1 lux during cloudy days). At the same time, night illuminance go
down to 0.01 lux or a quarter moon. So this means the this CL added little
privacy protection during the day and broke the API for dark rooms. Or am I
missing something?
> 
> I think that we need a logarithmic scale that we divide into N steps.

+1. Or, should I say, +2**0.

[timvolodine, 2017-05-10 17:46:43]

On 2017/04/20 19:21:55, palmer wrote:
> > According to https://en.wikipedia.org/wiki/Daylight, bright sunlight is up
to
> 120,000 lux (~1 lux during cloudy days). At the same time, night illuminance
go
> down to 0.01 lux or a quarter moon. So this means the this CL added little
> privacy protection during the day and broke the API for dark rooms. Or am I
> missing something?
> > 
> > I think that we need a logarithmic scale that we divide into N steps.
> 
> +1. Or, should I say, +2**0.

I've commented on the bug with details, just reiterating here FYI the
DeviceLight implementation has been removed recently. Hopefully the
suggestions/discussion can continue in the context of generic sensors..

[battre, 2017-05-11 09:29:18]

On 2017/05/10 17:46:43, timvolodine wrote:
> On 2017/04/20 19:21:55, palmer wrote:
> > > According to https://en.wikipedia.org/wiki/Daylight, bright sunlight is up
> to
> > 120,000 lux (~1 lux during cloudy days). At the same time, night illuminance
> go
> > down to 0.01 lux or a quarter moon. So this means the this CL added little
> > privacy protection during the day and broke the API for dark rooms. Or am I
> > missing something?
> > > 
> > > I think that we need a logarithmic scale that we divide into N steps.
> > 
> > +1. Or, should I say, +2**0.
> 
> I've commented on the bug with details, just reiterating here FYI the
> DeviceLight implementation has been removed recently. Hopefully the
> suggestions/discussion can continue in the context of generic sensors..

Do you have a launch blocking issue for this already?

[timvolodine, 2017-05-11 15:04:15]

On 2017/05/11 09:29:18, battre wrote:
> On 2017/05/10 17:46:43, timvolodine wrote:
> > On 2017/04/20 19:21:55, palmer wrote:
> > > > According to https://en.wikipedia.org/wiki/Daylight, bright sunlight is
up
> > to
> > > 120,000 lux (~1 lux during cloudy days). At the same time, night
illuminance
> > go
> > > down to 0.01 lux or a quarter moon. So this means the this CL added little
> > > privacy protection during the day and broke the API for dark rooms. Or am
I
> > > missing something?
> > > > 
> > > > I think that we need a logarithmic scale that we divide into N steps.
> > > 
> > > +1. Or, should I say, +2**0.
> > 
> > I've commented on the bug with details, just reiterating here FYI the
> > DeviceLight implementation has been removed recently. Hopefully the
> > suggestions/discussion can continue in the context of generic sensors..
> 
> Do you have a launch blocking issue for this already?

ok I've reopened the bug as it is still relevant for the Ambient Light Sensor +
added more people on it