Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a test for createImageBitmap inside a worker #21083

Merged

Conversation

Cwiiis
Copy link
Contributor

@Cwiiis Cwiiis commented Jan 8, 2020

This is tested in the offscreen-canvas tests, but not specifically in the ImageBitmap tests. Given that WebKit at least currently supports the latter and not the former (and this may be true of Gecko?), it'd be worthwhile to test it explicitly.

I ran into this as creation of image bitmaps in WebKit asserted when run off the main thread.

@annevk
Copy link
Member

annevk commented Jan 8, 2020

Do you know about https://web-platform-tests.org/writing-tests/testharness.html#multi-global-tests? I would generally recommend writing worker tests that way (or rewrite an existing window test to cover more globals).

@Cwiiis
Copy link
Contributor Author

Cwiiis commented Jan 8, 2020

Do you know about https://web-platform-tests.org/writing-tests/testharness.html#multi-global-tests? I would generally recommend writing worker tests that way (or rewrite an existing window test to cover more globals).

Interesting - in this case, the behaviour is specifically different when in a worker (you can't create an image bitmap from sources that rely on the DOM), so it's easier to write it like this, but I'll bear this in mind in the future.

@annevk
Copy link
Member

annevk commented Jan 8, 2020

@Cwiiis
Copy link
Contributor Author

Cwiiis commented Jan 8, 2020

Note that https://web-platform-tests.org/writing-tests/testharness.html#standalone-workers-tests exists too.

I was aware of this - in this case, I specifically wanted to have a worker transfer the ImageBitmap back to a non-worker, which I don't think you can do with a standalone worker test(?)

@annevk
Copy link
Member

annevk commented Jan 8, 2020

Ah fair, you can't. Sorry for the noise.

@Cwiiis
Copy link
Contributor Author

Cwiiis commented Jan 8, 2020

Ah fair, you can't. Sorry for the noise.

Not at all, I learnt something useful :)

@clopez
Copy link
Contributor

clopez commented Jan 21, 2020

This landed on WebKit in https://bugs.webkit.org/show_bug.cgi?id=205850

@annevk
Copy link
Member

annevk commented Jan 22, 2020

Does that mean we should close this or should we still land it? Is it not being landed because @Cwiiis has no merge rights? Not sure what the status is of this PR.

@Cwiiis
Copy link
Contributor Author

Cwiiis commented Jan 22, 2020

I thought we'd landed this already - it'd be good for it to be landed, otherwise WebKit will be out of sync and the change will be overwritten if someone happens to update that test. I don't have merge rights to do so.

@annevk annevk merged commit 8fddfc3 into web-platform-tests:master Jan 22, 2020
@annevk
Copy link
Member

annevk commented Jan 22, 2020

(I've asked on IRC for someone to give you those rights.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants