Closed Bug 1303970 Opened 8 years ago Closed 8 years ago

Intermittent dom/media/test/test_audio2.html | Assertion count 1 is greater than expected range 0-0 assertions.

Categories

(Core :: Audio/Video, defect, P3)

Product:
Core
Component:
Audio/Video
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox50 --- unaffected
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: jya)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Bug 1303970: P1. MEDIA_ERR_ABORTED can be returned even if readyState is HAVE_NOTHING.
58 bytes, text/x-review-board-request
jwwang
: review+
gchang
: approval-mozilla-aurora+
Details
Bug 1303970: P2. Fix "If the media data fetching process is aborted by the user steps".
58 bytes, text/x-review-board-request
jwwang
: review+
Details
Assignee: nobody → jyavenard
Blocks: 1302632
Comment on attachment 8792830 [details]
Bug 1303970: P1. MEDIA_ERR_ABORTED can be returned even if readyState is HAVE_NOTHING.

https://reviewboard.mozilla.org/r/79722/#review78700

Can you explain the root cause of this issue?
you can see what's happening on the stack trace provided in the bug description

 20:41:43     INFO -  [Child 1774] ###!!! ASSERTION: Shouldn't be called when readyState is HAVE_NOTHING: 'mReadyState > HAVE_NOTHING', file /builds/slave/m-in-m64-d-0000000000000000000/build/src/dom/html/HTMLMediaElement.cpp, line 4471
 20:42:24     INFO -  #01: non-virtual thunk to mozilla::dom::HTMLMediaElement::LoadAborted() [xpcom/string/nsTSubstring.h:95]
 20:42:24     INFO -  #02: mozilla::detail::RunnableFunction<mozilla::MediaDecoder::ResourceCallback::NotifyDataEnded(nsresult)::$_2>::Run() [dom/media/MediaDecoder.cpp:251]

MediaDecoder::ResourceCallback::NotifyDataEnded calls LoadAborted() which calls Error()

HTMLMEdiaElement::Error now expects readyState to be >= HAVE_METADATA which isn't always true.

Additionally, the handling of abort was wrong anyway and the required events wasn't fired (that's what P2 fixes)
Comment on attachment 8792831 [details]
Bug 1303970: P2. Fix "If the media data fetching process is aborted by the user steps".

https://reviewboard.mozilla.org/r/79726/#review78706

::: dom/html/HTMLMediaElement.cpp:4489
(Diff revision 1)
>      return;
>    }
>    mError = new MediaError(this, aErrorCode, aErrorDetails);
>  
>    DispatchAsyncEvent(NS_LITERAL_STRING("error"));
> +  if (aErrorCode != MEDIA_ERR_ABORTED) {

This will prevent Error(MEDIA_ERR_ABORTED) from chaning networkState to NETWORK_IDLE when readyState > HAVE_NOTHING.

I think we should move the statement of #4461 into this function so we have only one place for error code handling (involing checking readyState and changing networkState).
Attachment #8792831 - Flags: review?(jwwang) → review-
Comment on attachment 8792830 [details]
Bug 1303970: P1. MEDIA_ERR_ABORTED can be returned even if readyState is HAVE_NOTHING.

https://reviewboard.mozilla.org/r/79722/#review78710

::: dom/html/HTMLMediaElement.cpp:4471
(Diff revision 1)
>    NS_ASSERTION(aErrorCode == MEDIA_ERR_DECODE ||
>                 aErrorCode == MEDIA_ERR_NETWORK ||
>                 aErrorCode == MEDIA_ERR_ABORTED,
>                 "Only use MediaError codes!");
> -  NS_ASSERTION(mReadyState > HAVE_NOTHING,
> +  NS_ASSERTION(mReadyState > HAVE_NOTHING || aErrorCode == MEDIA_ERR_ABORTED,
>                 "Shouldn't be called when readyState is HAVE_NOTHING");

Fix the comment or just remove it which is not very informative.
Attachment #8792830 - Flags: review?(jwwang) → review+
Comment on attachment 8792831 [details]
Bug 1303970: P2. Fix "If the media data fetching process is aborted by the user steps".

https://reviewboard.mozilla.org/r/79726/#review78706

> This will prevent Error(MEDIA_ERR_ABORTED) from chaning networkState to NETWORK_IDLE when readyState > HAVE_NOTHING.
> 
> I think we should move the statement of #4461 into this function so we have only one place for error code handling (involing checking readyState and changing networkState).

done
Comment on attachment 8792831 [details]
Bug 1303970: P2. Fix "If the media data fetching process is aborted by the user steps".

https://reviewboard.mozilla.org/r/79726/#review78714
Attachment #8792831 - Flags: review?(jwwang) → review+
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/673807c7e76f
P1. MEDIA_ERR_ABORTED can be returned even if readyState is HAVE_NOTHING. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/e2e14420350f
P2. Fix "If the media data fetching process is aborted by the user steps". r=jwwang
Patch P2 fixes a spec bug that got introduced in 51. It deserves a higher priority.
Priority: P5 → P3
https://hg.mozilla.org/mozilla-central/rev/673807c7e76f
https://hg.mozilla.org/mozilla-central/rev/e2e14420350f
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8792830 [details]
Bug 1303970: P1. MEDIA_ERR_ABORTED can be returned even if readyState is HAVE_NOTHING.

Approval Request Comment
[Feature/regressing bug #]: 1302632
[User impact if declined]: Assertions, incorrect events being fired making us non HTML5 compliant
[Describe test coverage new/current, TreeHerder]: In central, manual test. 
[Risks and why]: Low. We have removed an assertion and re-enabled the firing of the proper event when the user abort playback.
[String/UUID change made/needed]: None
Attachment #8792830 - Flags: approval-mozilla-aurora?
Hi :jya,
Just for my clarification. Per comment #14, is the patch 2 included in the uplift request?
Flags: needinfo?(jyavenard)
(In reply to Gerry Chang [:gchang] from comment #17)
> Hi :jya,
> Just for my clarification. Per comment #14, is the patch 2 included in the
> uplift request?

yes... sorry for not indicating that earlier (I thought I did)
Flags: needinfo?(jyavenard)
Hi :jya,
Do you need to create another uplift request for patch2?
Flags: needinfo?(jyavenard)
I don't no. The comment and description in the uplift request already mentioned the changes added by those two patches.
Flags: needinfo?(jyavenard)
Comment on attachment 8792830 [details]
Bug 1303970: P1. MEDIA_ERR_ABORTED can be returned even if readyState is HAVE_NOTHING.

The patch fixes a intermittent-failure. Take it in 51 aurora for both patches.
Attachment #8792830 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: