Migrate WebUSB LayoutTests into external/wpt

Migrate WebUSB LayoutTests into external/wpt

This test moves all the tests from third_party/WebKit/LayoutTests/usb
to third_party/WebKit/LayoutTests/external/wpt/webusb. The WebUSB Test
API polyfill is included in a new resources/chromium directory and
loaded automatically when the tests are run in a Chromium-based browser
with the --enable-blink-features=MojoJS,MojoJSTest flag set.

BUG=705734
Review-Url: https://codereview.chromium.org/2789723003
Cr-Commit-Position: refs/heads/master@{#481732}
Committed: https://chromium.googlesource.com/chromium/src/+/a58aca05fb59a513a0857538de9e79eea712d4e4

[Reilly Grant (use Gerrit), 2017-03-31 01:22:45]

Patchset #1 (id:1) has been deleted

[Reilly Grant (use Gerrit), 2017-03-31 01:24:07]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 01:24:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Reilly Grant (use Gerrit), 2017-03-31 01:25:53]

reillyg@chromium.org changed reviewers:
+ foolip@chromium.org, ortuno@chromium.org

[Reilly Grant (use Gerrit), 2017-03-31 01:25:54]

I'm doing something new and interesting here so I'd appreciate folks taking a
careful look at it.

[commit-bot: I haz the power, 2017-03-31 02:27:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 02:27:19]

Dry run: This issue passed the CQ dry run.

[ortuno, 2017-04-03 01:47:18]

ortuno@chromium.org changed reviewers:
+ nzolghadr@chromium.org

[ortuno, 2017-04-03 01:47:19]

