Closed Bug 600020 Opened 10 years ago Closed 9 years ago

"ASSERTION: Must know the source we were loading from"

Categories

(Core :: Audio/Video, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla6

People

(Reporter: jruderman, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file testcase
This is a variant on bug 593305 that triggers an additional assertion.  (The second assertion is the "new" one.)

###!!! ASSERTION: Load event should not be delayed at start of resource selection.: '!mDelayingLoadEvent', file content/html/content/src/nsHTMLMediaElement.cpp, line 632

###!!! ASSERTION: Must know the source we were loading from!: 'mSourceLoadCandidate', file content/html/content/src/nsHTMLMediaElement.cpp, line 688
Attached file stack traces
Thanks for the testcase Jesse!
The assertion "Must know the source we were loading from!" no longer occurs thanks to the landing of bug 604067. The "Must know the source we were loading from" assertion still occurs.
(In reply to comment #3)
> The assertion "Must know the source we were loading from!" no longer occurs
> thanks to the landing of bug 604067. The "Must know the source we were loading
> from" assertion still occurs.

Oops, I mean the assertion "Load event should not be delayed at start of resource
selection." still occurs.
Now I get this assertion instead, which seems like it's kind of the opposite:

###!!! ASSERTION: Load event not delayed during source selection?: 'mDelayingLoadEvent', file content/html/content/src/nsHTMLMediaElement.cpp, line 527
(In reply to comment #5)
> Now I get this assertion instead, which seems like it's kind of the opposite:
> 
> ###!!! ASSERTION: Load event not delayed during source selection?:
> 'mDelayingLoadEvent', file content/html/content/src/nsHTMLMediaElement.cpp,
> line 527

This occurs because we're receiving an OnStartRequest for a channel which has been cancelled because we've started a new load. We're dispatching an error event in this case to the element (which is where we're triggering the assertion), even though the element has started a new load, and isn't necessarily in an error state. We should not dispatch an error event in the OnStartRequest for a channel which is no longer the currently loading channel; we'll have dispatched "emptied" and "abort" events when the new load aborts/cancels the old load.
Attached patch Patch v1Splinter Review
* Abort in an nsMediaLoadListener's OnStartRequest() (without firing an error event) if the load has been cancelled.

Patch is greenish on Try
http://tbpl.mozilla.org/?tree=Try&rev=e99e885e06a4
Assignee: nobody → chris
Status: NEW → ASSIGNED
Attachment #529913 - Flags: review?(roc)
Comment on attachment 529913 [details] [diff] [review]
Patch v1

Review of attachment 529913 [details] [diff] [review]:

Can we check in a test based on Jesse's test?
Attachment #529913 - Flags: review?(roc) → review+
(In reply to comment #8)
> Can we check in a test based on Jesse's test?

No need really. This assertion was randomly failing with my patch for bug 652751 applied in content/media/test/crashtests/459431-1.html, so that effectively is a testcase.
http://hg.mozilla.org/mozilla-central/rev/eb15fab87e7e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
(In reply to comment #9)
> This assertion was randomly failing with my patch for bug 652751 applied 

Oops, I meant with my patch for bug 650994 applied.
You need to log in before you can comment on or make changes to this bug.