"ASSERTION: Load event should not be delayed at start of resource selection"

RESOLVED FIXED in mozilla6

Status

()

Core
Audio/Video
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: cpearce)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla6
x86_64
Linux
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
Created attachment 471787 [details]
testcase

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

nsHTMLMediaElement::SelectResource [content/html/content/src/nsHTMLMediaElement.cpp:637]
nsSyncSection::Run [content/html/content/src/nsHTMLMediaElement.cpp:561]
nsBaseAppShell::RunSyncSections [widget/src/xpwidgets/nsBaseAppShell.cpp:349]
nsBaseAppShell::AfterProcessNextEvent [widget/src/xpwidgets/nsBaseAppShell.cpp:362]
nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:559]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:250]
mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:110]
MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:220]
MessageLoop::RunHandler [ipc/chromium/src/base/message_loop.cc:203]
MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:176]
nsBaseAppShell::Run [widget/src/xpwidgets/nsBaseAppShell.cpp:186]
nsAppStartup::Run [toolkit/components/startup/src/nsAppStartup.cpp:191]
XRE_main [toolkit/xre/nsAppRunner.cpp:3665]
main [browser/app/nsBrowserApp.cpp:158]
libc.so.6 + 0x1ec4d
firefox-bin + 0xd19

###!!! ASSERTION: Load event not delayed during source selection?: 'mDelayingLoadEvent', file content/html/content/src/nsHTMLMediaElement.cpp, line 530

nsHTMLMediaElement::NoSupportedMediaSourceError [content/html/content/src/nsHTMLMediaElement.cpp:530]
nsHTMLMediaElement::NotifyLoadError [content/html/content/src/nsHTMLMediaElement.cpp:690]
nsHTMLMediaElement::MediaLoadListener::OnStartRequest [content/html/content/src/nsHTMLMediaElement.cpp:302]
nsJSChannel::NotifyListener [dom/src/jsurl/nsJSProtocolHandler.cpp:855]
nsJSChannel::EvaluateScript [dom/src/jsurl/nsJSProtocolHandler.cpp:777]
nsRunnableMethodImpl<void (nsJSChannel::*)(), true>::Run [nsThreadUtils.h:348]
nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:547]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:250]
mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:110]
MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:220]
MessageLoop::RunHandler [ipc/chromium/src/base/message_loop.cc:203]
MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:176]
nsBaseAppShell::Run [widget/src/xpwidgets/nsBaseAppShell.cpp:186]
nsAppStartup::Run [toolkit/components/startup/src/nsAppStartup.cpp:191]
XRE_main [toolkit/xre/nsAppRunner.cpp:3665]
main [browser/app/nsBrowserApp.cpp:158]
libc.so.6 + 0x1ec4d
firefox-bin + 0xd19
Probably new from bug 485288 landing?
Blocks: 485288
(Assignee)

Comment 2

7 years ago
(In reply to comment #1)
> Probably new from bug 485288 landing?

Definitely. That didn't take long!
(Assignee)

Comment 3

6 years ago
My patch in bug 650994 causes this assertion to fire a lot in crashtest content/media/test/crashtests/459439-1.html, so I investigated this...

What happens there (and in Jesse's testcase here) is:
1. In an event we start a load, and then set a timeout to start a new load. Starting a new load queues up a "synchronous section", which will run immediately after we return to the main thread event loop (it jumps to the front of the event queue), before the timeout runs.
2. We return to the event loop, run nsHTMLMediaElement::SelectResource() in the sync section, which sets mDelayingLoadEvent=PR_TRUE, and triggers a load, then returns.
3. The timer runs before the load has a chance to progress, and triggers another load and queues up SelectResource() to run in a sync section. We normally set mDelayingLoadEvent to PR_FALSE when we load metadata, but we didn't get a chance to in this case.
4. SelectResource() runs and asserts that mDelayingLoadEvent==PR_FALSE, but it isn't because the previous load didn't fail or reach metadataloaded, so the assertion fires.
5. When the metadata loading finishes for the second load, we set mDelayingLoadEvent to PR_FALSE.

I think the assertion is wrong, if we start a new resource load before the old resource load has stopped delaying the load event, we should still delay the load event until the new resource load stops delaying the load event. We should remove the assertion.

FWIW in the spec's load algorithm description, it is specified that media element has "a delaying-the-load-event flag, which must begin in the false state", but doesn't explicitly state that the delaying the load event flag is reset when a previous load is aborted.
(Assignee)

Comment 4

6 years ago
Created attachment 527149 [details] [diff] [review]
Patch: Remove assertion
Assignee: nobody → chris
Status: NEW → ASSIGNED
Attachment #527149 - Flags: review?(roc)
Attachment #527149 - Flags: review?(roc) → review+
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/298cbc884044
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.