+nzolghadr for question about input test API.

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/fake-devices.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/fake-devices.js:4:
return define('Fake USB Devices', [
This is chrome specific so I don't think we can upstream it.

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:45:
// TODO(reillyg): Remove when jyasskin upstreams this to testharness.js:
I don't think we can upstream yet because of this TODO.

nzolghadr: How far along is the input injection test API?

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:59:
function runGarbageCollection() {
This is also Chromium specific. I think we will have to keep tests that use this
downstream for now and block their upstreaming on a standard GC Test API, which
might be a long shot. I can already imagine people arguing against it on the
premise that this types of tests should really be unit tests rather than Layout
Tests.

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/resources/testharnessreport.js (right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/resources/testharnessreport.js:135: var
automationPath = location.pathname.replace(/\/external\/wpt\/.*$/,
'/external/wpt_automation');
wpt_automation has a very specific purpose: Turn manual tests into automated
tests. Given that these tests are not manual tests I don't think we should put
our polyfills there.

[Navid Zolghadr, 2017-04-03 14:24:34]

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:45:
// TODO(reillyg): Remove when jyasskin upstreams this to testharness.js:
On 2017/04/03 01:47:19, ortuno wrote:
> I don't think we can upstream yet because of this TODO.
> 
> nzolghadr: How far along is the input injection test API?

I'm trying to push it through this PR.
https://github.com/w3c/testharness.js/pull/251

Although I didn't have any API for injecting key inputs as we don't have any way
of injecting it before IME. eventSender sends it after those filters and does
not test the whole pipeline. I'll talk about it with others and consider adding
a key injection API as well. I'm hoping to get it in by the end of this week.
But again I'm trying to convince people to have something like this there.

However, I'd like to unblock you guys as soon as I can. Maybe I can explain a
little better in a meeting. But would a structure similar to what we have in wpt
tests right now help?

https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/...

We also need some input from users and for now those tests are ending with
"-manual" in their name. Then we are using our Chrome APIs to automate them
through those "-automation.js" files on our bots.

[ortuno, 2017-04-03 23:22:30]

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:45:
// TODO(reillyg): Remove when jyasskin upstreams this to testharness.js:
On 2017/04/03 at 14:24:34, Navid Zolghadr wrote:
> On 2017/04/03 01:47:19, ortuno wrote:
> > I don't think we can upstream yet because of this TODO.
> > 
> > nzolghadr: How far along is the input injection test API?
> 
> I'm trying to push it through this PR.
> https://github.com/w3c/testharness.js/pull/251
> 
> Although I didn't have any API for injecting key inputs as we don't have any
way of injecting it before IME. eventSender sends it after those filters and
does not test the whole pipeline. I'll talk about it with others and consider
adding a key injection API as well. I'm hoping to get it in by the end of this
week. But again I'm trying to convince people to have something like this there.
> 

We don't necessarily need injecting key inputs. The current API as proposed
should be enough.

> However, I'd like to unblock you guys as soon as I can. Maybe I can explain a
little better in a meeting. But would a structure similar to what we have in wpt
tests right now help?
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/...
> 
> We also need some input from users and for now those tests are ending with
"-manual" in their name. Then we are using our Chrome APIs to automate them
through those "-automation.js" files on our bots.

I would like to avoid having a "-manual" suffix for pretty much all of our
tests. Many new APIs require a user gesture which means that every single one of
our tests would end up with a "-manual" suffix. Ideally the API would just be
there or fail gracefully. For instance Bluetooth has 200+ tests, most of which
require a user gesture. Adding "-manual" to them would just generate noise.

[foolip, 2017-04-05 06:03:32]

It looks like the automation bits are in fake-devices.js, usb-helpers.js and
webusb-test.js.

There is a relevant thread on public-test-infra that's split over the month
boundary:
https://lists.w3.org/Archives/Public/public-test-infra/2017JanMar/thread.html...
https://lists.w3.org/Archives/Public/public-test-infra/2017AprJun/thread.html...

I think we have roughly two paths to choose between:

1. Have specs define their testing APIs and let web-platform-tests assume that
they are implemented. This is the closest to this CL. Something like the
testharnessreport.js additions would do it, although we wouldn't put any of the
implementation in upstream web-platform-tests.

2. Extend WebDriver to support mocking of USB devices. I don't know how much
work this is in practice but assume it would be more work than defining APIs on
top of mojo as has been done recently for many features.

If all of these tests were made into manual tests, they could be upstreamed
right now, but I don't think that's an approach that will scale very well.

What would be very helpful is to get feedback from other vendors interested in
implementing WebUSB about how they would prefer to test it. Are the any such
vendors?

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:45:
// TODO(reillyg): Remove when jyasskin upstreams this to testharness.js:
On 2017/04/03 23:22:30, ortuno wrote:
> On 2017/04/03 at 14:24:34, Navid Zolghadr wrote:
> > On 2017/04/03 01:47:19, ortuno wrote:
> > > I don't think we can upstream yet because of this TODO.
> > > 
> > > nzolghadr: How far along is the input injection test API?
> > 
> > I'm trying to push it through this PR.
> > https://github.com/w3c/testharness.js/pull/251
> > 
> > Although I didn't have any API for injecting key inputs as we don't have any
> way of injecting it before IME. eventSender sends it after those filters and
> does not test the whole pipeline. I'll talk about it with others and consider
> adding a key injection API as well. I'm hoping to get it in by the end of this
> week. But again I'm trying to convince people to have something like this
there.
> > 
> 
> We don't necessarily need injecting key inputs. The current API as proposed
> should be enough.
> 
> > However, I'd like to unblock you guys as soon as I can. Maybe I can explain
a
> little better in a meeting. But would a structure similar to what we have in
wpt
> tests right now help?
> > 
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/...
> > 
> > We also need some input from users and for now those tests are ending with
> "-manual" in their name. Then we are using our Chrome APIs to automate them
> through those "-automation.js" files on our bots.
> 
> I would like to avoid having a "-manual" suffix for pretty much all of our
> tests. Many new APIs require a user gesture which means that every single one
of
> our tests would end up with a "-manual" suffix. Ideally the API would just be
> there or fail gracefully. For instance Bluetooth has 200+ tests, most of which
> require a user gesture. Adding "-manual" to them would just generate noise.

It looks like all the calls to callWithKeyDown() were removed in this CL, at
least it looks like that when searching the raw diff. Are there other parts of
the helpers that are unused and can be removed?

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt_automation/webusb/webusb-test.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/webusb/webusb-test.js:3:
// This polyfil library implements the following WebIDL:
Is this test API included in some spec? It's not in
https://wicg.github.io/webusb/ itself.

[Reilly Grant (use Gerrit), 2017-04-05 18:39:16]

Addressed comments from ortuno@ and foolip@

[Reilly Grant (use Gerrit), 2017-04-05 18:39:26]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[Reilly Grant (use Gerrit), 2017-04-05 18:39:37]

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/fake-devices.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/fake-devices.js:4:
return define('Fake USB Devices', [
On 2017/04/03 01:47:19, ortuno wrote:
> This is chrome specific so I don't think we can upstream it. 

Good catch. I forgot to remove that when I switched all the tests to
fakeDeviceInit.

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:45:
// TODO(reillyg): Remove when jyasskin upstreams this to testharness.js:
On 2017/04/05 06:03:32, foolip_UTC7 wrote:
> On 2017/04/03 23:22:30, ortuno wrote:
> > On 2017/04/03 at 14:24:34, Navid Zolghadr wrote:
> > > On 2017/04/03 01:47:19, ortuno wrote:
> > > > I don't think we can upstream yet because of this TODO.
> > > > 
> > > > nzolghadr: How far along is the input injection test API?
> > > 
> > > I'm trying to push it through this PR.
> > > https://github.com/w3c/testharness.js/pull/251
> > > 
> > > Although I didn't have any API for injecting key inputs as we don't have
any
> > way of injecting it before IME. eventSender sends it after those filters and
> > does not test the whole pipeline. I'll talk about it with others and
consider
> > adding a key injection API as well. I'm hoping to get it in by the end of
this
> > week. But again I'm trying to convince people to have something like this
> there.
> > > 
> > 
> > We don't necessarily need injecting key inputs. The current API as proposed
> > should be enough.
> > 
> > > However, I'd like to unblock you guys as soon as I can. Maybe I can
explain
> a
> > little better in a meeting. But would a structure similar to what we have in
> wpt
> > tests right now help?
> > > 
> > >
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/...
> > > 
> > > We also need some input from users and for now those tests are ending with
> > "-manual" in their name. Then we are using our Chrome APIs to automate them
> > through those "-automation.js" files on our bots.
> > 
> > I would like to avoid having a "-manual" suffix for pretty much all of our
> > tests. Many new APIs require a user gesture which means that every single
one
> of
> > our tests would end up with a "-manual" suffix. Ideally the API would just
be
> > there or fail gracefully. For instance Bluetooth has 200+ tests, most of
which
> > require a user gesture. Adding "-manual" to them would just generate noise.
> 
> It looks like all the calls to callWithKeyDown() were removed in this CL, at
> least it looks like that when searching the raw diff. Are there other parts of
> the helpers that are unused and can be removed?

callWithKeyDown() is used when testing navigator.usb.requestDevice(). This is
only done in usb.https.html so I'm looking at making that a manual test and
using our existing automation scripts.

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:59:
function runGarbageCollection() {
On 2017/04/03 01:47:19, ortuno wrote:
> This is also Chromium specific. I think we will have to keep tests that use
this
> downstream for now and block their upstreaming on a standard GC Test API,
which
> might be a long shot. I can already imagine people arguing against it on the
> premise that this types of tests should really be unit tests rather than
Layout
> Tests.

I've moved the one test that used this back to LayoutTests/usb.

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt_automation/webusb/webusb-test.js
(right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/webusb/webusb-test.js:3:
// This polyfil library implements the following WebIDL:
On 2017/04/05 06:03:32, foolip_UTC7 wrote:
> Is this test API included in some spec? It's not in
> https://wicg.github.io/webusb/ itself.

This test API is not yet included in any spec. The purpose of this patch is to
prove out the concept of polyfilling this kind of test API. If people approve I
will go write the spec. It would be similar to,

https://webbluetoothcg.github.io/web-bluetooth/tests

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/resources/testharnessreport.js (right):

https://codereview.chromium.org/2789723003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/resources/testharnessreport.js:135: var
automationPath = location.pathname.replace(/\/external\/wpt\/.*$/,
'/external/wpt_automation');
On 2017/04/03 01:47:19, ortuno wrote:
> wpt_automation has a very specific purpose: Turn manual tests into automated
> tests. Given that these tests are not manual tests I don't think we should put
> our polyfills there.

Created a new wpt_polyfill directory for these.

[commit-bot: I haz the power, 2017-04-05 18:40:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 20:03:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-05 20:03:41]

Dry run: This issue passed the CQ dry run.

[ortuno, 2017-04-06 01:14:06]

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/SlowTests (right):

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/SlowTests:352: crbug.com/592183
external/wpt/webusb/usbDevice.https.html [ Slow ]
We might have to separate the test to upstream them[1] but even if not I would
urge to consider separating them so they can be parallelized both locally and
when swarming.

[1] https://github.com/w3c/web-platform-tests/issues/5393

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html
(right):

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:1:
<!DOCTYPE html>
I don't think the "-manual" suffix pattern will scale. Most of our new APIs
require a user gesture and adding -manual to them will just generate noise imo.
I don't think this is too big of a deal for USB but it might for bluetooth which
has ~200 tests. Also not necessarily something we need to fix in this patch.

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:23:
navigator.usb.test.addFakeDevice(fakeDeviceInit);
In the vein of designing our tests APIs in a way that all browsers can implement
them I wonder if making these synchronous is rather limiting and will be a
problem for other browsers to implement.

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:53:
return callWithTrustedClick(() => navigator.usb.requestDevice({ filters: [] })
hmm callWithTrustedClick just adds a button and sets up a listener but it
doesn't actually click on that button so this would never run. What am I
missing?

[Reilly Grant (use Gerrit), 2017-04-06 01:52:32]

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html
(right):

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:1:
<!DOCTYPE html>
On 2017/04/06 01:14:06, ortuno wrote:
> I don't think the "-manual" suffix pattern will scale. Most of our new APIs
> require a user gesture and adding -manual to them will just generate noise
imo.
> I don't think this is too big of a deal for USB but it might for bluetooth
which
> has ~200 tests. Also not necessarily something we need to fix in this patch.

I can get away with it in these tests because the polyfill doesn't check that
the user has granted permission for the device. I agree we should have some
sanctioned WPT method for generating a trusted click.

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:23:
navigator.usb.test.addFakeDevice(fakeDeviceInit);
On 2017/04/06 01:14:06, ortuno wrote:
> In the vein of designing our tests APIs in a way that all browsers can
implement
> them I wonder if making these synchronous is rather limiting and will be a
> problem for other browsers to implement.

This call doesn't guarantee that anything happens synchronously, just that the
browser eventually adds a new fake device matching the provided parameters.

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:53:
return callWithTrustedClick(() => navigator.usb.requestDevice({ filters: [] })
On 2017/04/06 01:14:06, ortuno wrote:
> hmm callWithTrustedClick just adds a button and sets up a listener but it
> doesn't actually click on that button so this would never run. What am I
> missing?

I include wpt_automation/fullscreen/auto-click.js which automatically clicks on
any button I add to the DOM.

[ortuno, 2017-04-06 02:51:47]

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html
(right):

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:23:
navigator.usb.test.addFakeDevice(fakeDeviceInit);
On 2017/04/06 at 01:52:32, Reilly Grant wrote:
> On 2017/04/06 01:14:06, ortuno wrote:
> > In the vein of designing our tests APIs in a way that all browsers can
implement
> > them I wonder if making these synchronous is rather limiting and will be a
> > problem for other browsers to implement.
> 
> This call doesn't guarantee that anything happens synchronously, just that the
browser eventually adds a new fake device matching the provided parameters.

Interesting, then how do you guarantee that the device will be returned when
getDevices() is called? It might be that the device hasn't been added by the
time getDevices() is called causing the test to fail right?

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:53:
return callWithTrustedClick(() => navigator.usb.requestDevice({ filters: [] })
On 2017/04/06 at 01:52:32, Reilly Grant wrote:
> On 2017/04/06 01:14:06, ortuno wrote:
> > hmm callWithTrustedClick just adds a button and sets up a listener but it
> > doesn't actually click on that button so this would never run. What am I
> > missing?
> 
> I include wpt_automation/fullscreen/auto-click.js which automatically clicks
on any button I add to the DOM.

Wow that's so subtle specially since there is nowhere in the test where
auto-click.js is included. Might be worth commenting on the callWithTrustedClick
method.

[Reilly Grant (use Gerrit), 2017-04-06 06:53:13]

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html
(right):

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/usb-manual.https.html:23:
navigator.usb.test.addFakeDevice(fakeDeviceInit);
On 2017/04/06 02:51:47, ortuno wrote:
> On 2017/04/06 at 01:52:32, Reilly Grant wrote:
> > On 2017/04/06 01:14:06, ortuno wrote:
> > > In the vein of designing our tests APIs in a way that all browsers can
> implement
> > > them I wonder if making these synchronous is rather limiting and will be a
> > > problem for other browsers to implement.
> > 
> > This call doesn't guarantee that anything happens synchronously, just that
the
> browser eventually adds a new fake device matching the provided parameters.
> 
> Interesting, then how do you guarantee that the device will be returned when
> getDevices() is called? It might be that the device hasn't been added by the
> time getDevices() is called causing the test to fail right?

Indeed, you're right, if I were really using this function correctly as I have
described it then I should be listening for a connection event instead of
calling getDevices(). I have the option of either specifying that the device
will be present by the time the Promise returned by the next getDevices() call
returns, or update my code to not rely on that.

[foolip, 2017-04-10 07:48:54]

I like what I'm seeing, but this is now essentially blocked on reaching some
kind of agreement, however tentative, on public-test-infra. Are there others
currently implementing WebUSB that we could ask to weigh in there?

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/external/wpt/webusb/idlharness.https.html
(right):

https://codereview.chromium.org/2789723003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webusb/idlharness.https.html:224: if
(navigator.usb.test &&
This is essentially how I'd like tests like this to look, with the small caveat
that I think there should be failing tests if navigator.usb.test.initialize()
doesn't exist, instead of silently not testing some things.

[Reilly Grant (use Gerrit), 2017-04-18 21:20:31]

Rebased

[Reilly Grant (use Gerrit), 2017-04-18 21:23:41]

I've rebased this patch on top of the changes suggested by ortuno@.

foolip@, these tests will fail if navigator.usb.test.initialize is unavailable.
The idlharness.js tests are the only exception to this because I feel it is
valuable to be able to test a subset of the IDL.

[Reilly Grant (use Gerrit), 2017-04-18 21:23:48]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 21:24:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ortuno, 2017-04-18 21:41:47]

Code lgtm

Though I think before we land this we have to sent out an email to
public-test-infra:

> first send a message to public-test-infra detailing how they'd like to test
[the API], and what other vendors would have to implement to automate the tests.

https://codereview.chromium.org/2789723003/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/usb/OWNERS (left):

https://codereview.chromium.org/2789723003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/usb/OWNERS:1: # TEAM: device-dev@chromium.org
I think you can keep this and it won't get sync'ed. Once this lands you can send
out a patch on the gh repo with an OWNERS with your gh username so you get
notified of changes to the tests.

[commit-bot: I haz the power, 2017-04-18 22:33:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 22:33:12]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2017-04-19 23:17:00]

I have begun specifying this test interface here:

https://wicg.github.io/webusb/test/

In the process I have found places where the current API is problematic or could
be made more ergonomical so I will continue to iterate.

[foolip, 2017-04-20 09:55:30]

https://codereview.chromium.org/2789723003/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/usb/OWNERS (left):

https://codereview.chromium.org/2789723003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/usb/OWNERS:1: # TEAM: device-dev@chromium.org
On 2017/04/18 21:41:46, ortuno wrote:
> I think you can keep this and it won't get sync'ed. Once this lands you can
send
> out a patch on the gh repo with an OWNERS with your gh username so you get
> notified of changes to the tests.

That's right, the OWNERS files are completely unsync'ed so we can keep things
like this. (Some syncing would be nice, but would require a mapping from GitHub
users to chromium accounts.)

[Reilly Grant (use Gerrit), 2017-04-20 23:16:04]

Add back OWNERS files

[foolip, 2017-04-21 16:18:18]

On 2017/04/19 23:17:00, Reilly Grant wrote:
> I have begun specifying this test interface here:
> 
> https://wicg.github.io/webusb/test/
> 
> In the process I have found places where the current API is problematic or
could
> be made more ergonomical so I will continue to iterate.

Cool! Is this further along than the testing API for Bluetooth, so that this
should be our first attempt at using testing APIs in web-platform-tests? Would
you like to draft a message to public-test-infra? Feel free to just do it, but
I'm happy to help in whatever way I can. I think getting some feedback from
others who would like to begin implementing WebUSB is the key thing. That would
be good to have on public-test-infra of course, but finding the people is
something that can be done in advance.

[Reilly Grant (use Gerrit), 2017-05-02 23:04:28]

On 2017/04/21 16:18:18, foolip wrote:
> On 2017/04/19 23:17:00, Reilly Grant wrote:
> > I have begun specifying this test interface here:
> > 
> > https://wicg.github.io/webusb/test/
> > 
> > In the process I have found places where the current API is problematic or
> could
> > be made more ergonomical so I will continue to iterate.
> 
> Cool! Is this further along than the testing API for Bluetooth, so that this
> should be our first attempt at using testing APIs in web-platform-tests? Would
> you like to draft a message to public-test-infra? Feel free to just do it, but
> I'm happy to help in whatever way I can. I think getting some feedback from
> others who would like to begin implementing WebUSB is the key thing. That
would
> be good to have on public-test-infra of course, but finding the people is
> something that can be done in advance.

I have started a thread on public-test-infra. No other browser vendors that I am
aware of have plans to implement Web Bluetooth or WebUSB. The question we should
be asking is whether they believe this work will be useful to them if they
decide to implement.

I would like to land this change as having this move outstanding makes it
difficult to do further iteration on these tests.

[foolip, 2017-05-03 15:36:02]

On 2017/05/02 23:04:28, Reilly Grant wrote:
> On 2017/04/21 16:18:18, foolip wrote:
> > On 2017/04/19 23:17:00, Reilly Grant wrote:
> > > I have begun specifying this test interface here:
> > > 
> > > https://wicg.github.io/webusb/test/
> > > 
> > > In the process I have found places where the current API is problematic or
> > could
> > > be made more ergonomical so I will continue to iterate.
> > 
> > Cool! Is this further along than the testing API for Bluetooth, so that this
> > should be our first attempt at using testing APIs in web-platform-tests?
Would
> > you like to draft a message to public-test-infra? Feel free to just do it,
but
> > I'm happy to help in whatever way I can. I think getting some feedback from
> > others who would like to begin implementing WebUSB is the key thing. That
> would
> > be good to have on public-test-infra of course, but finding the people is
> > something that can be done in advance.
> 
> I have started a thread on public-test-infra. No other browser vendors that I
am
> aware of have plans to implement Web Bluetooth or WebUSB. The question we
should
> be asking is whether they believe this work will be useful to them if they
> decide to implement.
> 
> I would like to land this change as having this move outstanding makes it
> difficult to do further iteration on these tests.

Thanks, that is
https://lists.w3.org/Archives/Public/public-test-infra/2017AprJun/thread.html...
where I'm still trying to figure out if I could say anything helpful.

While all we have is skepticism there, I think we shouldn't upstream the tests,
but we certainly need to solve this somehow.

[Reilly Grant (use Gerrit), 2017-05-19 23:16:20]

Rebased

[Reilly Grant (use Gerrit), 2017-05-19 23:17:22]

On 2017/05/03 15:36:02, foolip wrote:
> On 2017/05/02 23:04:28, Reilly Grant wrote:
> > On 2017/04/21 16:18:18, foolip wrote:
> > > On 2017/04/19 23:17:00, Reilly Grant wrote:
> > > > I have begun specifying this test interface here:
> > > > 
> > > > https://wicg.github.io/webusb/test/
> > > > 
> > > > In the process I have found places where the current API is problematic
or
> > > could
> > > > be made more ergonomical so I will continue to iterate.
> > > 
> > > Cool! Is this further along than the testing API for Bluetooth, so that
this
> > > should be our first attempt at using testing APIs in web-platform-tests?
> Would
> > > you like to draft a message to public-test-infra? Feel free to just do it,
> but
> > > I'm happy to help in whatever way I can. I think getting some feedback
from
> > > others who would like to begin implementing WebUSB is the key thing. That
> > would
> > > be good to have on public-test-infra of course, but finding the people is
> > > something that can be done in advance.
> > 
> > I have started a thread on public-test-infra. No other browser vendors that
I
> am
> > aware of have plans to implement Web Bluetooth or WebUSB. The question we
> should
> > be asking is whether they believe this work will be useful to them if they
> > decide to implement.
> > 
> > I would like to land this change as having this move outstanding makes it
> > difficult to do further iteration on these tests.
> 
> Thanks, that is
>
https://lists.w3.org/Archives/Public/public-test-infra/2017AprJun/thread.html...
> where I'm still trying to figure out if I could say anything helpful.
> 
> While all we have is skepticism there, I think we shouldn't upstream the
tests,
> but we certainly need to solve this somehow.

We have public support from Servo developers for upstreaming our layout tests
using a Testing API:

https://lists.w3.org/Archives/Public/public-test-infra/2017AprJun/0056.html

[Reilly Grant (use Gerrit), 2017-05-19 23:17:57]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 23:18:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-20 00:32:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-20 00:32:48]

Dry run: This issue passed the CQ dry run.

[foolip, 2017-05-24 11:48:56]

I'll be OOO for a while, if necessary please ask another reviewer.

[Reilly Grant (use Gerrit), 2017-06-20 01:29:13]

Upstream tests that will work in stable Chrome

[Reilly Grant (use Gerrit), 2017-06-20 01:30:58]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 01:31:26]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Reilly Grant (use Gerrit), 2017-06-20 01:32:02]

Description was changed from

==========
Migrate WebUSB LayoutTests into external/wpt

This test moves all the tests from third_party/WebKit/LayoutTests/usb
to third_party/WebKit/LayoutTests/external/wpt/webusb. The WebUSB Test
API polyfil is integrated into testharnessreport.js so that it is
available for these tests.

BUG=705734
==========

to

==========
Migrate WebUSB LayoutTests into external/wpt

This test moves all the tests from third_party/WebKit/LayoutTests/usb
to third_party/WebKit/LayoutTests/external/wpt/webusb. The WebUSB Test
API polyfill is included in a new resources/chromium directory and
loaded automatically when the tests are run in a Chromium-based browser
with the --enable-blink-features=MojoJS,MojoJSTest flag set.

BUG=705734
==========

[commit-bot: I haz the power, 2017-06-20 03:52:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-20 03:52:24]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2017-06-20 06:18:30]

Please take another look as much has changed in this patch.

[ortuno, 2017-06-21 00:43:50]

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
Are these stable enough that we can commit them somewhere else? Are we going to
have to update them every time we change the usb interface or mojo?

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:6:
let loadChromiumResources = Promise.resolve().then(() => {
Sad that we have to include Chrome-specific code in cross-browser code.

[foolip, 2017-06-21 08:09:36]

Reilly, do you want to follow up on
https://lists.w3.org/Archives/Public/public-test-infra/2017AprJun/0081.html to
say that this is where it's happening?

[foolip, 2017-06-21 08:23:30]

Overall approach LGTM. I didn't review all of the tests, just the ones that I
commented on.

To land this I think a README.md is also needed that explains how
https://wicg.github.io/webusb/test/ is assumed, why it's not yet possible to run
the tests in release versions of Chrome with a link to that issue, and anything
else we've discussed.

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/external/wpt/webusb/idlharness.https.html
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/idlharness.https.html:22: //
Copied from https://wicg.github.io/webusb/#idl-index minus the
Can you move this into external/wpt/interfaces/webusb.idl instead? That's the
new thing, and it will make it easier if we want to build tooling to
automatically update the IDLs when specs change.

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:6:
let loadChromiumResources = Promise.resolve().then(() => {
On 2017/06/21 00:43:50, ortuno wrote:
> Sad that we have to include Chrome-specific code in cross-browser code.

Could this be made part of wpt_automation instead?

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/webusb/usbConnectionEvent.https.html
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/usbConnectionEvent.https.html:18:
try {
Can be reduced to `assert_throws(new TypeError, () => new
USBConnectionEvent('connect', { device: null }));`

[Reilly Grant (use Gerrit), 2017-06-21 21:35:16]

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/external/wpt/webusb/idlharness.https.html
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/idlharness.https.html:22: //
Copied from https://wicg.github.io/webusb/#idl-index minus the
On 2017/06/21 08:23:29, foolip wrote:
> Can you move this into external/wpt/interfaces/webusb.idl instead? That's the
> new thing, and it will make it easier if we want to build tooling to
> automatically update the IDLs when specs change.

Done in https://chromium-review.googlesource.com/c/543816/.

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:6:
let loadChromiumResources = Promise.resolve().then(() => {
On 2017/06/21 08:23:29, foolip wrote:
> On 2017/06/21 00:43:50, ortuno wrote:
> > Sad that we have to include Chrome-specific code in cross-browser code.
> 
> Could this be made part of wpt_automation instead?

One of the goals of this patch is to make these tests work in stable Chrome when
you navigate to web-platform-tests.org with the right flags set and that
requires including the Chromium-specific code in the WPT repository.

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/webusb/usbConnectionEvent.https.html
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/usbConnectionEvent.https.html:18:
try {
On 2017/06/21 08:23:30, foolip wrote:
> Can be reduced to `assert_throws(new TypeError, () => new
> USBConnectionEvent('connect', { device: null }));`

Done in https://chromium-review.googlesource.com/c/544077/.

[Reilly Grant (use Gerrit), 2017-06-22 20:57:42]

Rebased

[Reilly Grant (use Gerrit), 2017-06-22 21:35:52]

Add README.md and more comments explaining the polyfill

[Reilly Grant (use Gerrit), 2017-06-22 21:37:35]

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
On 2017/06/21 00:43:50, ortuno wrote:
> Are these stable enough that we can commit them somewhere else? Are we going
to
> have to update them every time we change the usb interface or mojo?

They will need to be updated. Since these tests run in the CQ the breakage
should be noticed easily.

[ortuno, 2017-06-22 21:45:33]

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
On 2017/06/22 at 21:37:35, Reilly Grant (use Gerrit) wrote:
> On 2017/06/21 00:43:50, ortuno wrote:
> > Are these stable enough that we can commit them somewhere else? Are we going
to
> > have to update them every time we change the usb interface or mojo?
> 
> They will need to be updated. Since these tests run in the CQ the breakage
should be noticed easily.

OK. One more concern: this is coupled with a specific Chrome Build, right? So
the tests will only work on that specific version of Chrome. For example say
stable is 63.0.1234 and a breaking Mojo change happens in 65.0.1678. We will
update these files to land that breaking change but we will break these tests in
stable, i.e. 63.0.1234, right? Am I missing something?

[Reilly Grant (use Gerrit), 2017-06-22 21:58:27]

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
On 2017/06/22 21:45:33, ortuno wrote:
> On 2017/06/22 at 21:37:35, Reilly Grant (use Gerrit) wrote:
> > On 2017/06/21 00:43:50, ortuno wrote:
> > > Are these stable enough that we can commit them somewhere else? Are we
going
> to
> > > have to update them every time we change the usb interface or mojo?
> > 
> > They will need to be updated. Since these tests run in the CQ the breakage
> should be noticed easily.
> 
> OK. One more concern: this is coupled with a specific Chrome Build, right? So
> the tests will only work on that specific version of Chrome. For example say
> stable is 63.0.1234 and a breaking Mojo change happens in 65.0.1678. We will
> update these files to land that breaking change but we will break these tests
in
> stable, i.e. 63.0.1234, right? Am I missing something?

That is correct. It is not ideal but it mirrors the fact that these tests will
also break if we update them to match a change to the specification and then run
them on old Chrome versions.

[ortuno, 2017-06-22 22:07:41]

lgtm

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/resources/chromium/device_manager.mojom.js:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
On 2017/06/22 at 21:58:27, Reilly Grant (use Gerrit) wrote:
> On 2017/06/22 21:45:33, ortuno wrote:
> > On 2017/06/22 at 21:37:35, Reilly Grant (use Gerrit) wrote:
> > > On 2017/06/21 00:43:50, ortuno wrote:
> > > > Are these stable enough that we can commit them somewhere else? Are we
going
> > to
> > > > have to update them every time we change the usb interface or mojo?
> > > 
> > > They will need to be updated. Since these tests run in the CQ the breakage
> > should be noticed easily.
> > 
> > OK. One more concern: this is coupled with a specific Chrome Build, right?
So
> > the tests will only work on that specific version of Chrome. For example say
> > stable is 63.0.1234 and a breaking Mojo change happens in 65.0.1678. We will
> > update these files to land that breaking change but we will break these
tests in
> > stable, i.e. 63.0.1234, right? Am I missing something?
> 
> That is correct. It is not ideal but it mirrors the fact that these tests will
also break if we update them to match a change to the specification and then run
them on old Chrome versions.

Chatted a bit offline about how stable these files are. According to reillyg
changes shouldn't happen often.

[Reilly Grant (use Gerrit), 2017-06-22 22:22:50]

The CQ bit was checked by reillyg@chromium.org

[Reilly Grant (use Gerrit), 2017-06-22 22:22:51]

The patchset sent to the CQ was uploaded after l-g-t-m from foolip@chromium.org
Link to the patchset: https://codereview.chromium.org/2789723003/#ps160001
(title: "Add README.md and more comments explaining the polyfill")

[commit-bot: I haz the power, 2017-06-22 22:23:34]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2017-06-22 22:30:38]

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:6:
let loadChromiumResources = Promise.resolve().then(() => {
On 2017/06/21 21:35:16, Reilly Grant (use Gerrit) wrote:
> On 2017/06/21 08:23:29, foolip wrote:
> > On 2017/06/21 00:43:50, ortuno wrote:
> > > Sad that we have to include Chrome-specific code in cross-browser code.
> > 
> > Could this be made part of wpt_automation instead?
> 
> One of the goals of this patch is to make these tests work in stable Chrome
when
> you navigate to http://web-platform-tests.org with the right flags set and
that
> requires including the Chromium-specific code in the WPT repository.

Hmm, I didn't notice that /resources/chromium/*.js were also added in wpt. I can
see how this way solves the problem, but I don't think there are other
vendor-specific bits in wpt right now, what other approaches are at all
possible?

Would it not be a problem that mojo mocking bits are versioned together with the
wpt and not Chromium? If I take the stable Chromium and /resources/chromium/*.js
have changed on master, I presume the tests may not work at all?

Does the --enable-blink-features=MojoJS,MojoJSTest bit already work with a plain
Chromium build? If it doesn't, and there's some other work to be done, would it
be OK to have this only work in content_shell until that work is done?

[foolip, 2017-06-22 22:52:19]

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js
(right):

https://codereview.chromium.org/2789723003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/webusb/resources/usb-helpers.js:6:
let loadChromiumResources = Promise.resolve().then(() => {
On 2017/06/22 22:30:38, foolip wrote:
> On 2017/06/21 21:35:16, Reilly Grant (use Gerrit) wrote:
> > On 2017/06/21 08:23:29, foolip wrote:
> > > On 2017/06/21 00:43:50, ortuno wrote:
> > > > Sad that we have to include Chrome-specific code in cross-browser code.
> > > 
> > > Could this be made part of wpt_automation instead?
> > 
> > One of the goals of this patch is to make these tests work in stable Chrome
> when
> > you navigate to http://web-platform-tests.org with the right flags set and
> that
> > requires including the Chromium-specific code in the WPT repository.
> 
> Hmm, I didn't notice that /resources/chromium/*.js were also added in wpt. I
can
> see how this way solves the problem, but I don't think there are other
> vendor-specific bits in wpt right now, what other approaches are at all
> possible?
> 
> Would it not be a problem that mojo mocking bits are versioned together with
the
> wpt and not Chromium? If I take the stable Chromium and
/resources/chromium/*.js
> have changed on master, I presume the tests may not work at all?

Discussed in https://codereview.chromium.org/2789723003/#msg59, if it's not
likely to be a practical problem then that's fine, if it is a problem then
revisit.

> Does the --enable-blink-features=MojoJS,MojoJSTest bit already work with a
plain
> Chromium build? If it doesn't, and there's some other work to be done, would
it
> be OK to have this only work in content_shell until that work is done?

Apparently that already works.

If we're to run the tests with vanilla Chrome and vanilla WPT, then there are
only 3 options for where the mojo mocking scripts live:
1. In Chrome, but we'll not do that because of the binary size.
2. In WPT itself, as done here.
3. In a third thing which is versions together with Chrome and/or WPT, a bit
like ChromeDriver perhaps.

Option 3 would be more pure, but more work to get off the ground. And moving
from this to such a solution shouldn't be very disruptive.

Perhaps there will be some raised eyebrows in wpt, but I think I agree with the
choices now, and we can see how it goes. LGTM, thanks for your restless work on
this!

[commit-bot: I haz the power, 2017-06-23 00:01:15]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1498170170118450,
"parent_rev": "bedb4b202b4b909e7e1f79439cadd2842f105470", "commit_rev":
"a58aca05fb59a513a0857538de9e79eea712d4e4"}

[commit-bot: I haz the power, 2017-06-23 00:01:27]

Description was changed from

==========
Migrate WebUSB LayoutTests into external/wpt

This test moves all the tests from third_party/WebKit/LayoutTests/usb
to third_party/WebKit/LayoutTests/external/wpt/webusb. The WebUSB Test
API polyfill is included in a new resources/chromium directory and
loaded automatically when the tests are run in a Chromium-based browser
with the --enable-blink-features=MojoJS,MojoJSTest flag set.

BUG=705734
==========

to

==========
Migrate WebUSB LayoutTests into external/wpt

This test moves all the tests from third_party/WebKit/LayoutTests/usb
to third_party/WebKit/LayoutTests/external/wpt/webusb. The WebUSB Test
API polyfill is included in a new resources/chromium directory and
loaded automatically when the tests are run in a Chromium-based browser
with the --enable-blink-features=MojoJS,MojoJSTest flag set.

BUG=705734

Review-Url: https://codereview.chromium.org/2789723003
Cr-Commit-Position: refs/heads/master@{#481732}
Committed:
https://chromium.googlesource.com/chromium/src/+/a58aca05fb59a513a0857538de9e...
==========

[commit-bot: I haz the power, 2017-06-23 00:01:29]

Committed patchset #8 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/a58aca05fb59a513a0857538de9e...