Skip to content

Commit

Permalink
sensors: Cleanup iframe-related tests. (#25626)
Browse files Browse the repository at this point in the history
Even after r807421 those tests are presenting some occasional flakiness in
the form of timeouts and, in a few cases, crashes in other parts of the
stack.

Clean up the code a little bit to make the code easier to follow and perhaps
reduce the chance of timeouts:
* send_message_to_frame() does not need to be an async function.
* Use t.add_cleanup() to a few tests to make sure any iframe we add is
  properly removed even if an error occurs.
* Remove all calls to the assert_*() functions from both the code path used
  by initialize_generic_sensor_tests() as well as generic_sensor_mocks.js,
  so that we no longer need to include testharness.js in the iframe handler
  page.
* Make the onmessage handler in the iframe page also send a reply when
  errors occur.

Bug: 1073865
Change-Id: Id6f682c95212d7ad9662f8aadad31c866752b0af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418333
Auto-Submit: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808497}

Co-authored-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
  • Loading branch information
chromium-wpt-export-bot and rakuco committed Sep 21, 2020
1 parent 7445e5a commit 9fa293b
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 55 deletions.
18 changes: 9 additions & 9 deletions generic-sensor/generic-sensor-iframe-tests.sub.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
async function send_message_to_iframe(iframe, message, reply) {
function send_message_to_iframe(iframe, message, reply) {
if (reply === undefined) {
reply = 'success';
}
Expand Down Expand Up @@ -32,6 +32,10 @@ function run_generic_sensor_iframe_tests(sensorName) {
// Create sensor inside cross-origin nested browsing context.
const iframeLoadWatcher = new EventWatcher(t, iframe, 'load');
document.body.appendChild(iframe);
t.add_cleanup(async () => {
await send_message_to_iframe(iframe, { command: 'reset_sensor_backend' });
iframe.parentNode.removeChild(iframe);
});
await iframeLoadWatcher.wait_for('load');
await send_message_to_iframe(iframe, {command: 'create_sensor',
type: sensorName});
Expand All @@ -49,7 +53,6 @@ function run_generic_sensor_iframe_tests(sensorName) {
// the top level browsing context are suspended.
iframe.contentWindow.focus();
await send_message_to_iframe(iframe, {command: 'start_sensor'});
assert_equals(cachedTimeStamp, sensor.timestamp);

// Focus on the main frame, verify that sensor reading updates are resumed.
window.focus();
Expand All @@ -59,10 +62,6 @@ function run_generic_sensor_iframe_tests(sensorName) {

// Verify that sensor in cross-origin frame is suspended.
await send_message_to_iframe(iframe, {command: 'is_sensor_suspended'}, true);
await send_message_to_iframe(iframe, {command: 'reset_sensor_backend'});

// Remove iframe from main document.
iframe.parentNode.removeChild(iframe);
}, `${sensorName}: sensor is suspended and resumed when focus traverses from\
to cross-origin frame`);

Expand All @@ -75,6 +74,10 @@ function run_generic_sensor_iframe_tests(sensorName) {
// Create sensor inside same-origin nested browsing context.
const iframeLoadWatcher = new EventWatcher(t, iframe, 'load');
document.body.appendChild(iframe);
t.add_cleanup(async () => {
await send_message_to_iframe(iframe, { command: 'reset_sensor_backend' });
iframe.parentNode.removeChild(iframe);
});
await iframeLoadWatcher.wait_for('load');
await send_message_to_iframe(iframe, {command: 'create_sensor',
type: sensorName});
Expand Down Expand Up @@ -124,9 +127,6 @@ function run_generic_sensor_iframe_tests(sensorName) {
assert_greater_than(sensor.timestamp, cachedTimeStamp);
sensor.stop();
await send_message_to_iframe(iframe, {command: 'reset_sensor_backend'});

// Remove iframe from main document.
iframe.parentNode.removeChild(iframe);
}, `${sensorName}: sensor is not suspended when focus traverses from\
to same-origin frame`);

Expand Down
1 change: 0 additions & 1 deletion generic-sensor/resources/generic-sensor-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ async function initialize_generic_sensor_tests() {
await loadChromiumResources();
}
}
assert_implements(GenericSensorTest, 'GenericSensorTest is unavailable.');

let sensorTest = new GenericSensorTest();
await sensorTest.initialize();
Expand Down
76 changes: 41 additions & 35 deletions generic-sensor/resources/iframe_sensor_handler.html
Original file line number Diff line number Diff line change
@@ -1,62 +1,68 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>iframe sensor tester</title>
<script src="/resources/testharness.js"></script>
<script src="/generic-sensor/resources/generic-sensor-helpers.js"></script>
<script>
let mockBackend = null;
let sensor = null;
let sensorType = null;

window.onmessage = (e) => {
function postReply(event, reply) {
event.source.postMessage({ command: event.data.command, result: reply }, '*');
}

window.onmessage = async (e) => {
if (e.data.command === 'create_sensor') {
assert_equals(sensor, null);
initialize_generic_sensor_tests().then((backend) => {
mockBackend = backend;
try {
sensor = new self[e.data.type]();
sensorType = e.data.type;
e.source.postMessage({command: e.data.command,
result: 'success'}, '*');
} catch (error) {
e.source.postMessage({command: e.data.command, result: error}, '*');
}
});
if (sensor) {
postReply(e, 'success');
return;
}

try {
mockBackend = await initialize_generic_sensor_tests();
sensor = new self[e.data.type]();
sensorType = e.data.type;

postReply(e, 'success');
} catch (error) {
postReply(e, error);
}
} else if (e.data.command === 'start_sensor') {
assert_not_equals(sensor, null);
if (!sensor) {
postReply(e, '"create_sensor" must be called first');
return;
}

try {
sensor.addEventListener('reading', () => {
postReply(e, 'success');
}, { once: true });
sensor.start();
let onReadingListener = () => {
e.source.postMessage({command: e.data.command,
result: 'success'}, '*');
}
sensor.addEventListener('reading', onReadingListener, {once: true});
} catch (error) {
e.source.postMessage({command: e.data.command, result: error}, '*');
postReply(e, error);
}
} else if (e.data.command === 'is_sensor_suspended') {
if (!mockBackend) {
e.source.postMessage({
command: e.data.command,
result: '"create_sensor" must be called first'
}, '*');
postReply(e, '"create_sensor" must be called first');
return;
}

mockBackend.getSensorProvider().getCreatedSensor(sensorType).then(mockPlatformSensor => {
e.source.postMessage({
command: e.data.command,
result: !mockPlatformSensor.isReadingData()
}, '*');
});
try {
const mockPlatformSensor = await mockBackend.getSensorProvider().getCreatedSensor(sensorType);
postReply(e, !mockPlatformSensor.isReadingData());
} catch (error) {
postReply(e, error);
}
} else if (e.data.command === 'reset_sensor_backend') {
if (sensor) {
sensor.stop();
await mockBackend.reset();

sensor = null;
mockBackend = null;
}
mockBackend.reset().then(() => {
e.source.postMessage({command: e.data.command,
result: 'success'}, '*');
});

postReply(e, 'success');
}
}
</script>
29 changes: 19 additions & 10 deletions resources/chromium/generic_sensor_mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ var GenericSensorTest = (() => {
this.readingData_ = null;
this.requestedFrequencies_ = [];
let rv = handle.mapBuffer(offset, size);
assert_equals(rv.result, Mojo.RESULT_OK, "Failed to map shared buffer");
if (rv.result != Mojo.RESULT_OK) {
throw new Error("MockSensor(): Failed to map shared buffer");
}
this.buffer_ = new Float64Array(rv.buffer);
this.buffer_.fill(0);
this.binding_ = new mojo.Binding(device.mojom.Sensor, this,
Expand All @@ -64,8 +66,6 @@ var GenericSensorTest = (() => {
// Adds configuration for the sensor and starts reporting fake data
// through setSensorReading function.
async addConfiguration(configuration) {
assert_not_equals(configuration, null, "Invalid sensor configuration.");

this.requestedFrequencies_.push(configuration.frequency);
// Sort using descending order.
this.requestedFrequencies_.sort(
Expand Down Expand Up @@ -151,8 +151,10 @@ var GenericSensorTest = (() => {
// |buffer_| is a TypedArray, so we need to make sure pass an
// array to set().
const reading = this.readingData_.next().value;
assert_true(Array.isArray(reading), "The readings passed to " +
"setSensorReading() must be arrays.");
if (!Array.isArray(reading)) {
throw new TypeError("startReading(): The readings passed to " +
"setSensorReading() must be arrays");
}
this.buffer_.set(reading, 2);
}
// For all tests sensor reading should have monotonically
Expand All @@ -173,7 +175,9 @@ var GenericSensorTest = (() => {
}

getSamplingFrequency() {
assert_true(this.requestedFrequencies_.length > 0);
if (this.requestedFrequencies_.length == 0) {
throw new Error("getSamplingFrequency(): No configured frequency");
}
return this.requestedFrequencies_[0];
}

Expand All @@ -191,7 +195,9 @@ var GenericSensorTest = (() => {
this.sharedBufferSizeInBytes_ = this.readingSizeInBytes_ *
(device.mojom.SensorType.MAX_VALUE + 1);
const rv = Mojo.createSharedBuffer(this.sharedBufferSizeInBytes_);
assert_equals(rv.result, Mojo.RESULT_OK, "Failed to create buffer");
if (rv.result != Mojo.RESULT_OK) {
throw new Error("MockSensorProvider: Failed to map shared buffer");
}
this.sharedBufferHandle_ = rv.handle;
this.activeSensors_ = new Map();
this.resolveFuncs_ = new Map();
Expand Down Expand Up @@ -251,8 +257,9 @@ var GenericSensorTest = (() => {
}

const rv = this.sharedBufferHandle_.duplicateBufferHandle();

assert_equals(rv.result, Mojo.RESULT_OK);
if (rv.result != Mojo.RESULT_OK) {
throw new Error("getSensor(): failed to duplicate Mojo buffer handler");
}

const defaultConfig = { frequency: DEFAULT_FREQUENCY };
// Consider sensor traits to meet assertions in C++ code (see
Expand Down Expand Up @@ -337,7 +344,9 @@ var GenericSensorTest = (() => {
// Returns mock sensor that was created in getSensor to the layout test.
getCreatedSensor(sensorType) {
const type = this.mojomSensorType_.get(sensorType);
assert_equals(typeof type, "number", "A sensor type must be specified.");
if (typeof type != "number") {
throw new TypeError(`getCreatedSensor(): Invalid sensor type ${sensorType}`);
}

if (this.activeSensors_.has(type)) {
return Promise.resolve(this.activeSensors_.get(type));
Expand Down

0 comments on commit 9fa293b

Please sign in to comment.