[Sensors] Clamp given sampling frequency to the minimum supported one

[Sensors] Clamp given sampling frequency to the minimum supported  one

The given frequency must be clamped to the minimum supported by the platform, as follows from https://github.com/w3c/sensors/issues/169

This CL only introduces generic code which provides the minimum supported frequency from platform layer to the Sensor JS bindings (layout tests are updated accordingly), platform-specific implementations are to be added in following patches.

BUG=606766
Review-Url: https://codereview.chromium.org/2704123006
Cr-Commit-Position: refs/heads/master@{#452827}
Committed: https://chromium.googlesource.com/chromium/src/+/883ba43fb0d728741ba56dbd8ec355b942abe1fe

[Mikhail, 2017-02-22 16:02:51]

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

[commit-bot: I haz the power, 2017-02-22 16:03:39]

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

[Mikhail, 2017-02-22 16:26:58]

Description was changed from

==========
[Sensors] Clamp given sampling frequency to the minimum supported  one

The given frequency must be clamped to the minimum supported by the platform, as
follows from https://github.com/w3c/sensors/issues/169

This CL only introduces generic code which provides the minimum supported
frequency from platform layer to the Sensor JS bindings (layout tests are
updated accordingly), platform-specific implementations are to be added in
following patches.

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

to

==========
[Sensors] Clamp given sampling frequency to the minimum supported  one

The given frequency must be clamped to the minimum supported by the platform, as
follows from https://github.com/w3c/sensors/issues/169

This CL only introduces generic code which provides the minimum supported
frequency from platform layer to the Sensor JS bindings (layout tests are
updated accordingly), platform-specific implementations are to be added in
following patches.

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

[Mikhail, 2017-02-22 16:26:59]

mikhail.pozdnyakov@intel.com changed reviewers:
+ haraken@chromium.org, reillyg@chromium.org

[Mikhail, 2017-02-22 16:27:43]

PTAL

[commit-bot: I haz the power, 2017-02-22 16:31:24]

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

[commit-bot: I haz the power, 2017-02-22 16:31:25]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Mikhail, 2017-02-22 16:54:29]

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

[Mikhail, 2017-02-22 16:54:44]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-02-22 16:55:10]

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

[Reilly Grant (use Gerrit), 2017-02-22 18:40:15]

lgtm with nits

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/p...
File device/generic_sensor/platform_sensor.h (right):

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/p...
device/generic_sensor/platform_sensor.h:52: // The default implementation
returns '1.0 / (60 * 60)', i.e. once per hour.
This feels somewhat arbitrary but using
std::numeric_limits<double>::denorm_min() would probably cause problems.

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/s...
File device/generic_sensor/sensor_provider_impl.cc (right):

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/s...
device/generic_sensor/sensor_provider_impl.cc:104:
DCHECK(init_params->minimum_frequency > 0);
DCHECK_GT(init_params->minimum_frequency, 0);

(Also above for maximum_frequency, sorry I didn't catch that earlier.)

https://codereview.chromium.org/2704123006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/SensorProxy.cpp (right):

https://codereview.chromium.org/2704123006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorProxy.cpp:218:
DCHECK(m_frequencyLimits.first <= m_frequencyLimits.second);
DCHECK_GE

https://codereview.chromium.org/2704123006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorProxy.cpp:219:
DCHECK(m_frequencyLimits.second <= SensorConfiguration::kMaxAllowedFrequency);
DCHECK_GE

[Reilly Grant (use Gerrit), 2017-02-22 18:46:56]

reillyg@chromium.org changed reviewers:
+ mbarbella@chromium.org

[Reilly Grant (use Gerrit), 2017-02-22 18:46:57]

Adding mbarbella@ to review changes to sensor_provider.mojom.

[commit-bot: I haz the power, 2017-02-22 19:35:28]

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

[commit-bot: I haz the power, 2017-02-22 19:35:29]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-02-22 23:32:44]

LGTM

[Martin Barbella, 2017-02-23 01:05:39]

mojom lgtm

[Mikhail, 2017-02-23 12:42:01]

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

[commit-bot: I haz the power, 2017-02-23 12:42:16]

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

[Mikhail, 2017-02-23 12:42:34]

thanks for taking a look!

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/p...
File device/generic_sensor/platform_sensor.h (right):

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/p...
device/generic_sensor/platform_sensor.h:52: // The default implementation
returns '1.0 / (60 * 60)', i.e. once per hour.
On 2017/02/22 18:40:15, Reilly Grant wrote:
> This feels somewhat arbitrary but using
> std::numeric_limits<double>::denorm_min() would probably cause problems.

yeah, ideally we should get the actual values from the platform.

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/s...
File device/generic_sensor/sensor_provider_impl.cc (right):

