Skip to content

Commit

Permalink
REGRESSION (268393@main): [ Monterey Ventura Debug wk2 ] ASSERTION FA…
Browse files Browse the repository at this point in the history
…ILED: willBeComposited == needsToBeComposited(layer, queryData)

https://bugs.webkit.org/show_bug.cgi?id=267661
rdar://121154236

Reviewed by Eric Carlson and Simon Fraser.

It was possible the MediaPlayerPrivate's accelerated rendering capability
to change without a required match to MediaPlayer::renderingModeChanged.

We were querying the old value of MediaPlayer::renderingCanBeAccelerated
after modifying a class member that would impact what the new value would be.
Compare with the previous value and call renderingModeChanged if required.

Covered by existing tests.

* LayoutTests/platform/ios/TestExpectations: Remove obsolete expectations as original bug got fixed.
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm: Remove unnecessary call to renderModeChanged as removing the audio layer doesn't impact how images are rendered.
* Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm:
(WebCore::MediaPlayerPrivateWebM::destroyAudioRenderers): Same as above.
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::MediaPlayerPrivateRemote): There's no need to send to the GPU process that the rendering has mode has changed
as both content process and GPU process' player values are initialised to false.
(WebKit::MediaPlayerPrivateRemote::readyStateChanged):
(WebKit::MediaPlayerPrivateRemote::acceleratedRenderingStateChanged):
(WebKit::MediaPlayerPrivateRemote::updateConfiguration):
(WebKit::MediaPlayerPrivateRemote::setVideoFullscreenLayer):
(WebKit::MediaPlayerPrivateRemote::checkAcceleratedRenderingState): Deleted.
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:

Canonical link: https://commits.webkit.org/273406@main
  • Loading branch information
jyavenard committed Jan 24, 2024
1 parent 75f7d90 commit 6bf767b
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 27 deletions.
3 changes: 1 addition & 2 deletions LayoutTests/platform/ios/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -3366,8 +3366,7 @@ http/tests/websocket/tests/hybi/workers/worker-reload.html [ Pass Crash ]
http/tests/workers/service/service-worker-download-async-delegates.https.html [ Pass Crash ]

# rdar://80396393 ([ iOS15 ] http/wpt/mediarecorder/mute-tracks.html is a flaky failure)
# webkit.org/b/238774 [ iOS ] http/wpt/mediarecorder/mute-tracks.html is a flaky crash
http/wpt/mediarecorder/mute-tracks.html [ Pass Failure Crash ]
http/wpt/mediarecorder/mute-tracks.html [ Pass Failure ]

# rdar://80396502 ([ iOS15 ] http/wpt/mediarecorder/pause-recording.html is a flaky crash)
http/wpt/mediarecorder/pause-recording.html [ Pass Crash ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1361,8 +1361,6 @@ void getSupportedTypes(HashSet<String>& types) const final
[m_synchronizer removeRenderer:audioRenderer atTime:currentTime completionHandler:nil];

m_sampleBufferAudioRendererMap.remove(iter);
if (auto player = m_player.get())
player->renderingModeChanged();
}

void MediaPlayerPrivateMediaSourceAVFObjC::removeAudioTrack(AudioTrackPrivate& track)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1562,8 +1562,6 @@ static bool isCopyDisplayedPixelBufferAvailable()
destroyAudioRenderer(renderer);
}
m_audioRenderers.clear();
if (auto player = m_player.get())
player->renderingModeChanged();
}

void MediaPlayerPrivateWebM::clearTracks()
Expand Down
26 changes: 7 additions & 19 deletions Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ MediaPlayerPrivateRemote::MediaPlayerPrivateRemote(MediaPlayer* player, MediaPla
, m_documentSecurityOrigin(player->documentSecurityOrigin())
{
ALWAYS_LOG(LOGIDENTIFIER);

acceleratedRenderingStateChanged();
}

MediaPlayerPrivateRemote::~MediaPlayerPrivateRemote()
Expand Down Expand Up @@ -345,10 +343,8 @@ void MediaPlayerPrivateRemote::readyStateChanged(RemoteMediaPlayerState&& state)
{
ALWAYS_LOG(LOGIDENTIFIER, state.readyState);
updateCachedState(WTFMove(state));
if (auto player = m_player.get()) {
if (auto player = m_player.get())
player->readyStateChanged();
checkAcceleratedRenderingState();
}
}

void MediaPlayerPrivateRemote::volumeChanged(double volume)
Expand Down Expand Up @@ -491,26 +487,17 @@ bool MediaPlayerPrivateRemote::supportsAcceleratedRendering() const
void MediaPlayerPrivateRemote::acceleratedRenderingStateChanged()
{
if (auto player = m_player.get()) {
m_renderingCanBeAccelerated = player->renderingCanBeAccelerated();
connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(m_renderingCanBeAccelerated), m_id);
}
renderingModeChanged();
}

void MediaPlayerPrivateRemote::checkAcceleratedRenderingState()
{
if (auto player = m_player.get()) {
bool renderingCanBeAccelerated = player->renderingCanBeAccelerated();
if (m_renderingCanBeAccelerated != renderingCanBeAccelerated)
acceleratedRenderingStateChanged();
connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(player->renderingCanBeAccelerated()), m_id);
}
}

void MediaPlayerPrivateRemote::updateConfiguration(RemoteMediaPlayerConfiguration&& configuration)
{
bool oldSupportsAcceleraterRendering = supportsAcceleratedRendering();
m_configuration = WTFMove(configuration);
// player->renderingCanBeAccelerated() result is dependent on m_configuration.supportsAcceleratedRendering value.
checkAcceleratedRenderingState();
if (RefPtr player = m_player.get(); player && oldSupportsAcceleraterRendering != supportsAcceleratedRendering())
player->renderingModeChanged();
}

#if ENABLE(WIRELESS_PLAYBACK_TARGET)
Expand Down Expand Up @@ -886,7 +873,8 @@ void MediaPlayerPrivateRemote::setVideoFullscreenLayer(PlatformLayer* videoFulls
m_videoLayerManager->setVideoFullscreenLayer(videoFullscreenLayer, WTFMove(completionHandler), nullptr);
#endif
// A Fullscreen layer has been set, this could update the value returned by player->renderingCanBeAccelerated().
checkAcceleratedRenderingState();
if (RefPtr player = m_player.get())
player->renderingModeChanged();
}

void MediaPlayerPrivateRemote::updateVideoFullscreenInlineImage()
Expand Down
2 changes: 0 additions & 2 deletions Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ class MediaPlayerPrivateRemote final

bool supportsAcceleratedRendering() const final;
void acceleratedRenderingStateChanged() final;
void checkAcceleratedRenderingState();

void setShouldMaintainAspectRatio(bool) final;

Expand Down Expand Up @@ -481,7 +480,6 @@ class MediaPlayerPrivateRemote final
bool m_isCurrentPlaybackTargetWireless { false };
bool m_waitingForKey { false };
bool m_timeIsProgressing { false };
bool m_renderingCanBeAccelerated { false };
std::optional<bool> m_shouldMaintainAspectRatio;
std::optional<bool> m_pageIsVisible;
RefPtr<RemoteVideoFrameProxy> m_videoFrameForCurrentTime;
Expand Down

0 comments on commit 6bf767b

Please sign in to comment.