crash in mozilla::layers::BufferTextureClient::UpdateFromSurface(mozilla::gfx::DataSourceSurface*)

RESOLVED FIXED in Firefox 42

Status

()

Core
Graphics: Layers
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gcp, Assigned: bas)

Tracking

({crash, regression})

Trunk
mozilla42
Unspecified
Windows NT
crash, regression
Points:
---

Firefox Tracking Flags

(firefox41 wontfix, firefox42+ fixed)

Details

(crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
This bug was filed from the Socorro interface and is 
report bp-349d05de-f9fb-499a-bdb6-6ef062150802.
=============================================================
[Tracking Requested - why for this release]:
tracking-firefox42: --- → ?
Keywords: regression
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #1)
> [Tracking Requested - why for this release]:

Sorry, prematurely submit this change.

I'm asking to track since this appears to be a regression in Firefox 42.0a1. All the crashes reported so far are isolated to this version. It is currently #25 on Nightly accounting for 0.52% of crashes and appears to have first appeared on July 29, 2015.

@gcp, do you have steps to reproduce this crash?
(Reporter)

Comment 3

2 years ago
I don't, sorry. It was reported by a friend (who didn't have specific STR either) and I noticed it had many reports but was still unfiled.
This is new code added by Bas.

Both serializer.GetAsSurface() and GetAsDataSurface() (the source of aSurface) are fallible, so we should check for those before dereferencing.

Comment 5

2 years ago
I'm the one with the crash this bug was filed for and it just re-occured, no clear STR.

Three pinned tabs with gmail, gmail and TheOldReader, reading some Reddit with the Enhancement Suite.
Paged down in a thread and the left side of the screen turned white.
Shortly after the tab contents crashed to black and all other tabs were also filled with black.
Finally a prompt to restart the crashed tabs appeared.

Comment 6

2 years ago
DebugView has a message »[8016] D3D11: Removing Device.» whenever it blacks out and I seem to have gotten my configuration into a state where it has a high chance of crashing whenever looking at particular tabs.
(Assignee)

Comment 7

2 years ago
Created attachment 8643535 [details] [diff] [review]
Make mapping and surface access more robust for UpdateFromSurface
Assignee: nobody → bas
Status: NEW → ASSIGNED
Attachment #8643535 - Flags: review?(matt.woodrow)
Comment on attachment 8643535 [details] [diff] [review]
Make mapping and surface access more robust for UpdateFromSurface

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

::: gfx/layers/client/TextureClient.cpp
@@ +831,3 @@
>  
>  void
>  BufferTextureClient::UpdateFromSurface(gfx::DataSourceSurface* aSurface)

I believe aSurface is null in this case, so we should check for that too.

This is coming from here: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ImageContainer.cpp#650

If we just had a TDR, then GetDataSurface() could fail (on a d2d surface), without being a critical error, so we should probably try to fail gracefully in this case.

Comment 9

2 years ago
Tracking for 42, see Comment 2. Regression.
tracking-firefox42: ? → +
(Assignee)

Comment 10

2 years ago
Created attachment 8643928 [details] [diff] [review]
Make mapping and surface access more robust for UpdateFromSurface v2
Attachment #8643535 - Attachment is obsolete: true
Attachment #8643535 - Flags: review?(matt.woodrow)
Attachment #8643928 - Flags: review?(matt.woodrow)
Attachment #8643928 - Flags: review?(matt.woodrow) → review+

Comment 11

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f4d2d0ad511
https://hg.mozilla.org/mozilla-central/rev/1f4d2d0ad511
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Can we nominate this for beta approval (41 is being uplifted there today) so we can take the "real" fix from bug 1176363 on 41?
Flags: needinfo?(bas)

Comment 14

2 years ago
Since a few days now, the behaviour where tabs turn black shortly before they crash has gone away.
However, all tabs still crash regularly.
(Assignee)

Comment 15

2 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #13)
> Can we nominate this for beta approval (41 is being uplifted there today) so
> we can take the "real" fix from bug 1176363 on 41?

We should probably have both.
Flags: needinfo?(bas)
(Assignee)

Comment 16

2 years ago
Comment on attachment 8643928 [details] [diff] [review]
Make mapping and surface access more robust for UpdateFromSurface v2

Approval Request Comment
[Feature/regressing bug #]: Bug 1176363
[User impact if declined]: Crashes
[Describe test coverage new/current, TreeHerder]: Extended nightly coverage
[Risks and why]: Fairly low, mainly adds a bunch of checks and moves some things around
[String/UUID change made/needed]: None
Attachment #8643928 - Flags: approval-mozilla-aurora?
Comment on attachment 8643928 [details] [diff] [review]
Make mapping and surface access more robust for UpdateFromSurface v2

Bas, I guess you meant beta? and not aurora?. This patch already landed in 42.
Attachment #8643928 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
status-firefox41: --- → affected
(Assignee)

Comment 18

2 years ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #17)
> Comment on attachment 8643928 [details] [diff] [review]
> Make mapping and surface access more robust for UpdateFromSurface v2
> 
> Bas, I guess you meant beta? and not aurora?. This patch already landed in
> 42.

I did, silly me.
Comment on attachment 8643928 [details] [diff] [review]
Make mapping and surface access more robust for UpdateFromSurface v2

This crash doesn't show up on FF42 since 08-6 when the fix was checked into m-c. Given that the crash has gone away and the patch was in Nightly for about two weeks, it seems safe to uplift to Beta.
Attachment #8643928 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I'm hitting a lot of conflicts trying to merge this to beta, and I don't feel confident enough in myself to try to resolve them.
Can we get a branch-specific patch for this, Bas?
Flags: needinfo?(bas)
This depends on Bas' patch from bug 1176363, which also needs rebasing for 41. This doesn't need to land until if/when that does.
Blocks: 1176363
(In reply to Wes Kocher (:KWierso) from comment #20)
> I'm hitting a lot of conflicts trying to merge this to beta, and I don't
> feel confident enough in myself to try to resolve them.
> Can we get a branch-specific patch for this, Bas?

bas is there a update for this seems there are also questions in bug 1176363
status-firefox41: affected → wontfix
Flags: needinfo?(bas)
You need to log in before you can comment on or make changes to this bug.