Commit 073c4fe6 authored by Raphael Kubo da Costa's avatar Raphael Kubo da Costa Committed by Commit Bot

sensors: Make frequency hint test wait for another slow sensor "tick".

This test was flaky, especially on the Mac bots, where timers can often fire
with varying precision.

SensorProxy uses an ObserverSet to store Sensor instances that need to be
notified of a reading, so in some cases we could end up with the following
sequence of notifications in C++ and JavaScript:
1. OnSensorReadingChanged() is called for the fast sensor (running at either
   10Hz or 60Hz).
2. A "reading" event is delivered to the fast sensor's event handler in
   script.
3. OnSensorReadingChanged() is called for the slow sensor (running at 1/4th
   of the fast sensor's frequency).
4. A "reading" event is delivered to the slow sensor's event handler in
   script.
5. OnSensorReadingChanged() is called for the slow sensor again, but it is
   too early so it posts a delayed task to call NotifyReading().
6. OnSensorReadingChanged() is called for the fast sensor.
7. A "reading" event is delivered to the fast sensor's event handler in
   script.
8. The delayed task is executed, and a "reading" event is delivered to the
   slow sensor's event handler in script.

At this point, both |slowSensorNotifierCounter| and
|fastSensorNotifierCounter| are set to 2, and the assertion in the test
fails.

Fix it by waiting for another "reading" event to be delivered to the slow
sensor. Given the frequencies we choose, at this point a working
implementation's |fastSensorNotifierCounter| will definitely be higher than
2.

The downside is that we slow down the test by the value of another sensor
period -- for regular sensors, that is around 0.016s, while for slower
sensors such as AmbientLightSensor and Magnetometer (capped at 10Hz) this is
a 0.4s slowdown.

Bug: 731018
Change-Id: I249ab88eccf316888177a93614c2955b516bc835
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2204023
Auto-Submit: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770328}
parent fd9fbd72
......@@ -399,7 +399,7 @@ function runGenericSensorTests(sensorName,
const mockSensor = await sensorProvider.getCreatedSensor(sensorName);
await mockSensor.setSensorReading(readings);
const fastCounter = await new Promise((resolve, reject) => {
return new Promise((resolve, reject) => {
let fastSensorNotifiedCounter = 0;
let slowSensorNotifiedCounter = 0;
......@@ -413,10 +413,17 @@ function runGenericSensorTests(sensorName,
const slowSensor = new sensorType({frequency: slowFrequency});
slowSensor.onreading = () => {
// Skip the initial notification that always comes immediately.
if (slowSensorNotifiedCounter === 1) {
if (slowSensorNotifiedCounter === 2) {
fastSensor.stop();
slowSensor.stop();
resolve(fastSensorNotifiedCounter);
try {
assert_greater_than(fastSensorNotifiedCounter, 3,
"Fast sensor overtakes the slow one");
resolve();
} catch (e) {
reject(e);
}
}
slowSensorNotifiedCounter++;
}
......@@ -427,7 +434,6 @@ function runGenericSensorTests(sensorName,
}
fastSensor.onerror = reject;
});
assert_greater_than(fastCounter, 2, "Fast sensor overtakes the slow one");
}, `${sensorName}: frequency hint works.`);
// Re-enable after https://github.com/w3c/sensors/issues/361 is fixed.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment