Skip to content

Commit 6c874bb

Browse files
jyavenardjyavenard
committed
Safari pauses video when leaving tab
https://bugs.webkit.org/show_bug.cgi?id=303973 rdar://164514685 Reviewed by Eric Carlson. The information that the SourceBufferPrivate had created a new audio or video track wasn't passed back to the HTMLMediaElement. 302975@main changed the order of events which would have caused MediaPlayer::characteristicsChanged to be called before the hasAudio/hasVideo state was updated. Fly-By fix: replace auto player = m_player.get() with RefPtr player to be in line with current SaferCPP coding style. Added test media/media-source/media-source-has-audio-video-gpu.html LayoutTests/media/media-source/media-source-restrictions-expected.txt LayoutTests/media/media-source/media-source-restrictions.html: The test assumed that the paused event would be fired. But this was incorrect, playback should have never started as a user gesture was required. The test only passed earlier because the MockMediaPlayerMediaSource was never calling MediaPlayer::characteristicsChanged and so MediaPlayer::hasAudio() was never updated. Canonical link: https://commits.webkit.org/304336@main
1 parent 89d5b99 commit 6c874bb

13 files changed

+116
-45
lines changed

‎LayoutTests/media/media-source/media-source-has-audio-video-gpu-expected.txt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
2+
RUN(video.src = URL.createObjectURL(mediaSource))
3+
EVENT(sourceopen)
4+
RUN(mediaSource.duration = loader.duration())
5+
RUN(sourceBuffer = mediaSource.addSourceBuffer(loader.type()))
6+
RUN(sourceBuffer.appendBuffer(loader.initSegment()))
7+
EVENT(update)
8+
EXPECTED (internals.mediaUsageState(video).hasVideo == 'true') OK
9+
EXPECTED (internals.mediaUsageState(video).hasAudio == 'true') OK
10+
END OF TEST
11+

‎LayoutTests/media/media-source/media-source-has-audio-video-gpu.html‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!-- webkit-test-runner [ MediaSourceUseRemoteAudioVideoRenderer=false ] -->
2+
<!DOCTYPE html>
3+
<html>
4+
<head>
5+
<title>media-source-has-audio-video</title>
6+
<script src="media-source-loader.js"></script>
7+
<script src="../video-test.js"></script>
8+
<script>
9+
window.addEventListener('load', event => {
10+
findMediaElement();
11+
12+
window.sourceBuffer = new MediaSource();
13+
14+
loader = new MediaSourceLoader('content/test-fragmented-manifest.json');
15+
loader.onload = mediaDataLoaded;
16+
loader.onerror = mediaDataLoadingFailed;
17+
});
18+
19+
function mediaDataLoadingFailed() {
20+
failTest('Media data loading failed');
21+
}
22+
23+
async function mediaDataLoaded() {
24+
window.mediaSource = new MediaSource();
25+
run('video.src = URL.createObjectURL(mediaSource)');
26+
waitFor(video, 'error').then(failTest);
27+
await waitFor(mediaSource, 'sourceopen');
28+
29+
run('mediaSource.duration = loader.duration()');
30+
run('sourceBuffer = mediaSource.addSourceBuffer(loader.type())');
31+
run('sourceBuffer.appendBuffer(loader.initSegment())');
32+
await waitFor(sourceBuffer, 'update');
33+
34+
await testExpectedEventually('internals.mediaUsageState(video).hasVideo', true);
35+
await testExpectedEventually('internals.mediaUsageState(video).hasAudio', true);
36+
endTest();
37+
}
38+
39+
</script>
40+
</head>
41+
<body>
42+
<video></video>
43+
</body>
44+
</html>

‎LayoutTests/media/media-source/media-source-restrictions-expected.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ RUN(sourceBuffer.appendBuffer(initSegment))
77
EVENT(updateend)
88
RUN(sourceBuffer.appendBuffer(samples))
99
EVENT(updateend)
10-
EVENT(pause)
10+
EXPECTED (errorName == 'NotAllowedError') OK
11+
EXPECTED (true == 'true') OK
1112
END OF TEST
1213

‎LayoutTests/media/media-source/media-source-restrictions.html‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,39 +9,29 @@
99
var sourceBuffer;
1010
var initSegment;
1111
var samples;
12+
var errorName;
1213

