Ensure that drawImage(detachedCanvas) throws #43835
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
requested review from
AmeliaBR,
annevk,
domenic,
foolip,
fserb,
jdm,
jgraham and
kenrussell
kenrussell
approved these changes
Jan 3, 2024
See whatwg/html#10026 and https://bugs.webkit.org/show_bug.cgi?id=266918 The specs are currently underspecified, though all implementations do agree that a detached OffscreenCanvas has a size of 0, which should throw when passed to drawImage(). Only Safari currently fails the test. A fix is on the way at WebKit/WebKit#22297.
Kaiido
force-pushed
the
detached-canvas
branch
from
803e33b
to
f6de267
Compare
webkit-commit-queue
pushed a commit
to Kaiido/WebKit
that referenced
this pull request
Feb 13, 2024
…webkit.org/show_bug.cgi?id=266918 Reviewed by @mattwoodrow. Explanation of why this fixes the bug. By setting the OffscreenCanvas's `size()` after the DetachedCanvas has been created we ensure that the DetachedCanvas has the correct size to later create the new OffscreenCanvas, and that any method checking the size of the previous OffscreenCanvas, including `drawImage()`, gets the expected `0` width and height. Also deletes the custom `width` and `height` getters from OffscreenCanvas since these were only used to return a "fake" `0` in case the OffscreenCanvas got detached, and that they are now truly `0`. The special handling in `setWidth` and `setHeight` is still required though. Tests are not yet published at web-platform-tests/wpt#43835 * Source/WebCore/html/OffscreenCanvas.cpp: (WebCore::OffscreenCanvas::detach): (WebCore::OffscreenCanvas::width const): Deleted. (WebCore::OffscreenCanvas::height const): Deleted. * Source/WebCore/html/OffscreenCanvas.h: import new WPT tests Manually imported since while web-platform-tests/wpt#43835 has been approved, it still hasn't been merged. drawImage(detachedOffscreenCanvas) is supposed to throw https://bugs.webkit.org/show_bug.cgi?id=266918 Reviewed by @Ahmad-S792. Since the previous change this method was failing to trigger the proper exception type when called on a detached OffscreenCanvas, because now the size is actually empty for detached OffscreenCanvas. We need to separate the case 'detached' and 'no buffer', because zero size OffscreenCanvas will match 'no buffer' and we'd throw an InvalidStateError instead of an IndexSizeError for zero-sized OffscreenCanvas. * Source/WebCore/html/OffscreenCanvas.cpp: (WebCore::OffscreenCanvas::convertToBlob): Canonical link: https://commits.webkit.org/274534@main
|
Small update/bump, the WebKit patch has landed a few weeks ago and now this test has been manually added there. |
|
Thanks @Kaiido for your continued efforts on improving canvas! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See whatwg/html#10026 and https://bugs.webkit.org/show_bug.cgi?id=266918
The specs are currently underspecified, though all implementations do agree that a detached
OffscreenCanvashas a size of 0, which should throw when passed todrawImage().Only Safari currently fails the test. A fix is on the way.
Note that I couldn't run
update-builtdue to #23292 and hence I did create the generated html manually...