Closed Bug 1128179 Opened 6 years ago Closed 6 years ago

Firefox crash in mozilla::SourceBufferDecoder::WasTrimmed()

Categories

(Core :: Audio/Video, defect)

36 Branch
x86
Windows 7
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla38
Tracking Status
firefox35 --- unaffected
firefox36 + verified
firefox37 + verified
firefox38 --- verified

People

(Reporter: marcia, Assigned: mattwoodrow)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-10052775-fb3f-4e72-bc68-ede142150130.
=============================================================

Seen while looking at the Aurora explosive report, but this appears to be happening on Nightly as well as the most recent beta 5 which has yet to be pushed out (ID:20150129200438)

Crashes across all branches: http://bit.ly/1yYn0BQ

There are crashes dating back to build ID 2015012503, and crashes as recent as 2015013003 on nightly

Comments:
* Firefox crashs when i open videos on youtube using the html5 player 
* Youtube crashes 100% of the time. Normal HTML5 videos works correctly, for example http://www.w3.org/2010/05/video/mediaevents.html 
* just started firefox and visited youtube, when clicked a video to play the browser crashed. 
* when youtube is open on a tab, and open a new tab = crash 


rame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::SourceBufferDecoder::WasTrimmed() 	dom/media/mediasource/SourceBufferDecoder.h
1 	xul.dll 	mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64) 	dom/media/mediasource/TrackBuffer.cpp
2 	xul.dll 	mozilla::dom::SourceBuffer::AppendData(mozilla::LargeDataBuffer*, double) 	dom/media/mediasource/SourceBuffer.cpp
3 	xul.dll 	mozilla::dom::AppendDataRunnable::Run() 	dom/media/mediasource/SourceBuffer.cpp
4 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
5 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
6 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
7 	xul.dll 	nsThreadManager::GetMainThread(nsIThread**) 	xpcom/threads/nsThreadManager.cpp
8 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
9 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
10 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
11 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
12 	xul.dll 	nsAString_internal::Replace(unsigned int, unsigned int, wchar_t const*, unsigned int, mozilla::fallible_t const&) 	xpcom/string/nsTSubstring.cpp
13 	xul.dll 	nsLocalFile::AppendInternal(nsString const&, bool) 	xpcom/io/nsLocalFileWin.cpp
14 	xul.dll 	nsLocalFile::Append(nsAString_internal const&) 	xpcom/io/nsLocalFileWin.cpp
15 	kernel32.dll 	GetProcessPriorityBoost 	
16 	kernel32.dll 	GetCurrentProcess 	
17 	xul.dll 	std::basic_string<char, std::char_traits<char>, std::allocator<char> >::_Tidy(bool, unsigned int) 	c:/tools/vs2013/vc/include/xstring:2282
18 	xul.dll 	`anonymous namespace'::HistogramGet(char const*, char const*, unsigned int, unsigned int, unsigned int, unsigned int, bool, base::Histogram**) 	toolkit/components/telemetry/Telemetry.cpp
19 	xul.dll 	`anonymous namespace'::HistogramGet(char const*, char const*, unsigned int, unsigned int, unsigned int, unsigned int, bool, base::Histogram**) 	toolkit/components/telemetry/Telemetry.cpp
20 	xul.dll 	mozilla::TimeStamp::ProcessCreation(bool&) 	xpcom/ds/TimeStamp.cpp
21 		@0x2e9a7f
This one will be fixed with bug 1125776.

But I can make a quicker fix if need be.
Attached patch Crash fixSplinter Review
Are you planning on uplifting the other bug to beta? If not (or not by monday), then we should take this.
Attachment #8557574 - Flags: review?(jyavenard)
Comment on attachment 8557574 [details] [diff] [review]
Crash fix

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

I had this in my queue..

The fundamental issue is that it shows that you can get there with mCurrentDecoder being nullptr ; it should never have been the case as we must have received an init segment by now and a decoder would have been created.

As far as pushing my changes, I'm waiting for reviews...
Attachment #8557574 - Flags: review?(jyavenard) → review+
This can happen if initialization of the decoder failed, which is the case for bug 1128013.
(In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> This can happen if initialization of the decoder failed, which is the case
> for bug 1128013.

Then this is not the proper fix... it will prevent the crash, but things will crap out after that.

     if (!decoders.NewDecoder(aTimestampOffset)) {
       return false;
     }

should be:
     if (!decoders.NewDecoder(aTimestampOffset)) {
       mParser = ContainerParser::CreateForMIMEType(mType);
       return false;
     }

so we never attempt to present a buffered range we can't play... and further call to appendBuffer will abort early (currently , as soon as we've seen an init segment, we proceed forward)
(In reply to Jean-Yves Avenard [:jya] from comment #5)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> > This can happen if initialization of the decoder failed, which is the case
> > for bug 1128013.
> 
> Then this is not the proper fix... it will prevent the crash, but things
> will crap out after that.
> 
>      if (!decoders.NewDecoder(aTimestampOffset)) {
>        return false;
>      }
> 
> should be:
>      if (!decoders.NewDecoder(aTimestampOffset)) {
>        mParser = ContainerParser::CreateForMIMEType(mType);
>        return false;
>      }
> 
> so we never attempt to present a buffered range we can't play... and further
> call to appendBuffer will abort early (currently , as soon as we've seen an
> init segment, we proceed forward)

NewDecoder doesn't fail, ReadMetadata does (which calls DiscardDecoder).
Duplicate of this bug: 1128295
Crash Signature: [@ mozilla::SourceBufferDecoder::WasTrimmed()] → [@ mozilla::SourceBufferDecoder::WasTrimmed()] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64)]
This report was pretty explosive in Beta 5, so anything we can do to try to improve things in the next Beta would be great.
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Assignee: jyavenard → matt.woodrow
https://hg.mozilla.org/mozilla-central/rev/6037ec1faa35
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
[Tracking Requested - why for this release]:
Nominating for tracking 36 as this is ~3.2% of b5 crashes.
Matt, can we have an uplift request? Thanks
Flags: needinfo?(matt.woodrow)
Comment on attachment 8557574 [details] [diff] [review]
Crash fix

Approval Request Comment
[Feature/regressing bug #]: Bug 1096089
[User impact if declined]: Crashes on some videos
[Describe test coverage new/current, TreeHerder]: Few days on m-c
[Risks and why]: Very low risk, just a null check
[String/UUID change made/needed]: None
Flags: needinfo?(matt.woodrow)
Attachment #8557574 - Flags: approval-mozilla-beta?
Attachment #8557574 - Flags: approval-mozilla-aurora?
Comment on attachment 8557574 [details] [diff] [review]
Crash fix

Trivial fix for crashes. Beta+ Aurora+
Attachment #8557574 - Flags: approval-mozilla-beta?
Attachment #8557574 - Flags: approval-mozilla-beta+
Attachment #8557574 - Flags: approval-mozilla-aurora?
Attachment #8557574 - Flags: approval-mozilla-aurora+
Crash Signature: [@ mozilla::SourceBufferDecoder::WasTrimmed()] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64)] → [@ mozilla::SourceBufferDecoder::WasTrimmed()] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64)] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, long long)] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuf…
Socorro shows no more crashes after the fix landed.
You need to log in before you can comment on or make changes to this bug.