1314
if (window.internals)
1415
internals.initializeMockMediaSource();
1516

16-
function runTest() {
17+
async function runTest() {
1718
findMediaElement();
1819

1920
run(`internals.setMediaElementRestrictions(video, 'RequireUserGestureForAudioRateChange')`);
2021

2122
source = new MediaSource();
2223
testExpected('source.readyState', 'closed');
2324

24-
waitForEventOn(source, 'sourceopen', sourceOpen);
2525
video.src = URL.createObjectURL(source);
26-
}
26+
await waitFor(source, 'sourceopen');
2727

28-
function handlePause() {
29-
endTest();
30-
}
28+
waitFor(video, "ended").then(failTest);
3129

32-
function sourceOpen() {
3330
run('sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock")');
34-
waitForEventOn(sourceBuffer, 'updateend', loadSamples, false, true);
3531
initSegment = makeAInit(8, [makeATrack(1, 'mock', TRACK_KIND.AUDIO)]);
3632
run('sourceBuffer.appendBuffer(initSegment)');
33+
await waitFor(sourceBuffer, 'updateend');
3734

38-
waitFor(video, 'pause', false).then(handlePause);
39-
video.play().catch(function(error) {
40-
failTest();
41-
});
42-
}
43-
44-
function loadSamples() {
4535
samples = concatenateSamples([
4636
makeASample(0, 0, 1, 1, 1, SAMPLE_FLAG.SYNC),
4737
makeASample(1, 1, 1, 1, 1, SAMPLE_FLAG.SYNC),
@@ -53,12 +43,15 @@
5343
makeASample(7, 7, 1, 1, 1, SAMPLE_FLAG.SYNC),
5444
]);
5545

56-
waitFor(sourceBuffer, 'updateend', false).then(updateEnd);
5746
run('sourceBuffer.appendBuffer(samples)');
58-
}
47+
await waitFor(sourceBuffer, 'updateend');
5948

60-
function updateEnd() {
61-
waitForEventAndFail("onended");
49+
video.play().catch(function(error) {
50+
errorName = error.name;
51+
testExpected('errorName', "NotAllowedError");
52+
testExpected(error instanceof DOMException, true);
53+
endTest();
54+
});
6255
}
6356
</script>
6457
</head>

‎Source/WebCore/platform/graphics/MediaPlayerPrivate.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class MediaPlayerPrivateInterface : public AbstractRefCounted {
6666
#if ENABLE(MEDIA_SOURCE)
6767
virtual void load(const URL&, const LoadOptions&, MediaSourcePrivateClient&) = 0;
6868
virtual void readyStateFromMediaSourceChanged() { }
69+
virtual void characteristicsFromMediaSourceChanged() { }
6970
#endif
7071
#if ENABLE(MEDIA_STREAM)
7172
virtual void load(MediaStreamPrivate&) = 0;

‎Source/WebCore/platform/graphics/MediaSourcePrivate.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,15 @@ void MediaSourcePrivate::updateTracksType()
192192
TracksType tracksCombinedTypes;
193193
for (auto type : m_tracksTypes.values())
194194
tracksCombinedTypes |= type;
195-
m_tracksCombinedTypes = tracksCombinedTypes;
195+
if (m_tracksCombinedTypes.exchange(tracksCombinedTypes) != tracksCombinedTypes) {
196+
ensureOnMainThread([weakThis = ThreadSafeWeakPtr { *this }] {
197+
RefPtr protectedThis = weakThis.get();
198+
if (!protectedThis)
199+
return;
200+
if (RefPtr player = protectedThis->player())
201+
player->characteristicsFromMediaSourceChanged();
202+
});
203+
}
196204
}
197205

198206
void MediaSourcePrivate::durationChanged(const MediaTime& duration)

‎Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ class MediaPlayerPrivateMediaSourceAVFObjC
111111

112112
void effectiveRateChanged();
113113
void setNaturalSize(const FloatSize&);
114-
void characteristicsChanged();
114+
void characteristicsFromMediaSourceChanged() final;
115115

116116
MediaTime currentTime() const override;
117117
MediaTime currentOrPendingSeekTime() const final { return currentTime(); }

‎Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ void getSupportedTypes(HashSet<String>& types) const final
571571

572572
m_seeking = false;
573573

574-
if (auto player = m_player.get()) {
574+
if (RefPtr player = m_player.get()) {
575575
player->seeked(seekedTime);
576576
player->timeChanged();
577577
}
@@ -616,7 +616,7 @@ void getSupportedTypes(HashSet<String>& types) const final
616616
{
617617
assertIsMainThread();
618618
ALWAYS_LOG(LOGIDENTIFIER, preservesPitch);
619-
if (auto player = m_player.get())
619+
if (RefPtr player = m_player.get())
620620
m_renderer->setPreservesPitchAndCorrectionAlgorithm(preservesPitch, player->pitchCorrectionAlgorithm());
621621
}
622622

@@ -861,7 +861,7 @@ void getSupportedTypes(HashSet<String>& types) const final
861861
void MediaPlayerPrivateMediaSourceAVFObjC::notifyActiveSourceBuffersChanged()
862862
{
863863
assertIsMainThread();
864-
if (auto player = m_player.get())
864+
if (RefPtr player = m_player.get())
865865
player->activeSourceBuffersChanged();
866866
}
867867

@@ -913,7 +913,7 @@ void getSupportedTypes(HashSet<String>& types) const final
913913
if (!m_hasAvailableVideoFrame)
914914
return;
915915

916-
auto player = m_player.get();
916+
RefPtr player = m_player.get();
917917
if (player)
918918
player->firstVideoFrameAvailable();
919919

@@ -935,7 +935,7 @@ void getSupportedTypes(HashSet<String>& types) const final
935935
// Avoid emiting durationchanged in the case where the previous duration was unknown as that case is already handled
936936
// by the HTMLMediaElement.
937937
if (m_duration != duration && m_duration.isValid()) {
938-
if (auto player = m_player.get())
938+
if (RefPtr player = m_player.get())
939939
player->durationChanged();
940940
}
941941
m_duration = duration;
@@ -946,7 +946,7 @@ void getSupportedTypes(HashSet<String>& types) const final
946946
{
947947
assertIsMainThread();
948948
ALWAYS_LOG(LOGIDENTIFIER, effectiveRate());
949-
if (auto player = m_player.get())
949+
if (RefPtr player = m_player.get())
950950
player->rateChanged();
951951
}
952952

@@ -959,7 +959,7 @@ void getSupportedTypes(HashSet<String>& types) const final
959959
ALWAYS_LOG(LOGIDENTIFIER, size);
960960

961961
m_naturalSize = size;
962-
if (auto player = m_player.get())
962+
if (RefPtr player = m_player.get())
963963
player->sizeChanged();
964964
}
965965

@@ -987,7 +987,7 @@ void getSupportedTypes(HashSet<String>& types) const final
987987
#if ENABLE(LEGACY_ENCRYPTED_MEDIA) || ENABLE(ENCRYPTED_MEDIA)
988988
void MediaPlayerPrivateMediaSourceAVFObjC::keyNeeded(const SharedBuffer& initData)
989989
{
990-
if (auto player = m_player.get())
990+
if (RefPtr player = m_player.get())
991991
player->keyNeeded(initData);
992992
}
993993
#endif
@@ -1024,14 +1024,14 @@ void getSupportedTypes(HashSet<String>& types) const final
10241024
void MediaPlayerPrivateMediaSourceAVFObjC::waitingForKeyChanged()
10251025
{
10261026
ALWAYS_LOG(LOGIDENTIFIER);
1027-
if (auto player = m_player.get())
1027+
if (RefPtr player = m_player.get())
10281028
player->waitingForKeyChanged();
10291029
}
10301030

10311031
void MediaPlayerPrivateMediaSourceAVFObjC::initializationDataEncountered(const String& initDataType, RefPtr<ArrayBuffer>&& initData)
10321032
{
10331033
ALWAYS_LOG(LOGIDENTIFIER, initDataType);
1034-
if (auto player = m_player.get())
1034+
if (RefPtr player = m_player.get())
10351035
player->initializationDataEncountered(initDataType, WTFMove(initData));
10361036
}
10371037
#endif
@@ -1074,7 +1074,7 @@ void getSupportedTypes(HashSet<String>& types) const final
10741074
return;
10751075
}
10761076

