commit ecf7b95fb7122554c6a226584e0b18e7ad0e865b
author Reilly Grant <reillyg@chromium.org> Wed Apr 13 22:26:16 2022
committer Blink WPT Bot <blink-w3c-test-autoroller@chromium.org> Wed Apr 13 22:33:15 2022
tree 72b6b98b15c0e3a6ecdf7d9a7a231afb57c0f00d
parent 45d050aaa685ac540ffca065e2f64d7b21c89558

idle-detection: Reject start Promise on abort

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}

idle-detection/interceptor.https.html

diff --git a/idle-detection/interceptor.https.html b/idle-detection/interceptor.https.html
index 1d561f8..a710146 100644
--- a/idle-detection/interceptor.https.html
+++ b/idle-detection/interceptor.https.html
@@ -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;
 
@@ -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) => {
@@ -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) => {
@@ -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();
 
@@ -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>