ASSERTION: Progress timer should've been stopped (mse/webaudio)

RESOLVED FIXED in mozilla36

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ajones, Assigned: jwwang)

Tracking

(Blocks 1 bug)

Trunk
mozilla36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Steps to reproduce:

* Run Firefox (with MSE enabled)
* Watch a video (check stats for nerds Dash: Yes)
* Exit

Expected results:

Nothing special

Actual results:

[Child 23404] ###!!! ASSERTION: Progress timer should've been stopped.: '!mProgressTimer', file /home/ajones/src/mozilla-central/content/media/MediaDecoder.cpp, line 503
(Assignee)

Updated

5 years ago
Assignee: nobody → jwwang
(Assignee)

Comment 1

5 years ago
Since MediaShutdownManager will call MediaDecoder::Shutdown() without checking network status as the media element does, I will remove the assertion and stop the progress timer if necessary.
Attachment #8510832 - Flags: review?(cpearce)
(Assignee)

Updated

5 years ago
Attachment #8510832 - Flags: review?(cpearce)
(Assignee)

Comment 3

5 years ago
Since MediaShutdownManager will call MediaDecoder::Shutdown() without checking network status as the media element does, I will remove the assertion and stop the progress timer if necessary. Since HTMLMediaElement::~HTMLMediaElement() could come after MediaShutdownManager::Shutdown, HTMLMediaElement::ShutdownDecoder() should not call mDecoder->StopProgress either.
Attachment #8510832 - Attachment is obsolete: true
(Assignee)

Comment 4

5 years ago
Comment on attachment 8511881 [details] [diff] [review]
1088481_fix_MediaDecoder_Progress_time-v2.patch

Review of attachment 8511881 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/html/HTMLMediaElement.cpp
@@ -609,5 @@
> -  // TODO: This should be handled by ChangeNetworkState() so we have only one
> -  // place to call StopProgress().
> -  if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
> -    mDecoder->StopProgress();
> -  }

HTMLMediaElement::ShutdownDecoder could happen after MediaDecoder::Shutdown which has stopped the progress timer. So we remove the code and let MediaDecoder::Shutdown handle it.

::: dom/media/MediaDecoder.cpp
@@ +498,5 @@
>    }
>  
>    ChangeState(PLAY_STATE_SHUTDOWN);
>  
> +  if (mProgressTimer) {

The progress timer should be associated with the network state. However, MediaDecoder::Shutdown could be called by MediaShutdownManager without checking network status as the media element. So we stop it anyway if necessary.
Attachment #8511881 - Flags: review?(cpearce)
Attachment #8511881 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/b51eff4a2ec0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.