Skip to content

Conversation

annevk
Copy link
Contributor

@annevk annevk commented Apr 12, 2024

f8cecb9

Disable OffscreenCanvasRenderingContext2D's commit() method by default
https://bugs.webkit.org/show_bug.cgi?id=272591
rdar://126758254

Reviewed by Matt Woodrow.

This puts OffscreenCanvasRenderingContext2D's commit() behind a runtime
setting marked testable (OffscreenCanvasDeprecatedCommitEnabled). This
ensures this method continues to be tested for the time being and can
be re-enabled should there be a need.

The goal is to remove this method and underlying implementation
completely as discussed here:
whatwg/html#9979

* LayoutTests/inspector/canvas/recording-offscreen-canvas-2d-full-expected.txt:
* LayoutTests/inspector/canvas/resources/recording-2d.js:
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/html/canvas/OffscreenCanvasRenderingContext2D.idl:

Canonical link: https://commits.webkit.org/278047@main

7cb3f4b

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim loading 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch ✅ 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim ✅ 🧪 jsc-armv7-tests

@annevk annevk self-assigned this Apr 12, 2024
@annevk annevk added the Canvas Bugs related to the canvas element. label Apr 12, 2024
@webkit-early-warning-system

This comment was marked as outdated.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 12, 2024
@annevk annevk removed the merging-blocked Applied to prevent a change from being merged label Apr 13, 2024
@annevk annevk force-pushed the eng/Remove-OffscreenCanvasRenderingContext2Ds-commit-method branch from ccbdfb7 to 84dfc72 Compare April 13, 2024 06:57
@webkit-early-warning-system

This comment was marked as outdated.

@annevk annevk changed the title Remove OffscreenCanvasRenderingContext2D's commit() method Disable OffscreenCanvasRenderingContext2D's commit() method by default Apr 13, 2024
@annevk annevk force-pushed the eng/Remove-OffscreenCanvasRenderingContext2Ds-commit-method branch from 84dfc72 to 8205e40 Compare April 13, 2024 06:58
@webkit-early-warning-system

This comment was marked as outdated.

@annevk annevk marked this pull request as ready for review April 13, 2024 06:58
@annevk annevk requested review from cdumez and rniwa as code owners April 13, 2024 06:58
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 13, 2024
@annevk annevk removed the merging-blocked Applied to prevent a change from being merged label Apr 13, 2024
@annevk annevk force-pushed the eng/Remove-OffscreenCanvasRenderingContext2Ds-commit-method branch from 8205e40 to 4bc054f Compare April 13, 2024 15:43
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 13, 2024
Kaiido added a commit to Kaiido/content that referenced this pull request Apr 18, 2024
This method has been removed in whatwg/html#9979

The placeholder <canvas>'s bitmap is now updated during the "update the rendering" steps of the Worker or Window's event-loop that owns the OffscreenCanvas object. Not sure how to formulate that clearly.

This method has been (partially) implemented in 2 browsers before it got removed, but at least Safari is in the process of removing it (WebKit/WebKit#27196), and there may be another method with the same name coming later. So it's not clear if simply removing the method's page like that is fine or if it would be better to keep it with a deprecation notice instead?

Compat table would also need to be updated, but I'll wait for a position on the above question.

Also, while I was on that page I fixed an approximative wording, and the position of the roundRect() method.
8a4754d7ba
Kaiido added a commit to Kaiido/content that referenced this pull request Apr 19, 2024
This method has been removed in whatwg/html#9979

The placeholder <canvas>'s bitmap is now updated during the "update the rendering" steps of the Worker or Window's event-loop that owns the OffscreenCanvas object. Not sure how to formulate that clearly.

This method has been (partially) implemented in 2 browsers before it got removed, but at least Safari is in the process of removing it (WebKit/WebKit#27196), and there may be another method with the same name coming later. So it's not clear if simply removing the method's page like that is fine or if it would be better to keep it with a deprecation notice instead?

Compat table would also need to be updated, but I'll wait for a position on the above question.

Also, while I was on that page I fixed an approximative wording, and the position of the roundRect() method.
wbamberg added a commit to mdn/content that referenced this pull request Apr 25, 2024
* Deprecate OffscreenCanvas2D's commit() method

This method has been removed in whatwg/html#9979

The placeholder <canvas>'s bitmap is now updated during the "update the rendering" steps of the Worker or Window's event-loop that owns the OffscreenCanvas object. Not sure how to formulate that clearly.

This method has been (partially) implemented in 2 browsers before it got removed, but at least Safari is in the process of removing it (WebKit/WebKit#27196), and there may be another method with the same name coming later. So it's not clear if simply removing the method's page like that is fine or if it would be better to keep it with a deprecation notice instead?

Compat table would also need to be updated, but I'll wait for a position on the above question.

Also, while I was on that page I fixed an approximative wording, and the position of the roundRect() method.

* Better wording from suggestion

Co-authored-by: wbamberg <will@bootbonnet.ca>

* remove deprecation macros as per comments

These should hopefully be added back once mdn/browser-compat-data#22878 is merged

---------

Co-authored-by: wbamberg <will@bootbonnet.ca>
Co-authored-by: Jean-Yves Perrier <jypenator@gmail.com>
@annevk annevk removed the merging-blocked Applied to prevent a change from being merged label Apr 26, 2024
@annevk annevk force-pushed the eng/Remove-OffscreenCanvasRenderingContext2Ds-commit-method branch from 4bc054f to 7cb3f4b Compare April 26, 2024 14:36
@annevk annevk added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 26, 2024
https://bugs.webkit.org/show_bug.cgi?id=272591
rdar://126758254

Reviewed by Matt Woodrow.

This puts OffscreenCanvasRenderingContext2D's commit() behind a runtime
setting marked testable (OffscreenCanvasDeprecatedCommitEnabled). This
ensures this method continues to be tested for the time being and can
be re-enabled should there be a need.

The goal is to remove this method and underlying implementation
completely as discussed here:
whatwg/html#9979

* LayoutTests/inspector/canvas/recording-offscreen-canvas-2d-full-expected.txt:
* LayoutTests/inspector/canvas/resources/recording-2d.js:
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/html/canvas/OffscreenCanvasRenderingContext2D.idl:

Canonical link: https://commits.webkit.org/278047@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Remove-OffscreenCanvasRenderingContext2Ds-commit-method branch from 7cb3f4b to f8cecb9 Compare April 26, 2024 17:20
@webkit-commit-queue
Copy link
Collaborator

Committed 278047@main (f8cecb9): https://commits.webkit.org/278047@main

Reviewed commits have been landed. Closing PR #27196 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 26, 2024
@webkit-commit-queue webkit-commit-queue merged commit f8cecb9 into WebKit:main Apr 26, 2024
@annevk annevk deleted the eng/Remove-OffscreenCanvasRenderingContext2Ds-commit-method branch April 26, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canvas Bugs related to the canvas element.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants