Closed Bug 1243639 Opened 4 years ago Closed 4 years ago

crash in on Mali-400 MP


(Firefox for Android :: Toolbar, defect, critical)

Not set



Firefox 47
Tracking Status
firefox44 --- wontfix
firefox45 --- wontfix
firefox46 --- fixed
firefox47 --- fixed


(Reporter: kbrosnan, Assigned: droeh)


(Keywords: crash)

Crash Data


(1 file)

This bug was filed from the Socorro interface and is 
report bp-c440aa28-43d2-4000-98ab-e60dc2160127.

Mfg.    Model           And. API    CPU ABI      #      
samsung SM-T560         19 (REL)  armeabi-v7a  10507  63.291%
samsung SM-T561         19 (REL)  armeabi-v7a   3807  22.932%
samsung SM-J110H        19 (REL)  armeabi-v7a   1042   6.277%
HUAWEI HUAWEI Y336-U02  19 (REL)  armeabi-v7a    726   4.373%
samsung SM-T561M        19 (REL)  armeabi-v7a    281   1.693%
Sotaro, anything you can do to help?
Flags: needinfo?(sotaro.ikeda.g)
From the log, crash happened within It seems not clear why the crash happened. Last gecko's function call was EGLImageTextureSource::BindTexture(). It caused the crash. It actually does not allocate GL resource. It just bind EGLImage to GL texture.
One possibility is that EGLImage was not valid anymore when EGLImageTextureSource::BindTexture() was called.
EGLImageTextureSource is created by EGLImageTextureHost. EGLImageTextureHost's peers are EGLImageTextureData or SharedSurface_EGLImage.
Assignee: nobody → sotaro.ikeda.g
Flags: needinfo?(sotaro.ikeda.g)
SharedSurface_EGLImage is created for SkiaGL and WebGL.
EGLImageTextureData is created for EGLImageImage. EGLImageImage is created by nsPluginInstanceOwner.
snorp: Do you know if plugin is still enabled and used on Fennec? If it is used, how can test it?
Flags: needinfo?(snorp)
(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> snorp: Do you know if plugin is still enabled and used on Fennec? If it is
> used, how can test it?

Yeah. We support Flash if it's installed. You can get it from here:
Flags: needinfo?(snorp)
Hmm. The EGLImageImage code in nsPluginInstanceOwner should only be used on Honeycomb, which we no longer support. It looks like people have installed the wrong plugin on a ICS+ device so it's not trying to use the SurfaceTexture API. I'm not sure if there is a good way we can detect this, but it might be enough to just return an error when a plugin tries to request the Honeycomb API. I can take this and write that patch.
Assignee: sotaro.ikeda.g → snorp
Actually, Dylan can you do this? It should mostly just be removing some code.

Make this[0] request from the plugin return an error and remove ANPOpenGL.cpp[1]. Then you can remove nsNPAPIPluginInstance::LockContentTexture[2] and the associated machinery.

Assignee: snorp → droeh
Attached patch Proposed patchSplinter Review
Attachment #8725255 - Flags: review?(snorp)
Comment on attachment 8725255 [details] [diff] [review]
Proposed patch

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

lgtm, what kind of testing did you do? You should:

1) Make sure the ICS version of Flash works on a ICS+ device
2) Make sure the Gingerbread version of Flash does not crash on a ICS+ device.

I fear that 2) may not be true. In that case we may need to figure out something else.
Attachment #8725255 - Flags: review?(snorp) → review+
Yup, I tested both ICS and pre-ICS flash on my Nexus 6 and got the desired behavior.
Keywords: checkin-needed
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Is this fix verified? If so, is this upliftable to 46?
There are no reported crashes on 47.0a2 or 48.0a1 so it is hard to say if this is resolved on Nightly. Landing on beta should be conclusive as there are a few hundred crashes on that release.
Comment on attachment 8725255 [details] [diff] [review]
Proposed patch

Approval Request Comment
[Feature/regressing bug #]: 721741
[User impact if declined]: Potential crashes if the wrong version of flash is installed on ICS+ devices.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Should be low-risk, the patch removes code that is only used for the Gingerbread version of flash.
[String/UUID change made/needed]:
Flags: needinfo?(droeh)
Attachment #8725255 - Flags: approval-mozilla-beta?
Attachment #8725255 - Flags: approval-mozilla-aurora?
Comment on attachment 8725255 [details] [diff] [review]
Proposed patch

This landed on 47 (which is now aurora) so no need to request uplift to that channel.
Attachment #8725255 - Flags: approval-mozilla-aurora?
Comment on attachment 8725255 [details] [diff] [review]
Proposed patch

Crash fix, hooray! This should land tonight or tomorrow but would not affect Fennec until the beta 6 build next Monday.
Attachment #8725255 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
It would be good to verify this fix some time at the end of next week, if this makes it into beta 6.
Flags: qe-verify+
(In reply to Wes Kocher (:KWierso) from comment #21)

This remains a topcrash in Fennec 47 with 21,165 crashes reported over the last week. Should this get reopened or a new bug filed?

Is qe-verify+ flag still valid?

Thank you!
Flags: needinfo?(lhenry)
Hi miralobontiu, for this bug, it probably doesn't need verification at this point.  But, since it is based on crash reports, you can click through on the crash signature field, which will show if there are any current reports with the same crash signature:   There are a few, but you might notice they are from versions 47 beta 6 and earlier.    Because there aren't any crashes with current versions, we know this bug is fixed, and it was probably from the patch in this bug.
Flags: qe-verify-
Flags: qe-verify+
Flags: needinfo?(lhenry)
You need to log in before you can comment on or make changes to this bug.