1351370 - Crash in mozilla::java::CodecProxy::Flush

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect, P1)

Description

[Marcia Knous [:marcia]]

[Tracking Requested - why for this release]: Currently #6 overall crash in Beta 5. We should keep on eye on it since it appears to be a new signature.

This bug was filed from the Socorro interface and is 
report bp-a32c7bb5-1088-4f8b-8c07-40aba2170328.
=============================================================

Seen while looking at Firefox beta crash data - crashes started in B1 and have continued to B5.

Many of the devices seem to be running Kit Kat: http://bit.ly/2o7BSA3

Comment 1

[Marcia Knous [:marcia]]

It was suggested at the Channel meeting that I need Sebastian to take a look at this.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

This is a gecko/h264 decoding crash. This may go away with the disabling OOP video decoding. Bug 1350209

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

This is actually caused by the new OOP stuff.

-> jolin

Comment 4

[John Lin [:jhlin][:jolin]]

JW, from the call stack it looks like Flush() is called on a decoder that either wasn't initialized or has been shut down. Do you by any chance has some idea how this could happen?

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

This is a regression from bug 1316506.

Since MediaFormatReader::DecoderFactory::Data::mDecoder is never used for decoding, Flush() is not required before calling Shutdown().

I will submit a patch for that.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1351370_fix_53_beta.patch

Fix a regression caused by bug 1316506 which hangs the Android decoder if Flush() is called without initialization.

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8852306 [details] [diff] [review]
1351370_fix_53_beta.patch

Approval Request Comment
[Feature/Bug causing the regression]:1316506
[User impact if declined]:crash
[Is this code covered by automated tests?]:no
[Has the fix been verified in Nightly?]:no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:no
[Why is the change risky/not risky?]:one line change is simple.
[String changes made/needed]:none

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

If OOP decoding is disabled on beta why do we need to patch this on beta? We should land it on trunk and 54 if that is the next target for OOP decoding.

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

No. The crash is not caused by OOP which just makes it more likely to happen.

Comment 12

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Comment on attachment 8852306 [details] [diff] [review]
1351370_fix_53_beta.patch

Crash fix, let's take this for 53 based on comment 10.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f7dcb07ca418

Comment 14

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

looks like this bug should be RESOLVED FIXED already?

Comment 15

[Marcia Knous [:marcia]]

(In reply to Wesley Huang [:wesley_huang] (EPM) (NI me) from comment #14)
> looks like this bug should be RESOLVED FIXED already?

Looks like it just needs to be resolved fixed. Marking as such now.