Closed Bug 1278437 Opened 3 years ago Closed 3 years ago

Crash in mozilla::layers::TileClient::GetBackBuffer

Categories

(Core :: Graphics: Layers, defect, critical)

49 Branch
Unspecified
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: ting, Assigned: nical)

References

Details

(Keywords: crash, regression, topcrash-android-armv7, Whiteboard: [gfx-noted])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-5d65ceef-9230-4d97-9b73-e1cb82160605.
=============================================================

#4 of Nightly Android 20160605030215, 9 crashes from 5 installations. Occurred also on Mac OS X.

The first report was from build 20160603030242.
This crash was caused by the following gfx critical log.

|[0][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=4257.17)
Assignee: nobody → howareyou322
Whiteboard: [gfx-noted]
It certainly doesn't make sense to call:
mInvalidBack = IntRect(0, 0, mBackBuffer->GetSize().width, mBackBuffer->GetSize().height);
before:
if (!mBackBuffer) {
we're guarding too late.

Same for the mBackBufferOnWhite below.

Speaking of which, should we be using mBackBufferOnWhite instead of mBackBuffer here https://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TiledContentClient.cpp#687 (together with the null check beforehand of course.)

Bug 1272600 Part 3 seems to have played a part here.

Let's make sure the fix gets uplifted to 49.
Blocks: 1272600
Keywords: regression
Version: Trunk → 49 Branch
I just hit this on OS X.
Flags: needinfo?(nical.bugzilla)
OS: Android → All
(In reply to Milan Sreckovic [:milan] (June 10 PTO) from comment #2)
> It certainly doesn't make sense to call:
> mInvalidBack = IntRect(0, 0, mBackBuffer->GetSize().width,
> mBackBuffer->GetSize().height);
> before:
> if (!mBackBuffer) {
> we're guarding too late.

Indeed, doesn't make sense. mInvalidBack should be set after the nullcheck.

It is quite sad that this is #4 on android, because this can only happen if we fail to allocate the texture which is already very bad in itself.

> 
> Same for the mBackBufferOnWhite below.
> 
> Speaking of which, should we be using mBackBufferOnWhite instead of
> mBackBuffer here
> https://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/
> TiledContentClient.cpp#687

This doesn't really matter, the tile size is the same for the buffers on white and on black (and all other buffers really.

> 
> Bug 1272600 Part 3 seems to have played a part here.

Yes, definitely.

Peter you are assigned to the bug, let me know if you want me to steal it from you. It's a simple fix either way.
Flags: needinfo?(nical.bugzilla)
Nical, I just assigned this bug to you.
Assignee: howareyou322 → nobody
Assignee: nobody → nical.bugzilla
Attachment #8762686 - Flags: review?(jnicol) → review+
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eec36edf2899
Null-check after allocating a tile's texture, before accessing it. r=jnicol
https://hg.mozilla.org/mozilla-central/rev/eec36edf2899
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Comment on attachment 8762686 [details] [diff] [review]
Access mBackBuffer after we have null-checked it.

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Crashes when running low and memory.
[Describe test coverage new/current, TreeHerder]: none.
[Risks and why]: none, trivial patch, nul-check.
[String/UUID change made/needed]: none.
Attachment #8762686 - Flags: approval-mozilla-aurora?
Comment on attachment 8762686 [details] [diff] [review]
Access mBackBuffer after we have null-checked it.

Crash fix, aurora is affected, let's take it.
Attachment #8762686 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
The numbers are dropping but making a note that this was the #8 topcrash in Aurora on Android with 2.04% of the volume.
You need to log in before you can comment on or make changes to this bug.