Skip to content
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.

Sign up for GitHub

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

Jump to bottom

REGRESSION(260575@main): TestWebKitAPI.AudioRoutingArbitration.Deletion times out with UI-side-compositing enabled #11325

Merged
merged 1 commit into from
Mar 10, 2023
Merged

REGRESSION(260575@main): TestWebKitAPI.AudioRoutingArbitration.Deletion times out with UI-side-compositing enabled #11325

merged 1 commit into from
Mar 10, 2023

Conversation

jernoble
Copy link
Contributor

@jernoble jernoble commented Mar 9, 2023 •
edited by webkit-early-warning-system

d06c4a7

REGRESSION(260575@main): TestWebKitAPI.AudioRoutingArbitration.Deletion times out with UI-side-compositing enabled
https://bugs.webkit.org/show_bug.cgi?id=253670
rdar://106322713

Reviewed by Ryosuke Niwa.

When we did the "don't host layers from the WebContent process" work, we re-used the
PlaybackSessionManager/VideoFullscreenManager codepath from element fullscreen and
picture-in-picture to handle creation and sizing of the video layer in the UI process. This caused
a pre-existing bug where a fullscreened HTMLMediaElement would never be destroyed (due to having
eventListeners) to now apply to all HTMLMediaElements displayed in a page.

The eventListeners aren't removed because a boolean ivar intended to track whether eventListeners
were added was never set to true. Once that ivar is correctly set, the test passes.

* Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:
(WebCore::PlaybackSessionModelMediaElement::setMediaElement):

Canonical link: https://commits.webkit.org/261503@main

24d3f52

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
❌ 🛠 🧪 merge ✅ 🛠 watch-sim

@jernoble jernoble self-assigned this Mar 9, 2023
@jernoble jernoble added the Media Bugs related to the HTML 5 Media elements. label Mar 9, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 24d3f52)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
❌ 🛠 🧪 merge ✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 10, 2023
@jernoble
Copy link
Contributor Author

New failure is in fast/scrolling/keyboard-scrolling-distance-pageDown.html. Seems entirely unrelated. Landing.

@jernoble jernoble added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Mar 10, 2023
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION260575main-TestWebKitAPI-AudioRoutingArbitration-Deletion-times-out-with-UI-side-compositing-enabled branch from 24d3f52 to ee85c41 Compare March 10, 2023 17:13
@jernoble
REGRESSION(260575@main): TestWebKitAPI.AudioRoutingArbitration.Deleti…
d06c4a7 
…on times out with UI-side-compositing enabled

https://bugs.webkit.org/show_bug.cgi?id=253670
rdar://106322713

Reviewed by Ryosuke Niwa.

When we did the "don't host layers from the WebContent process" work, we re-used the
PlaybackSessionManager/VideoFullscreenManager codepath from element fullscreen and
picture-in-picture to handle creation and sizing of the video layer in the UI process. This caused
a pre-existing bug where a fullscreened HTMLMediaElement would never be destroyed (due to having
eventListeners) to now apply to all HTMLMediaElements displayed in a page.

The eventListeners aren't removed because a boolean ivar intended to track whether eventListeners
were added was never set to true. Once that ivar is correctly set, the test passes.

* Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:
(WebCore::PlaybackSessionModelMediaElement::setMediaElement):

Canonical link: https://commits.webkit.org/261503@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION260575main-TestWebKitAPI-AudioRoutingArbitration-Deletion-times-out-with-UI-side-compositing-enabled branch from ee85c41 to d06c4a7 Compare March 10, 2023 17:17
@webkit-commit-queue
Copy link
Collaborator

Committed 261503@main (d06c4a7): https://commits.webkit.org/261503@main

Reviewed commits have been landed. Closing PR #11325 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit d06c4a7 into WebKit:main Mar 10, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rniwa rniwa rniwa approved these changes

@jyavenard jyavenard Awaiting requested review from jyavenard

@eric-carlson eric-carlson Awaiting requested review from eric-carlson

Assignees

@jernoble jernoble

Labels
Media Bugs related to the HTML 5 Media elements.
Projects
None yet
Milestone
No milestone
5 participants
@jernoble @webkit-early-warning-system @webkit-commit-queue @rniwa @webkit-ews-buildbot