https://codereview.chromium.org/2704123006/diff/20001/device/generic_sensor/s...
device/generic_sensor/sensor_provider_impl.cc:104:
DCHECK(init_params->minimum_frequency > 0);
On 2017/02/22 18:40:15, Reilly Grant wrote:
> DCHECK_GT(init_params->minimum_frequency, 0);
> 
> (Also above for maximum_frequency, sorry I didn't catch that earlier.)

Done.

https://codereview.chromium.org/2704123006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/SensorProxy.cpp (right):

https://codereview.chromium.org/2704123006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorProxy.cpp:218:
DCHECK(m_frequencyLimits.first <= m_frequencyLimits.second);
On 2017/02/22 18:40:15, Reilly Grant wrote:
> DCHECK_GE

Done.

https://codereview.chromium.org/2704123006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/SensorProxy.cpp:219:
DCHECK(m_frequencyLimits.second <= SensorConfiguration::kMaxAllowedFrequency);
On 2017/02/22 18:40:15, Reilly Grant wrote:
> DCHECK_GE

Done.

[commit-bot: I haz the power, 2017-02-23 12:51:25]

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

[commit-bot: I haz the power, 2017-02-23 12:51:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mikhail, 2017-02-23 14:34:07]

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

[commit-bot: I haz the power, 2017-02-23 14:34:21]

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-02-23 14:44:04]

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

[commit-bot: I haz the power, 2017-02-23 14:44:05]

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

[Mikhail, 2017-02-24 11:11:12]

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

[commit-bot: I haz the power, 2017-02-24 11:11:23]

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

[Mikhail, 2017-02-24 11:14:49]

Patchset #3 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-02-24 13:20:01]

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

[commit-bot: I haz the power, 2017-02-24 13:20:02]

Dry run: This issue passed the CQ dry run.

[Mikhail, 2017-02-24 13:22:45]

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

[Mikhail, 2017-02-24 13:22:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, mbarbella@chromium.org, reillyg@chromium.org
Link to the patchset: https://codereview.chromium.org/2704123006/#ps80001
(title: "Build fix")

[commit-bot: I haz the power, 2017-02-24 13:23:04]

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-02-24 13:26:46]

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

[commit-bot: I haz the power, 2017-02-24 13:26:47]

Failed to apply patch for
third_party/WebKit/Source/modules/sensor/SensorProxy.cpp:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/modules/sensor/SensorProxy.cpp:212
  error: third_party/WebKit/Source/modules/sensor/SensorProxy.cpp: patch does
not apply

Patch:       third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
Index: third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
diff --git a/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
b/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
index
53c7e9f3597fae81ededb1a03cacf3b82841a112..710fd452d7cb37bf51b358948f4e9165d9685899
100644
--- a/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
+++ b/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
@@ -24,8 +24,7 @@ SensorProxy::SensorProxy(SensorType sensorType,
       m_provider(provider),
       m_clientBinding(this),
       m_state(SensorProxy::Uninitialized),
-      m_suspended(false),
-      m_maximumFrequency(0.0) {}
+      m_suspended(false) {}
 
 SensorProxy::~SensorProxy() {}
 
@@ -212,9 +211,14 @@ void SensorProxy::onSensorCreated(SensorInitParamsPtr
params,
     handleSensorError();
     return;
   }
-
-  m_maximumFrequency = params->maximum_frequency;
-  DCHECK(m_maximumFrequency <= SensorConfiguration::kMaxAllowedFrequency);
+  m_frequencyLimits.first = params->minimum_frequency;
+  m_frequencyLimits.second = params->maximum_frequency;
+
+  DCHECK_GT(m_frequencyLimits.first, 0.0);
+  DCHECK_GE(m_frequencyLimits.second, m_frequencyLimits.first);
+  constexpr double kMaxAllowedFrequency =
+      SensorConfiguration::kMaxAllowedFrequency;
+  DCHECK_GE(kMaxAllowedFrequency, m_frequencyLimits.second);
 
   auto errorCallback =
       WTF::bind(&SensorProxy::handleSensorError, wrapWeakPersistent(this),

[Mikhail, 2017-02-24 13:51:24]

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

[Mikhail, 2017-02-24 13:51:25]

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

[Mikhail, 2017-02-24 13:51:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, mbarbella@chromium.org, reillyg@chromium.org
Link to the patchset: https://codereview.chromium.org/2704123006/#ps100001
(title: "rebased")

[commit-bot: I haz the power, 2017-02-24 13:51:40]

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-02-24 15:25:45]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487944285140850,
"parent_rev": "628844c6dd31d3727fae5bcee1b990f3ecde068a", "commit_rev":
"883ba43fb0d728741ba56dbd8ec355b942abe1fe"}

[commit-bot: I haz the power, 2017-02-24 15:26:28]

Description was changed from

==========
[Sensors] Clamp given sampling frequency to the minimum supported  one

The given frequency must be clamped to the minimum supported by the platform, as
follows from https://github.com/w3c/sensors/issues/169

This CL only introduces generic code which provides the minimum supported
frequency from platform layer to the Sensor JS bindings (layout tests are
updated accordingly), platform-specific implementations are to be added in
following patches.

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

to

==========
[Sensors] Clamp given sampling frequency to the minimum supported  one

The given frequency must be clamped to the minimum supported by the platform, as
follows from https://github.com/w3c/sensors/issues/169

This CL only introduces generic code which provides the minimum supported
frequency from platform layer to the Sensor JS bindings (layout tests are
updated accordingly), platform-specific implementations are to be added in
following patches.

BUG=606766

Review-Url: https://codereview.chromium.org/2704123006
Cr-Commit-Position: refs/heads/master@{#452827}
Committed:
https://chromium.googlesource.com/chromium/src/+/883ba43fb0d728741ba56dbd8ec3...
==========

[commit-bot: I haz the power, 2017-02-24 15:26:29]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/883ba43fb0d728741ba56dbd8ec3...