DevTools: Fix flaky worker-autoattach-order test

There are several ways this test can flake the way it's written.
Use a handler for AttachedToTarget instead of onceAttachedToTarget to
avoid the ordering problems there.

Use evaluateAsync to actually wait for the Promise.all call to
resolve.

We also can't make any assumption about the resolved status of
setAutoAttach except that it is resolved after the two attached
events are fired - which is what we are trying to test.
So just rely on the ordering of the output to prove that it is
resolved after the two events arrive.

Drive-by fix to two other tests which used onceAttachedToTarget
too late and could flake if the event fires after evaluate()
but before the event handler is added.

Fixed: 915472
Change-Id: Id4a20488273a0b1a963e732c14e38b5efcd094ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1943170Reviewed-by: Mathias Bynens <mathias@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720156}

third_party/blink/web_tests/TestExpectations

@@ -94,8 +94,6 @@ crbug.com/762529 http/tests/devtools/network/network-datareceived.js [ Failure ]
 crbug.com/759632 http/tests/devtools/network/network-datasaver-warning.js [ Failure ]
 # ====== Browserside navigation until here ======
 
-crbug.com/915472 inspector-protocol/worker/worker-autoattach-order.js [ Failure Pass ]
-
 # ====== Paint team owned tests from here ======
 # The paint team tracks and triages its test failures, and keeping them colocated
 # makes tracking much easier.

third_party/blink/web_tests/inspector-protocol/debugger/suspend-setTimeout-on-pause-in-dedicated-worker.js

@@ -3,6 +3,7 @@
 
   await dp.Target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: true,
                            flatten: true});
+  const attachedPromise = dp.Target.onceAttachedToTarget();
   await session.evaluate(`
     window.worker = new Worker('${testRunner.url('resources/dedicated-worker-suspend-setTimeout.js')}');
     window.worker.onmessage = function(event) { };
@@ -10,7 +11,7 @@
   `);
   testRunner.log('Started worker');
 
-  const event = await dp.Target.onceAttachedToTarget();
+  const event = await attachedPromise;
   const childSession = session.createChild(event.params.sessionId);
   testRunner.log('Worker created');
 

third_party/blink/web_tests/inspector-protocol/target/target-send-message.js

 (async function(testRunner) {
   let {page, session, dp} = await testRunner.startBlank('Tests sendMessage with invalid message.');
 
+  // TODO(johannes): We plan to retire the non-flattened mode, in which
+  // case there's no need for this test. Or if we do want to keep it, it should
+  // use child sessions. See also crbug.com/991325.
+  await dp.Target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: false,
+    flatten: false});
+  const attachedPromise = dp.Target.onceAttachedToTarget();
   await session.evaluate(`
     window.worker = new Worker('${testRunner.url('../resources/worker-console-worker.js')}');
     window.worker.onmessage = function(event) { };
     window.worker.postMessage(1);
   `);
-
-  // TODO(johannes): We plan to retire the non-flattened mode, in which
-  // case there's no need for this test. Or if we do want to keep it, it should
-  // use child sessions. See also crbug.com/991325.
-  dp.Target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: false,
-                           flatten: false});
-  let {params:{sessionId}} = await dp.Target.onceAttachedToTarget();
+  let {params:{sessionId}} = await attachedPromise;
 
   let p = dp.Target.onceReceivedMessageFromTarget();
   testRunner.log('JSON syntax error..');

third_party/blink/web_tests/inspector-protocol/worker/worker-autoattach-order-expected.txt

-Target.setAutoAttach should report all workers before returning.
+Target.setAutoAttach should report all existing workers before returning.
 worker
 worker
-Before await. Resolved: false
-After await. Resolved: true
+setAutoAttach resolved
 

third_party/blink/web_tests/inspector-protocol/worker/worker-autoattach-order.js

 (async function(testRunner) {
   const {page, session, dp} =
       await testRunner.startBlank(
-          'Target.setAutoAttach should report all workers before returning.');
+          'Target.setAutoAttach should report all existing workers before returning.');
 
-  await session.evaluate(`
+  // Create two workers and wait until they are initialized. We wait until we
+  // can receive onmessage events although this might not be totally necessary.
+  await session.evaluateAsync(`
     const w1 = new Worker('${testRunner.url('../resources/worker-console-worker.js')}');
     const promise1 = new Promise(x => w1.onmessage = x);
     const w2 = new Worker('${testRunner.url('../resources/worker-console-worker.js')}');
@@ -11,22 +13,14 @@
     Promise.all([promise1, promise2]);
   `);
 
-  let autoAttachPromiseResolved = false;
+  dp.Target.onAttachedToTarget((event) => {
+    testRunner.log(event.params.targetInfo.type);
+  });
 
-  const autoAttachPromise = dp.Target.setAutoAttach({
-    autoAttach: true, waitForDebuggerOnStart: false, flatten: true}).then(
-        () => { autoAttachPromiseResolved = true; });
-
-  testRunner.log((await dp.Target.onceAttachedToTarget()).params.targetInfo.type);
-  testRunner.log((await dp.Target.onceAttachedToTarget()).params.targetInfo.type);
-
-  // Up to here, the promise from Target.setAutoAttach is still not resolved,
-  // meaning that we've received the attachedToTarget events before
-  // setAutoAttach has returned. We log this fact, and then show that our
-  // mechanism for testing (the autoAttachPromiseResolved variable) is
-  // working by awaiting and logging again.
-  testRunner.log('Before await. Resolved: ' + autoAttachPromiseResolved);
-  await autoAttachPromise;
-  testRunner.log('After await. Resolved: ' + autoAttachPromiseResolved);
+  // setAutoAttach should fire AttachedToTarget events for all existing workers
+  // before resolving. We check this in the ordering of the log output.
+  await dp.Target.setAutoAttach({
+    autoAttach: true, waitForDebuggerOnStart: false, flatten: true});
+  testRunner.log('setAutoAttach resolved');
   testRunner.completeTest();
 })