1077-
if (auto player = m_player.get())
1077+
if (RefPtr player = m_player.get())
10781078
player->readyStateChanged();
10791079
}
10801080

@@ -1086,7 +1086,7 @@ void getSupportedTypes(HashSet<String>& types) const final
10861086

10871087
ALWAYS_LOG(LOGIDENTIFIER, networkState);
10881088
m_networkState = networkState;
1089-
if (auto player = m_player.get())
1089+
if (RefPtr player = m_player.get())
10901090
player->networkStateChanged();
10911091
}
10921092

@@ -1120,28 +1120,28 @@ void getSupportedTypes(HashSet<String>& types) const final
11201120
void MediaPlayerPrivateMediaSourceAVFObjC::removeAudioTrack(AudioTrackPrivate& track)
11211121
{
11221122
assertIsMainThread();
1123-
if (auto player = m_player.get())
1123+
if (RefPtr player = m_player.get())
11241124
player->removeAudioTrack(track);
11251125
}
11261126

11271127
void MediaPlayerPrivateMediaSourceAVFObjC::removeVideoTrack(VideoTrackPrivate& track)
11281128
{
11291129
assertIsMainThread();
1130-
if (auto player = m_player.get())
1130+
if (RefPtr player = m_player.get())
11311131
player->removeVideoTrack(track);
11321132
}
11331133

