Skip to content

Commit

Permalink
sensors: Make virtual sensors always remember and report the latest r…
Browse files Browse the repository at this point in the history
…eading

This concerns a discussion that started in https://crrev.com/c/5249795:
the idea is to make a virtual sensor leak fewer implementation details,
such as the fact that the current reading is lost when the sensor is
deactivated. This should not be observable from e.g. a web test, as
there is nothing mandating that this happen in the first place.

From an API user's perspective, once a reading is added to a virtual
sensor (including if this happens before a Sensor instance is created in
Chromium) it remains present until it is replaced by another one, even
if the sensor is stopped and reactivated. This is complemented by the
behavior added in https://crrev.com/c/5245506, in which any pending
reading (or, with this CL, the latest reading) is stashed until it can
be published.

In other words, a VirtualPlatformSensor now respects the following two
invariants:
1. Once a reading is set, it will remain set until it is replaced with
   another reading.
2. Said reading will be added to the shared buffer and clients will be
   notified whenever the sensor is activated.

While here, revert the part of https://crrev.com/c/5245506 that renamed
PlatformSensor::IsActiveForTesting() to is_active(), as this is no
longer needed.

Bug: 1520919, 1506995
Change-Id: I205432cbf196376f67a6bc1c84d69302ffe739ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5272509
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Auto-Submit: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258046}
  • Loading branch information
rakuco authored and chromium-wpt-export-bot committed Feb 8, 2024
1 parent 11f539a commit effd91d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 32 deletions.
37 changes: 13 additions & 24 deletions generic-sensor/generic-sensor-iframe-tests.sub.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,24 +147,21 @@ function run_generic_sensor_iframe_tests(sensorData, readingData) {
sensor.stop();
await send_message_to_iframe(iframe, {command: 'stop_sensor'});

// The sensors are stopped; queue the same reading. The virtual sensor
// would send it anyway, but this update changes its timestamp.
await test_driver.update_virtual_sensor(testDriverName, reading);

// Now focus the cross-origin iframe. The situation should be the opposite:
// the sensor in the main frame should not fire any "reading" events or
// provide access to updated readings, but the sensor in the iframe should.
iframe.contentWindow.focus();

// Start both sensors. They should both have the same state: active, but no
// readings have been provided to them yet.
await send_message_to_iframe(iframe, {command: 'start_sensor'});
// Start both sensors. Only the iframe sensor should receive a reading
// event and contain readings.
sensor.start();
await sensorWatcher.wait_for('activate');
assert_false(
await send_message_to_iframe(iframe, {command: 'has_reading'}));
assert_false(sensor.hasReading);

const [serializedIframeSensor] = await Promise.all([
iframeSensorWatcher.wait_for_reading(),
test_driver.update_virtual_sensor(testDriverName, reading),
]);
await send_message_to_iframe(iframe, {command: 'start_sensor'});
const serializedIframeSensor = await iframeSensorWatcher.wait_for_reading();
assert_true(await send_message_to_iframe(iframe, {command: 'has_reading'}));
assert_false(sensor.hasReading);

Expand Down Expand Up @@ -225,25 +222,17 @@ function run_generic_sensor_iframe_tests(sensorData, readingData) {
// Focus a different same-origin window each time and check that everything
// works the same.
for (const windowObject of [window, iframe.contentWindow]) {
await test_driver.update_virtual_sensor(
testDriverName, readings.next().value);

windowObject.focus();

iframeSensor.start();
sensor.start();
await Promise.all([
iframeSensorWatcher.wait_for('activate'),
sensorWatcher.wait_for('activate')
]);

assert_false(sensor.hasReading);
assert_false(iframeSensor.hasReading);

// We store `reading` here because we want to make sure the very same
// value is accepted later.
const reading = readings.next().value;
await Promise.all([
test_driver.update_virtual_sensor(testDriverName, reading),
iframeSensorWatcher.wait_for('reading'),
sensorWatcher.wait_for('reading')
iframeSensorWatcher.wait_for(['activate', 'reading']),
sensorWatcher.wait_for(['activate', 'reading'])
]);

assert_greater_than(
Expand Down
17 changes: 9 additions & 8 deletions generic-sensor/generic-sensor-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -429,16 +429,16 @@ function runGenericSensorTests(sensorData, readingData) {
assert_false(sensor.hasReading);
assert_false(sensor.activated);

readings.reset();
await test_driver.update_virtual_sensor(
testDriverName, readings.next().value);

sensor.start();

// Starting |sensor| again will cause the backing virtual sensor to report
// the previous reading automatically.
await sensorWatcher.wait_for('activate');
assert_false(sensor.hasReading);
readings.reset();
await Promise.all([
test_driver.update_virtual_sensor(testDriverName, readings.next().value),
sensorWatcher.wait_for('reading')
]);
assert_true(sensor.hasReading);
await sensorWatcher.wait_for('reading');

assert_sensor_reading_equals(sensor, expected);
// Make sure that 'timestamp' is already initialized.
Expand Down Expand Up @@ -607,7 +607,8 @@ function runGenericSensorTests(sensorData, readingData) {
const eventWatcher = new EventWatcher(t, sensor1, ['activate']);
sensor1.start();
await eventWatcher.wait_for('activate');
test_driver.update_virtual_sensor(testDriverName, readings.next().value);
await test_driver.update_virtual_sensor(
testDriverName, readings.next().value);
});
}, `${sensorName}: Readings delivered by shared platform sensor are\
immediately accessible to all sensors.`);
Expand Down

0 comments on commit effd91d

Please sign in to comment.