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

sensors: Ensure a document without an associated frame does not crash

Commit d1034e1e ("sensors: Make SensorProviderProxy supplement Document,
not LocalFrame") tied a sensor's lifetime to a document rather than a frame,
but we continued to assume Document::GetFrame() would never return null.

This is not true, as evidenced by the crash reports in bug 889754, caused by
SensorProxy::ShouldSuspendUpdates() trying to invoke methods on a LocalFrame
that can actually be a nullptr.

The original backtrace in the bug report seems to come from sensor creation,
but it is easier to trigger the same crash with a focus change after
destroying a sensor's document's frame.

Bug: 861675, 889754
Change-Id: Idb9ed5c18a655e113e2fb76cde6615aeefcc544a
Reviewed-on: https://chromium-review.googlesource.com/1256826Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#595958}
parent 54e089c6
This is a testharness.js-based test.
PASS Accelerometer: sensor is suspended and resumed when focus traverses from to cross-origin frame
PASS Accelerometer: sensor is not suspended when focus traverses from to same-origin frame
PASS Accelerometer: losing a document's frame with an active sensor does not crash
PASS LinearAccelerationSensor: sensor is suspended and resumed when focus traverses from to cross-origin frame
PASS LinearAccelerationSensor: sensor is not suspended when focus traverses from to same-origin frame
PASS LinearAccelerationSensor: losing a document's frame with an active sensor does not crash
FAIL GravitySensor: sensor is suspended and resumed when focus traverses from to cross-origin frame assert_true: expected true got false
FAIL GravitySensor: sensor is not suspended when focus traverses from to same-origin frame assert_true: expected true got false
FAIL GravitySensor: losing a document's frame with an active sensor does not crash assert_true: expected true got false
Harness: the test ran to completion.
......@@ -5,7 +5,6 @@ async function send_message_to_iframe(iframe, message, reply) {
return new Promise((resolve, reject) => {
let messageHandler = e => {
if (e.data.command !== message.command) {
return;
}
......@@ -131,4 +130,31 @@ function run_generic_sensor_iframe_tests(sensorName) {
iframe.parentNode.removeChild(iframe);
}, `${sensorName}: sensor is not suspended when focus traverses from\
to same-origin frame`);
sensor_test(async t => {
assert_true(sensorName in self);
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');
await send_message_to_iframe(iframe, {command: 'create_sensor',
type: sensorName});
iframe.contentWindow.focus();
await send_message_to_iframe(iframe, {command: 'start_sensor'});
// Remove iframe from main document and change focus. When focus changes,
// we need to determine whether a sensor must have its execution suspended
// or resumed (section 4.2.3, "Focused Area" of the Generic Sensor API
// spec). In Blink, this involves querying a frame, which might no longer
// exist at the time of the check.
// Note that we cannot send the "reset_sensor_backend" command because the
// iframe is discarded with the removeChild call.
iframe.parentNode.removeChild(iframe);
window.focus();
}, `${sensorName}: losing a document's frame with an active sensor does not crash`);
}
This is a testharness.js-based test.
FAIL GeolocationSensor: sensor is suspended and resumed when focus traverses from to cross-origin frame assert_true: expected true got false
FAIL GeolocationSensor: sensor is not suspended when focus traverses from to same-origin frame assert_true: expected true got false
FAIL GeolocationSensor: losing a document's frame with an active sensor does not crash assert_true: expected true got false
Harness: the test ran to completion.
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
FAIL UncalibratedMagnetometer: sensor is suspended and resumed when focus traverses from to cross-origin frame assert_true: expected true got false
FAIL UncalibratedMagnetometer: sensor is not suspended when focus traverses from to same-origin frame assert_true: expected true got false
FAIL UncalibratedMagnetometer: losing a document's frame with an active sensor does not crash assert_true: expected true got false
Harness: the test ran to completion.
......@@ -115,10 +115,11 @@ bool SensorProxy::ShouldSuspendUpdates() const {
return true;
LocalFrame* focused_frame = GetPage()->GetFocusController().FocusedFrame();
if (!focused_frame)
LocalFrame* this_frame = provider_->GetSupplementable()->GetFrame();
if (!focused_frame || !this_frame)
return true;
LocalFrame* this_frame = provider_->GetSupplementable()->GetFrame();
if (focused_frame == this_frame)
return false;
......
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