-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[ macOS EWS ] media/video-remove-insert-repaints.html is a flaky crash #17021
[ macOS EWS ] media/video-remove-insert-repaints.html is a flaky crash #17021
Conversation
EWS run on previous version of this PR (hash 7a7f93f) |
7a7f93f
to
50c2c09
Compare
EWS run on previous version of this PR (hash 50c2c09) |
if (!model || !interface) | ||
return; | ||
|
||
Ref videoElement = *model->videoElement(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could still crash, potentially. Maybe another ASSERT(model->videoElement()) and early return here?
50c2c09
to
d9af583
Compare
EWS run on previous version of this PR (hash d9af583) |
d9af583
to
1732994
Compare
EWS run on current version of this PR (hash 1732994) |
https://bugs.webkit.org/show_bug.cgi?id=260663 rdar://114387091 Reviewed by Jer Noble. If VideoFullscreenManager::removeContext is called with a contextId that doesn't exist in m_contextMap then a new model would be created with a null video element, leading to a crash when attempting to remove the video element from m_videoElements. Addressed this by returning early in VideoFullscreenManager::removeContext if no model/interface pair exists for the given contextId. Assert that this does not occur to help us track down the underlying issue in Debug builds (removeContext should not be called for a non-existent contextId). * Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.mm: (WebKit::PlaybackSessionManager::removeContext): * Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm: (WebKit::VideoFullscreenManager::removeContext): Canonical link: https://commits.webkit.org/267257@main
1732994
to
8f95bb7
Compare
Committed 267257@main (8f95bb7): https://commits.webkit.org/267257@main Reviewed commits have been landed. Closing PR #17021 and removing active labels. |
8f95bb7
1732994
π§ͺ wpe-wk2