Skip to content

Commit

Permalink
idle-detection: Reject start Promise on abort
Browse files Browse the repository at this point in the history
This change fixes the IdleDetector interface's start() method so that if
the provided AbortSignal is signaled before the Promise it returns
settles it will reject with the provided abort reason instead.

In addition to violating the specification this scenario created an
internal consistenty error which triggered a DCHECK in Update() but was
otherwise harmless. This issue was introduced by the original change
that moved IdleDetector to use an AbortSignal and made it so that the
`resolver_` field was never actually populated with the Promise returned
by start().

Tests have been added to more completely exercise the AbortSignal
behavior.

Bug: 1315755
Change-Id: I394587bddf2e8176ff3f691d7f33e494af364684
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3582549
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992258}
  • Loading branch information
reillyeon authored and chromium-wpt-export-bot committed Apr 13, 2022
1 parent 45d050a commit ecf7b95
Showing 1 changed file with 60 additions and 6 deletions.
66 changes: 60 additions & 6 deletions idle-detection/interceptor.https.html
Expand Up @@ -210,13 +210,13 @@
});

const controller = new AbortController();
const detector = new IdleDetector();
const detector = new IdleDetector({ signal: controller.signal });

const watcher = new EventWatcher(t, detector, ["change"]);
const initial_state = watcher.wait_for("change");

// Only the first call to start() is allowed.
const start_promise = detector.start();
const start_promise = detector.start({ signal: controller.signal });
await promise_rejects_dom(t, 'InvalidStateError', detector.start());
await start_promise;

Expand All @@ -229,7 +229,7 @@
controller.abort();
controller.abort();
controller.abort();
}, 'Safe to call start() or stop() multiple times');
}, 'Calling start() and abort() multiple times');

promise_test(async t => {
expect(addMonitor).andReturn(async (monitorPtr) => {
Expand All @@ -245,12 +245,64 @@
const controller = new AbortController();
const detector = new IdleDetector();

// Calling abort() before start() causes start() to fail.
controller.abort();

await promise_rejects_dom(
t, 'AbortError', detector.start({ signal: controller.signal }));
}, 'Calling stop() after start() is a no-op');
}, 'Calling abort() before start() makes it fail');

promise_test(async t => {
expect(addMonitor).andReturn(async (monitorPtr) => {
return {
error: IdleDetectorError.SUCCESS,
state: {
idleTime: null,
screenLocked: false
}
};
});

const controller = new AbortController();
const detector = new IdleDetector();

const promise = promise_rejects_dom(
t, 'AbortError', detector.start({ signal: controller.signal }))
controller.abort();

await promise;
}, 'Calling abort() after start() makes it fail');

promise_test(async t => {
expect(addMonitor).andReturn(async (monitorPtr) => {
return {
error: IdleDetectorError.SUCCESS,
state: {
idleTime: null,
screenLocked: false
}
};
});

const detector = new IdleDetector();
const watcher = new EventWatcher(t, detector, ["change"]);

let controller = new AbortController();
const first_start = promise_rejects_dom(
t, 'AbortError', detector.start({ signal: controller.signal }))
controller.abort();

controller = new AbortController();
const initial_state = watcher.wait_for("change");
const second_start = detector.start({ signal: controller.signal });

await first_start;
await second_start;
await initial_state;
assert_equals(detector.userState, "active");
assert_equals(detector.screenState, "unlocked");

controller.abort();
}, 'A start() that has been aborted can be retried');

promise_test(async t => {
expect(addMonitor).andReturn(async (monitorPtr) => {
Expand All @@ -270,6 +322,8 @@

await detector.start({ signal: controller.signal });
await initial_state;
assert_equals(detector.userState, "active");
assert_equals(detector.screenState, "unlocked");

controller.abort();

Expand All @@ -293,6 +347,6 @@
assert_equals(detector.screenState, "locked");

controller.abort();
}, 'Calling start() after stop(): re-starting monitor.');
}, 'Calling start() after abort(): re-starting monitor.');

</script>

0 comments on commit ecf7b95

Please sign in to comment.