1618717 - Crash in [@ mozilla::dom::HTMLMediaElement::MediaControlEventListener::OnKeyPressed]

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Andrew McCreight [:mccr8]]

This bug is for crash report bp-2e95c2ae-7cf9-40b1-862a-3c5b90200227.

Top 10 frames of crashing thread:

0 XUL mozilla::dom::HTMLMediaElement::MediaControlEventListener::OnKeyPressed dom/html/HTMLMediaElement.cpp:467
1 XUL mozilla::dom::ContentMediaController::OnKeyPressed dom/media/mediacontrol/ContentMediaController.cpp:153
2 XUL mozilla::dom::PlaybackController::Pause dom/media/mediacontrol/PlaybackController.cpp:61
3 XUL mozilla::dom::MediaActionHandler::HandleMediaControlKeysEvent dom/media/mediacontrol/PlaybackController.cpp:129
4 XUL mozilla::dom::ContentChild::RecvUpdateMediaControlKeysEvent dom/ipc/ContentChild.cpp:3775
5 XUL mozilla::dom::PContentChild::OnMessageReceived ipc/ipdl/PContentChild.cpp:12781
6 XUL mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2214
7 XUL nsThread::ProcessNextEvent ipc/glue/MessageChannel.cpp:2006
8 XUL <name omitted> xpcom/threads/nsThreadUtils.cpp:481
9 XUL mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:109

This first showed up in the 20200226092757 Nightly. This is the set of changesets for that Nightly: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f3da8ae9d1a3e74cd273746da51a035ddc572bee&tochange=7f41334e10443f4f1c7426e86fb0cb7adfdf4d62

The crash is a null deref on this line: !Owner()->Paused(). Owner() is a weak reference to a media element.

I think this is a regression from bug 1535617, which clears weak references on Unlink. We must be calling OnKeyPressed on a MediaControlEventListener where its media element has gone away. Maybe this method just needs a null check on Owner() at the start of the method?

Comment 1

[Andrew McCreight [:mccr8]]

I don't see why this couldn't have happened before bug 1535617, but I don't see any evidence of it.

Comment 2

[Alastor Wu [:alwu]]

Thank you for reporting this, so it seems to me that we should stop the listener as well when unlink happens.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1618717 - stop and clear the listener during CC unlink process.

At the time CC unlink happens, we would no longer need to listen to the media control key events. Therefore, we should remember to stop and unlink the listener to avoid its any method ran after CC.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fbabca36fb1
stop and clear the listener during CC unlink process. r=bryce

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/0fbabca36fb1

Comment 6

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Hi Alastor, is there something that QA can verify here?

Comment 7

[Alastor Wu [:alwu]]

No, I don't have any way to reproduce this issue because you don't know when CC would happen.