Ensure tests don't depend on fake devices being added synchronously

third_party/WebKit/LayoutTests/usb/usb.html

Index: third_party/WebKit/LayoutTests/usb/usb.html
diff --git a/third_party/WebKit/LayoutTests/usb/usb.html b/third_party/WebKit/LayoutTests/usb/usb.html
index 82bc0f5af3e447ef55ec6951685715093044c2ab..af46dfcabf32db145e11e8e34754a52ca3554aec 100644
--- a/third_party/WebKit/LayoutTests/usb/usb.html
+++ b/third_party/WebKit/LayoutTests/usb/usb.html
@@ -7,35 +7,29 @@
 <script>
 'use strict';
 
-// Returns a promise that is resolved when the next USBConnectionEvent of the
-// given type is received.
-function connectionEventPromise(eventType) {
-  return new Promise(resolve => {
-    let eventHandler = e => {
-      assert_true(e instanceof USBConnectionEvent);
-      navigator.usb.removeEventListener(eventType, eventHandler);
-      resolve(e.device);
-    };
-    navigator.usb.addEventListener(eventType, eventHandler);
-  });
-}
-
 usb_test(() => {
+  let promise = connectionEventPromise('connect');

[ortuno, 2017/04/12 03:31:07]

How high do you think this event is in the priority list of someone implementing
the API? We are blocking many tests on this feature and it would be unfortunate
if other browsers can't re-use these tests until this specific
maybe-not-that-important feature is implemented.

[Reilly Grant (use Gerrit), 2017/04/12 22:47:07]

As discussed offline I gave this a try but it requires guaranteeing that we can
flush an event through the system and I think that's a more complex thing to
implement correctly than the connection events.

   navigator.usb.test.addFakeDevice(fakeDeviceInit);
 
-  return navigator.usb.getDevices().then(devices => {
-    assert_equals(devices.length, 1);
-    assertDeviceInfoEquals(devices[0], fakeDeviceInit);
+  return promise.then(device => {

[ortuno, 2017/04/12 03:31:07]

Why are you nesting the promises? Why not:

return promise
  .then(() => navigator.usb.getDevices())
  .then(devices => {
    assert_equals(devices.length, 1);
    // ...
   });

Sometimes it is necessary to nest but IMO code is much easier to read if
promises are not nested.

https://pouchdb.com/2015/05/18/we-have-a-problem-with-promises.html

[Reilly Grant (use Gerrit), 2017/04/12 22:47:07]

I need |device| to available in the scope of the handler passed to the next
then().

+    return navigator.usb.getDevices().then(devices => {
+      assert_equals(devices.length, 1);
+      assert_equals(device, devices[0]);
+      assertDeviceInfoEquals(devices[0], fakeDeviceInit);
+    });
   });
-}, 'getDevices returns devices exposed by the DeviceManager service');
+}, 'getDevices returns devices that are connected');
 
 usb_test(() => {
+  let promise = connectionEventPromise('connect');

[ortuno, 2017/04/12 03:31:07]

Why not getConnectedDevice()?

[Reilly Grant (use Gerrit), 2017/04/12 22:47:07]

Done.

   navigator.usb.test.addFakeDevice(fakeDeviceInit);
 
-  return navigator.usb.getDevices().then(devicesFirstTime => {
-    assert_equals(devicesFirstTime.length, 1);
-    return navigator.usb.getDevices().then(devicesSecondTime => {
-      assert_array_equals(devicesSecondTime, devicesFirstTime);
+  return promise.then(device => {

[ortuno, 2017/04/12 03:31:07]

nit: You don't need `device`. You can replace it with `()` or `_`.

[Reilly Grant (use Gerrit), 2017/04/12 22:47:07]

Done.

+    return navigator.usb.getDevices().then(devicesFirstTime => {
+      assert_equals(devicesFirstTime.length, 1);
+      return navigator.usb.getDevices().then(devicesSecondTime => {
+        assert_array_equals(devicesSecondTime, devicesFirstTime);
+      });
     });
   });
 }, 'getDevices returns the same objects for each USB device');
@@ -62,26 +56,35 @@ usb_test(() => {
 }, 'requestDevice rejects when no device is chosen');
 
 usb_test(() => {
+  let promise = connectionEventPromise('connect');
   let guid = navigator.usb.test.addFakeDevice(fakeDeviceInit);
   navigator.usb.test.chosenDevice = guid;
 
-  return callWithKeyDown(() => navigator.usb.requestDevice({ filters: [] })
-    .then(device => {
-      assertDeviceInfoEquals(device, fakeDeviceInit);
-    })
-  );
+  return promise.then(device => {
+    return callWithKeyDown(() => {
+      return navigator.usb.requestDevice({ filters: [] }).then(chosenDevice => {
+        assert_equals(chosenDevice, device);
+      });
+    });
+  });
 }, 'requestDevice returns the device chosen by the user');
 
 usb_test(() => {
+  let promise = connectionEventPromise('connect');
   let guid = navigator.usb.test.addFakeDevice(fakeDeviceInit);
   navigator.usb.test.chosenDevice = guid;
 
-  return callWithKeyDown(() => navigator.usb.requestDevice({ filters: [] })
-      .then(device => navigator.usb.getDevices().then(devices => {
-        assert_equals(devices.length, 1);
-        assert_equals(devices[0], device);
-      }))
-  );
+  return promise.then(device => {
+    return callWithKeyDown(() => {
+      return navigator.usb.requestDevice({ filters: [] }).then(chosenDevice => {
+        assert_equals(chosenDevice, device);
+        return navigator.usb.getDevices().then(devices => {
+          assert_equals(devices.length, 1);
+          assert_equals(devices[0], chosenDevice);
+        });
+      });
+    });
+  });
 }, 'getDevices returns the same object as requestDevice');
 
 usb_test(() => {
@@ -115,17 +118,6 @@ usb_test(() => {
 }, 'onconnect event is trigged by adding a device');
 
 usb_test(usb => {
-  let promise = connectionEventPromise('connect');
-  navigator.usb.test.addFakeDevice(fakeDeviceInit);
-
-  return promise
-      .then(device => navigator.usb.getDevices().then(devices => {
-        assert_equals(devices.length, 1);
-        assert_equals(devices[0], device);
-      }));
-}, 'getDevices returns the same object as sent in the onconnect event');
-
-usb_test(usb => {
   let deviceAdded = connectionEventPromise('connect');
   let guid = navigator.usb.test.addFakeDevice(fakeDeviceInit);