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

Fix timeout in fast/images/animated-image-mp4-crash.html #22810

Conversation

@shallawa shallawa self-assigned this Jan 16, 2024
@shallawa shallawa added the Images For bugs in image handling. label Jan 16, 2024
@shallawa shallawa requested review from smfr and heycam January 17, 2024 16:23
Comment on lines +9 to +10
if (window.internals)
internals.clearMemoryCache();
Copy link
Contributor

@heycam heycam Jan 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand why this resolves the timeout. Does image.decode() not resolve its promise if the image exists in the MemoryCache and we've tried to (and failed to) decode it before? I wonder if that's actually what the spec wants. Either way, can you please explain a little in a comment here to make it clear why we're doing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling internals.clearMemoryCache() resets the animation state of the image. Because the same web process can be used to run the same test multiple times, the image will be cached for later runs. So a run may use the image after the animation has advanced. If the animation ends, the promise will be rejected. And this test expects it to be resolved, so it times out.

@shallawa shallawa added the merge-queue Applied to send a pull request to merge-queue label Jan 29, 2024
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Fix-timeout-in-fastimagesanimated-image-mp4-crash-html branch from 6a921b9 to 5bef447 Compare January 29, 2024 18:41
https://bugs.webkit.org/show_bug.cgi?id=267571
rdar://121035201

Reviewed by Cameron McCormack.

Call "internals.clearMemoryCache()" when initializing the test to reset the
animation state of the image.

* LayoutTests/fast/images/animated-image-mp4-crash.html:

Canonical link: https://commits.webkit.org/273669@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Fix-timeout-in-fastimagesanimated-image-mp4-crash-html branch from 5bef447 to b89b579 Compare January 29, 2024 18:44
@webkit-commit-queue
Copy link
Collaborator

Committed 273669@main (b89b579): https://commits.webkit.org/273669@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 29, 2024
@webkit-commit-queue webkit-commit-queue merged commit b89b579 into WebKit:main Jan 29, 2024
@shallawa shallawa deleted the eng/Fix-timeout-in-fastimagesanimated-image-mp4-crash-html branch February 1, 2024 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Images For bugs in image handling.
Projects
None yet
4 participants