Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 118bdae825bb28da9cd77930f931928325c11042
https://github.com/WebKit/WebKit/commit/118bdae825bb28da9cd77930f931928325c11042
Author: Jean-Yves Avenard <[email protected]>
Date: 2026-05-14 (Thu, 14 May 2026)
Changed paths:
M LayoutTests/media/media-fullscreen-loop-inline.html
A LayoutTests/media/video-timeupdate-before-seeking-expected.txt
A LayoutTests/media/video-timeupdate-before-seeking.html
M Source/WebCore/html/HTMLMediaElement.cpp
M Source/WebCore/html/HTMLMediaElement.h
Log Message:
-----------
[Media] timeupdate event can dispatch ahead of 'seeking' under stress
https://bugs.webkit.org/show_bug.cgi?id=314792
rdar://problem/177044662
Reviewed by Jer Noble.
Commit 612b528 ("[MSE] timeupdate event can fire during seek operation,
before the seek completes.") routed periodic timeupdates through
m_periodicTimeupdateCancellationGroup so HTMLMediaElement::seek() could
drop them before queueing 'seeking', preserving the spec-required
seeking -> timeupdate -> seeked ordering. It deliberately left
non-periodic timeupdates outside the group:
Non-periodic timeupdate calls (finishSeek, updateReadyState,
mediaPlayerTimeChanged's discontinuity path) continue to use the
general async-events group and are not cancelled.
Under stress, mediasource-duration.html test 2 ("Test appendBuffer
completes previous seek to truncated duration") still races. A
non-periodic timeupdate scheduled by HTMLMediaElement::pauseInternal,
by HTMLMediaElement::setReadyState's else branch (when
MediaSource::monitorSourceBuffers drops readyState below
HAVE_FUTURE_DATA after sourceBuffer.remove()), or by
mediaPlayerTimeChanged from the AVFObjC MSE boundary-observer callback,
dispatches ahead of the JS-driven setDuration -> seek's 'seeking'
event.
Per HTML spec time-marches-on step 6
(https://html.spec.whatwg.org/multipage/media.html#time-marches-on):
"(In the other cases, such as explicit seeks, relevant events get
fired as part of the overall process of changing the current
playback position.)"
โ a timeupdate queued before an explicit seek must give way to the
seek's own events. Generalise 612b528's approach to cover all
timeupdates: route every scheduleTimeupdateEvent call through the
cancellation group, and have the seek path cancel the group before
any subsequent task can dispatch a queued timeupdate. The
seek-completion timeupdate is queued by finishSeek after the cancel
runs and is unaffected.
The cancel must run synchronously from the DOM-side entry point. The
original 612b528 placed it in seekTask, which runs asynchronously from
setCurrentTime; any timeupdate queued on the same MediaElement task
queue by an earlier synchronous call (e.g. pause()) is dispatched by
the event loop before seekTask runs, so its cancel() is too late to
drop it. Move the cancel into seekInternal, right after setSeeking(true)
and before seekTask is queued, so the cancellation happens in the same
synchronous chain as setCurrentTime.
Rename m_periodicTimeupdateCancellationGroup ->
m_timeupdateCancellationGroup to reflect the broadened scope. Also add
an explanatory comment in the AVFObjC boundary-observer callback noting
that timeChanged()'s timeupdate side effect is now cancellable by a
follow-up seek.
The synchronous cancel() closes the backward window (timeupdates queued
before the seek) but not the forward one: AVFoundation's periodic time
observer posts work via callOnMainThread/ensureOnMainThread, which can
run on the main runloop after the cancel() but before the event-loop
drain. Inside that callback, mediaPlayerTimeChanged's else branch calls
scheduleTimeupdateEvent(false) and queues a fresh cancellable task that
the earlier cancel() cannot retroactively catch; it then dispatches
before seekTask schedules 'seeking'. Close the forward window by
generalising the existing m_seeking guard in scheduleTimeupdateEvent
from periodic-only to unconditional: once setSeeking(true) has run, any
new timeupdate call is by definition a racer that belongs to the seek's
own event sequence. seekTask's noSeekRequired-and-time==now fast path
reaches scheduleTimeupdateEvent with m_seeking still true, so emit that
middle timeupdate directly via scheduleEvent as finishSeek does for its
step 16 timeupdate.
LayoutTests/media/media-fullscreen-loop-inline.html: the handler
required video.currentTime == 0 exactly after the loop-back seek. That
was never spec-guaranteed โ HTML ยง4.8.10.9 clears 'seeking' at step 14
before queueing the post-seek 'timeupdate' at step 16, so at dispatch
time currentMediaTime() reads from the player and can have advanced
past 0 on fast hardware. The test only passed historically because the
non-cancellable pre-loop 'timeupdate' scheduled from
mediaPlayerTimeChanged's else branch dispatched while m_seeking was
still true, causing currentMediaTime() to fall back to m_lastSeekTime
(0) via the early-return at HTMLMediaElement::currentMediaTime(). With
that call now routed through the cancellation group, seekInternal(0)
drops it and only finishSeek's post-clear 'timeupdate' remains โ flaky
on slower systems. Detect the backward time jump instead, which matches
the test's stated intent.
Test: media/video-timeupdate-before-seeking.html. Reproduces the race
* LayoutTests/media/media-fullscreen-loop-inline.html: Detect loop via
backward time jump instead of strict currentTime == 0.
* LayoutTests/media/video-timeupdate-before-seeking-expected.txt: Added.
* LayoutTests/media/video-timeupdate-before-seeking.html: Added.
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::seekWithTolerance):
(WebCore::HTMLMediaElement::seekTask):
(WebCore::HTMLMediaElement::scheduleTimeupdateEvent):
* Source/WebCore/html/HTMLMediaElement.h:
Canonical link: https://commits.webkit.org/313283@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
