[Sensors] Align sensor reading attribute implementation with the specification

[Sensors] Align sensor reading attribute implementation with the specification

This patch updates sensor reading implementation accordingly to specification [1] which says: "the reading attribute of two instances of the same Sensor interface associated with the same sensor hold the same SensorReading during a single event loop turn".

Besides, this patch also drops 'SensorReadingEvent' and 'SensorReadingEventInit' types which no longer exist in the specification.

[1] https://w3c.github.io/sensors/#sensor-reading

BUG=606766
Committed: https://crrev.com/62a49b3515112ea2ca6bbab6196f68717c52f94b
Cr-Commit-Position: refs/heads/master@{#430578}

[Mikhail, 2016-11-04 12:56:41]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 12:56:51]

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

[Mikhail, 2016-11-04 12:57:53]

Description was changed from

==========
[Sensors] Align sensor reading attribute implementation with the specification

This patch updates sensor reading implementation accordingly to specification
[1] which says: "the reading attribute of two instances of the same Sensor
interface associated with the same sensor hold the same SensorReading during a
single event loop turn".

Besides, this patch also drops 'ReadingEvent' and 'ReadingEventInit' types which
no longer exist in the specification.

[1] https://w3c.github.io/sensors/#sensor-reading

BUG=606766
==========

to

==========
[Sensors] Align sensor reading attribute implementation with the specification

This patch updates sensor reading implementation accordingly to specification
[1] which says: "the reading attribute of two instances of the same Sensor
interface associated with the same sensor hold the same SensorReading during a
single event loop turn".

Besides, this patch also drops 'SensorReadingEvent' and 'SensorReadingEventInit'
types which no longer exist in the specification.

[1] https://w3c.github.io/sensors/#sensor-reading

BUG=606766
==========

[Mikhail, 2016-11-04 12:57:53]

mikhail.pozdnyakov@intel.com changed reviewers:
+ alexander.shalamov@intel.com, haraken@chromium.org, timvolodine@chromium.org

[shalamov, 2016-11-04 13:06:53]

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/sensor/ambient-light-sensor.html (right):

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/sensor/ambient-light-sensor.html:186:
ambientLightSensor.onchange = e => {
If event doesn't have associated reading, maybe you can remove 'e'
() => {...}

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensorReading.cpp
(right):

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensorReading.cpp:18:
result.values[0] = init.illuminance();
else?
What value would be returned on line 36?

[Mikhail, 2016-11-04 13:20:34]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[Mikhail, 2016-11-04 13:20:48]

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/sensor/ambient-light-sensor.html (right):

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/sensor/ambient-light-sensor.html:186:
ambientLightSensor.onchange = e => {
On 2016/11/04 13:06:53, shalamov wrote:
> If event doesn't have associated reading, maybe you can remove 'e'
> () => {...}

Done.

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensorReading.cpp
(right):

https://codereview.chromium.org/2472403002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensorReading.cpp:18:
result.values[0] = init.illuminance();
On 2016/11/04 13:06:53, shalamov wrote:
> else?
> What value would be returned on line 36?

By default it's constructed with '0'.

[commit-bot: I haz the power, 2016-11-04 13:20:48]

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, 2016-11-04 13:50:33]

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

[commit-bot: I haz the power, 2016-11-04 13:50:33]

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

[Mikhail, 2016-11-04 18:49:16]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 18:49: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, 2016-11-04 21:14:43]

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

[commit-bot: I haz the power, 2016-11-04 21:14:44]

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

[Mikhail, 2016-11-05 15:50:54]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-05 15:51:06]

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, 2016-11-05 17:13:17]

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

[commit-bot: I haz the power, 2016-11-05 17:13:18]

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

[Mikhail, 2016-11-07 06:43:13]

Please take a look

[timvolodine, 2016-11-07 15:49:19]

a few comments below, otherwise looks good to me

https://codereview.chromium.org/2472403002/diff/60001/content/test/data/gener...
File content/test/data/generic_sensor/ambient_light_sensor_test.html (right):

https://codereview.chromium.org/2472403002/diff/60001/content/test/data/gener...
content/test/data/generic_sensor/ambient_light_sensor_test.html:7: function
onAmbientLightReadingChange(event) {
what's the event here? SensorReadingEvent has been removed right?

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp (right):

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp:56:
SensorProxy* SensorProviderProxy::getSensor(
perhaps easier to just have getSensor with on demand creation semantics?
i.e. instead of returning null create a new proxy, then the createSensor method
and related if constructions are not needed.

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/SensorReading.h (right):

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorReading.h:22:
DEFINE_INLINE_VIRTUAL_TRACE() {}
why the change from DECLARE to DEFINE? is this intentional?

[haraken, 2016-11-08 02:31:33]

Implementation LGTM

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/SensorReading.h (right):

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorReading.h:22:
DEFINE_INLINE_VIRTUAL_TRACE() {}
On 2016/11/07 15:49:19, timvolodine wrote:
> why the change from DECLARE to DEFINE? is this intentional?

Now the trace method is empty, we can just inline it in the header using DEFINE.

[shalamov, 2016-11-08 09:04:08]

lgtm

[Mikhail, 2016-11-08 09:09:15]

https://codereview.chromium.org/2472403002/diff/60001/content/test/data/gener...
File content/test/data/generic_sensor/ambient_light_sensor_test.html (right):

https://codereview.chromium.org/2472403002/diff/60001/content/test/data/gener...
content/test/data/generic_sensor/ambient_light_sensor_test.html:7: function
onAmbientLightReadingChange(event) {
On 2016/11/07 15:49:19, timvolodine wrote:
> what's the event here? SensorReadingEvent has been removed right?

indeed, thanks for the catch!

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp (right):

https://codereview.chromium.org/2472403002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp:56:
SensorProxy* SensorProviderProxy::getSensor(
On 2016/11/07 15:49:19, timvolodine wrote:
> perhaps easier to just have getSensor with on demand creation semantics?
> i.e. instead of returning null create a new proxy, then the createSensor
method
> and related if constructions are not needed.

This is how it was so far, I decided to split 'get' and 'create' as their
semantics starts to be different: 'createSensor' needs a new 'readingFactory'
instance which is unneeded at 'getSensor' call.

[Mikhail, 2016-11-08 09:09:22]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[Mikhail, 2016-11-08 09:09:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, alexander.shalamov@intel.com
Link to the patchset: https://codereview.chromium.org/2472403002/#ps80001
(title: "Comments from Tim")

[commit-bot: I haz the power, 2016-11-08 09:09:45]

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, 2016-11-08 11:33:19]

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

[commit-bot: I haz the power, 2016-11-08 11:33:24]

Failed to apply patch for
third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp:
While running git rm
third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp;
  fatal: pathspec
'third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp' did not match
any files

Patch:  D    third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp
Index: third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp
diff --git a/third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp
b/third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp
deleted file mode 100644
index
f5f64aaa14725c9d0383b5890edd3e4a9962bc85..0000000000000000000000000000000000000000
--- a/third_party/WebKit/Source/modules/sensor/SensorReadingEvent.cpp
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright 2016 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "modules/sensor/SensorReadingEvent.h"
-
-namespace blink {
-
-SensorReadingEvent::~SensorReadingEvent() = default;
-
-SensorReadingEvent::SensorReadingEvent(const AtomicString& eventType,
-                                       SensorReading* reading)
-    : Event(eventType, false, false),  // Does not bubble and is not
cancelable.
-      m_reading(reading) {
-  DCHECK(m_reading);
-}
-
-SensorReadingEvent::SensorReadingEvent(
-    const AtomicString& eventType,
-    const SensorReadingEventInit& initializer)
-    : SensorReadingEvent(eventType, initializer.reading()) {}
-
-const AtomicString& SensorReadingEvent::interfaceName() const {
-  return EventNames::SensorReadingEvent;
-}
-
-DEFINE_TRACE(SensorReadingEvent) {
-  Event::trace(visitor);
-  visitor->trace(m_reading);
-}
-
-}  // namespace blink

[commit-bot: I haz the power, 2016-11-08 11:34:52]

Description was changed from

==========
[Sensors] Align sensor reading attribute implementation with the specification

This patch updates sensor reading implementation accordingly to specification
[1] which says: "the reading attribute of two instances of the same Sensor
interface associated with the same sensor hold the same SensorReading during a
single event loop turn".

Besides, this patch also drops 'SensorReadingEvent' and 'SensorReadingEventInit'
types which no longer exist in the specification.

[1] https://w3c.github.io/sensors/#sensor-reading

BUG=606766
==========

to

==========
[Sensors] Align sensor reading attribute implementation with the specification

This patch updates sensor reading implementation accordingly to specification
[1] which says: "the reading attribute of two instances of the same Sensor
interface associated with the same sensor hold the same SensorReading during a
single event loop turn".

Besides, this patch also drops 'SensorReadingEvent' and 'SensorReadingEventInit'
types which no longer exist in the specification.

[1] https://w3c.github.io/sensors/#sensor-reading

BUG=606766

Committed: https://crrev.com/62a49b3515112ea2ca6bbab6196f68717c52f94b
Cr-Commit-Position: refs/heads/master@{#430578}
==========

[commit-bot: I haz the power, 2016-11-08 11:34:53]

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