Closed Bug 1215439 Opened 4 years ago Closed 4 years ago

Don't call ResetDecode() in the destructor of MediaDecoderReader

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(1 file)

http://www.artima.com/cppsource/nevercall.html

ResetDecode() is a virtual method and it is bad to call it in the destructor. Furthermore, I don't see the necessity to call it in the destructor.

nsresult MediaDecoderReader::ResetDecode()
{
  VideoQueue().Reset(); <-- we don't care about it since it will be destroyed soon.
  AudioQueue().Reset(); <-- ditto.

  mAudioDiscontinuity = true; <-- don't care
  mVideoDiscontinuity = true; <-- don't care

  mBaseAudioPromise.RejectIfExists(CANCELED, __func__); <-- already rejected in Shutdown()
  mBaseVideoPromise.RejectIfExists(CANCELED, __func__); <-- ditto.

  return NS_OK;
}

We should be fine without calling ResetDecode() in the destructor.
Assignee: nobody → jwwang
Blocks: 1213764
Bug 1215439 - Don't call ResetDecode() in the destructor of MediaDecoderReader. r=gerald.
Attachment #8675489 - Flags: review?(gsquelart)
Comment on attachment 8675489 [details]
MozReview Request: Bug 1215439 - Don't call ResetDecode() in the destructor of MediaDecoderReader. r=gerald.

https://reviewboard.mozilla.org/r/22381/#review19975

Makes sense.
Attachment #8675489 - Flags: review?(gsquelart) → review+
Thanks for the review!
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/8aa35040b3bf
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.