11341134
void MediaPlayerPrivateMediaSourceAVFObjC::removeTextTrack(InbandTextTrackPrivate& track)
11351135
{
11361136
assertIsMainThread();
1137-
if (auto player = m_player.get())
1137+
if (RefPtr player = m_player.get())
11381138
player->removeTextTrack(track);
11391139
}
11401140

1141-
void MediaPlayerPrivateMediaSourceAVFObjC::characteristicsChanged()
1141+
void MediaPlayerPrivateMediaSourceAVFObjC::characteristicsFromMediaSourceChanged()
11421142
{
11431143
assertIsMainThread();
1144-
if (auto player = m_player.get())
1144+
if (RefPtr player = m_player.get())
11451145
player->characteristicChanged();
11461146
}
11471147

@@ -1187,7 +1187,7 @@ void getSupportedTypes(HashSet<String>& types) const final
11871187
ALWAYS_LOG(LOGIDENTIFIER, shouldPlayToTarget);
11881188
m_shouldPlayToTarget = shouldPlayToTarget;
11891189

1190-
if (auto player = m_player.get())
1190+
if (RefPtr player = m_player.get())
11911191
player->currentPlaybackTargetIsWirelessChanged(isCurrentPlaybackTargetWireless());
11921192
}
11931193

@@ -1217,7 +1217,7 @@ void getSupportedTypes(HashSet<String>& types) const final
12171217
void MediaPlayerPrivateMediaSourceAVFObjC::audioOutputDeviceChanged()
12181218
{
12191219
#if HAVE(AUDIO_OUTPUT_DEVICE_UNIQUE_ID)
1220-
auto player = m_player.get();
1220+
RefPtr player = m_player.get();
12211221
if (!player)
12221222
return;
12231223
auto deviceId = player->audioOutputDeviceId();
@@ -1243,7 +1243,7 @@ void getSupportedTypes(HashSet<String>& types) const final
12431243
void MediaPlayerPrivateMediaSourceAVFObjC::checkNewVideoFrameMetadata(MediaTime presentationTime, double displayTime)
12441244
{
12451245
assertIsMainThread();
1246-
auto player = m_player.get();
1246+
RefPtr player = m_player.get();
12471247
if (!player)
12481248
return;
12491249

‎Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,9 +283,6 @@
283283

284284
setTrackChangeCallbacks(*segment, true);
285285
}
286-
callOnMainThreadWithPlayer([](auto& player) {
287-
player.characteristicsChanged();
288-
});
289286

290287
ALWAYS_LOG(LOGIDENTIFIER, "initialization segment was processed");
291288
}

‎Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,13 @@ void MediaPlayerPrivateGStreamerMSE::notifyActiveSourceBuffersChanged()
685685
player->activeSourceBuffersChanged();
686686
}
687687

688+
void MediaPlayerPrivateGStreamerMSE::characteristicsFromMediaSourceChanged()
689+
{
690+
assertIsMainThread();
691+
if (RefPtr player = m_player.get())
692+
player->characteristicChanged();
693+
}
694+
688695
#undef GST_CAT_DEFAULT
689696

690697
} // namespace WebCore.

0 commit comments

Comments
 (0)