From 667c4ab710f06cc099078a0d951200b1d52053ad Mon Sep 17 00:00:00 2001 From: Raphael Kubo da Costa Date: Tue, 25 Jan 2022 11:13:21 +0000 Subject: [PATCH] sensors: Do nothing in Sensor.start() when the document is not fully active. When a sensor is created on e.g. an iframe that is later removed from its parent via removeChild(), we end up in a situation where a sensor instance did get created but which does not have a valid ExecutionContext by the time start() is invoked. Check for a valid ExecutionContext when start() is called and bail out early if it is null. https://github.com/w3c/sensors/issues/415 tracks handling non-fully active documents from a spec perspective; once that one is fixed we should probably throw an error in this case rather than silently doing nothing. Bug: 1289924 Change-Id: I2b033252d93347ba7c91385bdb510b69b8298aa2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3412476 Reviewed-by: Reilly Grant Commit-Queue: Raphael Kubo Da Costa Cr-Commit-Position: refs/heads/main@{#962930} NOKEYCHECK=True GitOrigin-RevId: b93b733b928610c36c328a4685c2129ed8a80e7b --- blink/renderer/modules/sensor/sensor.cc | 2 ++ .../generic-sensor-iframe-tests.sub.js | 28 +++++++++++++++++++ ...ionSensor-iframe-access.https-expected.txt | 1 + ...netometer-iframe-access.https-expected.txt | 2 ++ 4 files changed, 33 insertions(+) diff --git a/blink/renderer/modules/sensor/sensor.cc b/blink/renderer/modules/sensor/sensor.cc index 98f72dbe1486..002aa4245fa3 100644 --- a/blink/renderer/modules/sensor/sensor.cc +++ b/blink/renderer/modules/sensor/sensor.cc @@ -91,6 +91,8 @@ Sensor::Sensor(ExecutionContext* execution_context, Sensor::~Sensor() = default; void Sensor::start() { + if (!GetExecutionContext()) + return; if (state_ != SensorState::kIdle) return; state_ = SensorState::kActivating; diff --git a/blink/web_tests/external/wpt/generic-sensor/generic-sensor-iframe-tests.sub.js b/blink/web_tests/external/wpt/generic-sensor/generic-sensor-iframe-tests.sub.js index 11c9f50f4ab6..1d1a012380f6 100644 --- a/blink/web_tests/external/wpt/generic-sensor/generic-sensor-iframe-tests.sub.js +++ b/blink/web_tests/external/wpt/generic-sensor/generic-sensor-iframe-tests.sub.js @@ -155,4 +155,32 @@ function run_generic_sensor_iframe_tests(sensorName) { iframe.parentNode.removeChild(iframe); window.focus(); }, `${sensorName}: losing a document's frame with an active sensor does not crash`); + + sensor_test(async t => { + assert_implements(sensorName in self, `${sensorName} is not supported.`); + const iframe = document.createElement('iframe'); + iframe.allow = featurePolicies.join(';') + ';'; + iframe.src = 'https://{{host}}:{{ports[https][0]}}/generic-sensor/resources/iframe_sensor_handler.html'; + + // Create sensor in the iframe (we do not care whether this is a + // cross-origin nested context in this test). + const iframeLoadWatcher = new EventWatcher(t, iframe, 'load'); + document.body.appendChild(iframe); + await iframeLoadWatcher.wait_for('load'); + + // The purpose of this message is to initialize the mock backend in the + // iframe. We are not going to use the sensor created there. + await send_message_to_iframe(iframe, {command: 'create_sensor', + type: sensorName}); + + const iframeSensor = new iframe.contentWindow[sensorName](); + assert_not_equals(iframeSensor, null); + + // Remove iframe from main document. |iframeSensor| no longer has a + // non-null browsing context. Calling start() should probably throw an + // error when called from a non-fully active document, but that depends on + // https://github.com/w3c/sensors/issues/415 + iframe.parentNode.removeChild(iframe); + iframeSensor.start(); + }, `${sensorName}: calling start() in a non-fully active document does not crash`); } diff --git a/blink/web_tests/external/wpt/geolocation-sensor/GeolocationSensor-iframe-access.https-expected.txt b/blink/web_tests/external/wpt/geolocation-sensor/GeolocationSensor-iframe-access.https-expected.txt index 821f00d1a8ce..9adf77affdd3 100644 --- a/blink/web_tests/external/wpt/geolocation-sensor/GeolocationSensor-iframe-access.https-expected.txt +++ b/blink/web_tests/external/wpt/geolocation-sensor/GeolocationSensor-iframe-access.https-expected.txt @@ -2,5 +2,6 @@ This is a testharness.js-based test. FAIL GeolocationSensor: sensor is suspended and resumed when focus traverses from to cross-origin frame assert_implements: GeolocationSensor is not supported. undefined FAIL GeolocationSensor: sensor is not suspended when focus traverses from to same-origin frame assert_implements: GeolocationSensor is not supported. undefined FAIL GeolocationSensor: losing a document's frame with an active sensor does not crash assert_implements: GeolocationSensor is not supported. undefined +FAIL GeolocationSensor: calling start() in a non-fully active document does not crash assert_implements: GeolocationSensor is not supported. undefined Harness: the test ran to completion. diff --git a/blink/web_tests/external/wpt/magnetometer/Magnetometer-iframe-access.https-expected.txt b/blink/web_tests/external/wpt/magnetometer/Magnetometer-iframe-access.https-expected.txt index 35f075392c02..6b161d3577ba 100644 --- a/blink/web_tests/external/wpt/magnetometer/Magnetometer-iframe-access.https-expected.txt +++ b/blink/web_tests/external/wpt/magnetometer/Magnetometer-iframe-access.https-expected.txt @@ -2,8 +2,10 @@ This is a testharness.js-based test. PASS Magnetometer: sensor is suspended and resumed when focus traverses from to cross-origin frame PASS Magnetometer: sensor is not suspended when focus traverses from to same-origin frame PASS Magnetometer: losing a document's frame with an active sensor does not crash +PASS Magnetometer: calling start() in a non-fully active document does not crash FAIL UncalibratedMagnetometer: sensor is suspended and resumed when focus traverses from to cross-origin frame assert_implements: UncalibratedMagnetometer is not supported. undefined FAIL UncalibratedMagnetometer: sensor is not suspended when focus traverses from to same-origin frame assert_implements: UncalibratedMagnetometer is not supported. undefined FAIL UncalibratedMagnetometer: losing a document's frame with an active sensor does not crash assert_implements: UncalibratedMagnetometer is not supported. undefined +FAIL UncalibratedMagnetometer: calling start() in a non-fully active document does not crash assert_implements: UncalibratedMagnetometer is not supported. undefined Harness: the test ran to completion.