Skip to content

[pull] main from oven-sh:main - #2

Merged
pull[bot] merged 1 commit into
coleleavitt:mainfrom
oven-sh:main
Jul 18, 2026
Merged

pull[bot] merged 1 commit into
coleleavitt:mainfrom
oven-sh:main

Conversation

@pull

@pull pull Bot commented Jul 18, 2026

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )


Summary by cubic

Removes ThrowScope from static property reification/lookup paths to avoid simulated throws, and switches to early-return on real exceptions via vm.exceptionForInspection(). This fixes crashes in callers that don’t expect throws while preserving correct handling of PropertyCallback builder exceptions.

  • Bug Fixes
    • Replace ThrowScope with DeferTerminationForAWhile + vm.exceptionForInspection() in JSObject::reifyAllStaticProperties and setUpStaticFunctionSlot.
    • Prevents false “throwing” behavior and crashes in first static-table lookups and deleteProperty when validateExceptionChecks=1.

Written for commit a8d15c1. Summary will update on new commits.

Review in cubic

ae5110d added DECLARE_THROW_SCOPE inside setUpStaticFunctionSlot and
reifyAllStaticProperties so a PropertyCallback builder that throws can be
detected and propagated. A ThrowScope's destructor unconditionally
simulates a throw to its caller, so this made both functions look
throwing to the exception-scope verifier. setUpStaticFunctionSlot runs on
every first lookup of a static hashtable property via getOwnPropertySlot,
and JSC-internal callers of that path (CodeBlock::finishCreation via
initializeTemplateObjects, among others) never check afterward, so with
validateExceptionChecks=1 they now crash even though nothing threw.
reifyAllStaticProperties has the same issue for JSObject::deleteProperty.

Keep the DeferTerminationForAWhile and the early-return-on-exception,
but read vm.exceptionForInspection() directly instead of declaring a
ThrowScope. That preserves the ae5110d behaviour (report the slot as
not found when the builder left an exception pending, bail from the
reify-all loop) without forcing a simulated throw onto callers that have
never had to handle one.
@pull pull Bot locked and limited conversation to collaborators Jul 18, 2026
@pull pull Bot added the ⤵️ pull label Jul 18, 2026
@pull
pull Bot merged commit a8d15c1 into coleleavitt:main Jul 18, 2026
pull Bot pushed a commit that referenced this pull request Aug 25, 2026
https://bugs.webkit.org/show_bug.cgi?id=319665

Reviewed by Alicia Boya Garcia and Nikolas Zimmermann.

This bundles three related fixes uncovered while restoring pages containing
a media element (e.g.: "video") from the back/forward cache:

1. HTMLMediaElement::suspend() destroyed the media player on entering the
   cache (via stopWithoutDestroyingMediaPlayer() -> userCancelledLoad()),
   so a restored page reported video.error.code == MEDIA_ERR_ABORTED.
   userCancelledLoad() now takes a destroyMediaPlayer flag, false for the
   BackForwardCache suspension path, so it early-returns instead of tearing
   the player down.

2. A <video> that was playing before being cached did not resume playing
   after restore. HTMLMediaElement::resume() schedules an updatePlayState()
   task while the media session is still interrupted. mayResumePlayback()
   now also retries updatePlayState() when the element isn't paused.

3. A <video> that was paused before being cached showed a blank/white
   frame instead of its last decoded frame after restore. Restoring a page
   rebuilds the render tree, so CoordinatedPlatformLayerBufferProxy gets
   attached to a brand new CoordinatedPlatformLayer with no buffer of its
   own, and a paused GStreamer pipeline has no new sample to push. The
   proxy now invokes a layer-attached callback whenever a new non-null
   layer is attached, which MediaPlayerPrivateGStreamer uses to re-deliver
   its current sample so the new layer isn't left blank.

Also fixes a related macOS-only assert crash (ASSERTION FAILED:
m_mediaSessions.contains(identifier) in MediaUsageManagerCocoa::
updateMediaUsage): the UI process unconditionally clears its usage-tracking
map on every navigation commit, including a back/forward cache restore, but
MediaElementSession kept believing it had already added its session across
the whole suspend/resume cycle. mediaUsageManagerSessionWillBeSuspended()
resets that flag on suspend so the next update re-adds the session first.

A LayoutTest for the playing-before-caching scenario (fix #2):
it plays a video, waits for the playing event, navigates away and
back via history.back(), and asserts the video is still not paused and
currentTime has advanced after restore.

Added a LayoutTest based on media/restore-from-page-cache.html for a video
that was paused before entering the cache, as a real pixel/screenshot
comparison to catch the blank-frame regression from fix #3.

Tests: media/restore-from-page-cache-paused.html
       media/restore-from-page-cache-playing.html
* LayoutTests/media/restore-from-page-cache-paused-expected.png: Added.
* LayoutTests/media/restore-from-page-cache-paused-expected.txt: Added.
* LayoutTests/media/restore-from-page-cache-paused.html: Added.
* LayoutTests/media/restore-from-page-cache-playing-expected.txt: Added.
* LayoutTests/media/restore-from-page-cache-playing.html: Added.
* LayoutTests/platform/ios/TestExpectations:
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::userCancelledLoad):
(WebCore::HTMLMediaElement::stopWithoutDestroyingMediaPlayer):
(WebCore::HTMLMediaElement::suspend):
(WebCore::HTMLMediaElement::mayResumePlayback):
* Source/WebCore/html/HTMLMediaElement.h:
* Source/WebCore/html/MediaElementSession.cpp:
(WebCore::MediaElementSession::mediaUsageManagerSessionWillBeSuspended):
* Source/WebCore/html/MediaElementSession.h:
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):
(WebCore::MediaPlayerPrivateGStreamer::pushTextureToCompositor):
(WebCore::MediaPlayerPrivateGStreamer::triggerRepaint):
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedPlatformLayerBufferProxy.cpp:
(WebCore::CoordinatedPlatformLayerBufferProxy::CoordinatedPlatformLayerBufferProxy):
(WebCore::CoordinatedPlatformLayerBufferProxy::create):
(WebCore::CoordinatedPlatformLayerBufferProxy::setTargetLayer):
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedPlatformLayerBufferProxy.h:

Canonical link: https://commits.webkit.org/319472@main
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant