Bug 1507093 - P4. Don't lazily allocate mVideoTrackList and mAudioTrackList. r=pehrsons

Should the media element be cycle collected and a decoder still be active, a change of state could have caused the track list to be re-created causing a cycle between the HTMLMediaElement and the track list.

We also check when potentially updating the ready state if the self reference is still needed.

Place various assertions.

Differential Revision: https://phabricator.services.mozilla.com/D11860
This commit is contained in:
Jean-Yves Avenard
2018-11-15 12:48:34 +00:00
parent e81afc16f2
commit e3ef685f6c
5 changed files with 41 additions and 28 deletions

View File

@@ -1767,8 +1767,12 @@ HTMLMediaElement::AbortExistingLoads()
// video track list, no events (in particular, no removetrack events) are
// fired as part of this. Ending MediaStream sends track ended notifications,
// so we empty the track lists prior.
AudioTracks()->EmptyTracks();
VideoTracks()->EmptyTracks();
if (AudioTracks()) {
AudioTracks()->EmptyTracks();
}
if (VideoTracks()) {
VideoTracks()->EmptyTracks();
}
if (mDecoder) {
fireTimeUpdate = mDecoder->GetCurrentTime() != 0.0;
@@ -2246,6 +2250,7 @@ HTMLMediaElement::NotifyMediaTrackDisabled(MediaTrack* aTrack)
if (aTrack->AsAudioTrack()) {
// If we don't have any alive track , we don't need to mute MediaElement.
MOZ_DIAGNOSTIC_ASSERT(AudioTracks(), "Element can't have been unlinked");
if (AudioTracks()->Length() > 0) {
bool shouldMute = true;
for (uint32_t i = 0; i < AudioTracks()->Length(); ++i) {
@@ -3601,6 +3606,7 @@ HTMLMediaElement::CaptureStreamInternal(StreamCaptureBehavior aFinishBehavior,
}
if (mSrcStream) {
MOZ_DIAGNOSTIC_ASSERT(AudioTracks(), "Element can't have been unlinked");
for (size_t i = 0; i < AudioTracks()->Length(); ++i) {
AudioTrack* t = (*AudioTracks())[i];
if (t->Enabled()) {
@@ -3608,6 +3614,7 @@ HTMLMediaElement::CaptureStreamInternal(StreamCaptureBehavior aFinishBehavior,
}
}
if (IsVideo() && !out->mCapturingAudioOnly) {
MOZ_DIAGNOSTIC_ASSERT(VideoTracks(), "Element can't have been unlinked");
// Only add video tracks if we're a video element and the output stream
// wants video.
for (size_t i = 0; i < VideoTracks()->Length(); ++i) {
@@ -3919,6 +3926,8 @@ HTMLMediaElement::HTMLMediaElement(
, mShutdownObserver(new ShutdownObserver)
, mPlayed(new TimeRanges(ToSupports(OwnerDoc())))
, mPaused(true, "HTMLMediaElement::mPaused")
, mAudioTrackList(new AudioTrackList(OwnerDoc()->GetParentObject(), this))
, mVideoTrackList(new VideoTrackList(OwnerDoc()->GetParentObject(), this))
, mErrorSink(new ErrorSink(this))
, mAudioChannelWrapper(new AudioChannelAgentCallback(this))
, mSink(MakePair(nsString(), RefPtr<AudioDeviceInfo>()))
@@ -5510,6 +5519,7 @@ HTMLMediaElement::NotifyMediaStreamTrackAdded(
#endif
if (AudioStreamTrack* t = aTrack->AsAudioStreamTrack()) {
MOZ_DIAGNOSTIC_ASSERT(AudioTracks(), "Element can't have been unlinked");
RefPtr<AudioTrack> audioTrack =
CreateAudioTrack(t, AudioTracks()->GetOwnerGlobal());
AudioTracks()->AddTrack(audioTrack);
@@ -5518,6 +5528,7 @@ HTMLMediaElement::NotifyMediaStreamTrackAdded(
if (!IsVideo()) {
return;
}
MOZ_DIAGNOSTIC_ASSERT(VideoTracks(), "Element can't have been unlinked");
RefPtr<VideoTrack> videoTrack =
CreateVideoTrack(t, VideoTracks()->GetOwnerGlobal());
VideoTracks()->AddTrack(videoTrack);
@@ -5547,6 +5558,8 @@ HTMLMediaElement::NotifyMediaStreamTrackRemoved(
aTrack->AsAudioStreamTrack() ? "Audio" : "Video",
NS_ConvertUTF16toUTF8(id).get()));
MOZ_DIAGNOSTIC_ASSERT(AudioTracks() && VideoTracks(),
"Element can't have been unlinked");
if (MediaTrack* t = AudioTracks()->GetTrackById(id)) {
AudioTracks()->RemoveTrack(t);
} else if (MediaTrack* t = VideoTracks()->GetTrackById(id)) {
@@ -5639,14 +5652,17 @@ HTMLMediaElement::MetadataLoaded(const MediaInfo* aInfo,
if (!mSrcStream) {
return;
}
for (OutputMediaStream& ms : mOutputStreams) {
for (size_t i = 0; i < AudioTracks()->Length(); ++i) {
AudioTrack* t = (*AudioTracks())[i];
if (t->Enabled()) {
AddCaptureMediaTrackToOutputStream(t, ms);
if (AudioTracks()) {
for (size_t i = 0; i < AudioTracks()->Length(); ++i) {
AudioTrack* t = (*AudioTracks())[i];
if (t->Enabled()) {
AddCaptureMediaTrackToOutputStream(t, ms);
}
}
}
if (IsVideo() && !ms.mCapturingAudioOnly) {
if (VideoTracks() && IsVideo() && !ms.mCapturingAudioOnly) {
// Only add video tracks if we're a video element and the output stream
// wants video.
for (size_t i = 0; i < VideoTracks()->Length(); ++i) {
@@ -5706,8 +5722,12 @@ HTMLMediaElement::DecodeError(const MediaResult& aError)
DecoderDoctorDiagnostics diagnostics;
diagnostics.StoreDecodeError(OwnerDoc(), aError, src, __func__);
AudioTracks()->EmptyTracks();
VideoTracks()->EmptyTracks();
if (AudioTracks()) {
AudioTracks()->EmptyTracks();
}
if (VideoTracks()) {
VideoTracks()->EmptyTracks();
}
if (mIsLoadingFromSourceChildren) {
mErrorSink->ResetError();
if (mSourceLoadCandidate) {
@@ -6014,13 +6034,15 @@ HTMLMediaElement::UpdateReadyStateInternal()
return;
}
bool hasAudioTracks = !AudioTracks()->IsEmpty();
bool hasVideoTracks = !VideoTracks()->IsEmpty();
bool hasAudioTracks = AudioTracks() && !AudioTracks()->IsEmpty();
bool hasVideoTracks = VideoTracks() && !VideoTracks()->IsEmpty();
if (!hasAudioTracks && !hasVideoTracks) {
LOG(LogLevel::Debug,
("MediaElement %p UpdateReadyStateInternal() "
"Stream with no tracks",
this));
// Give it one last chance to remove the self reference if needed.
AddRemoveSelfReference();
return;
}
@@ -7554,22 +7576,12 @@ HTMLMediaElement::NotifyWaitingForKey()
AudioTrackList*
HTMLMediaElement::AudioTracks()
{
if (!mAudioTrackList) {
nsCOMPtr<nsPIDOMWindowInner> window =
do_QueryInterface(OwnerDoc()->GetParentObject());
mAudioTrackList = new AudioTrackList(window, this);
}
return mAudioTrackList;
}
VideoTrackList*
HTMLMediaElement::VideoTracks()
{
if (!mVideoTrackList) {
nsCOMPtr<nsPIDOMWindowInner> window =
do_QueryInterface(OwnerDoc()->GetParentObject());
mVideoTrackList = new VideoTrackList(window, this);
}
return mVideoTrackList;
}