Closed Bug 1190950 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Graphics: Layers, defect)

Unspecified
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox41 --- wontfix
firefox42 + fixed

People

(Reporter: gcp, Assigned: bas.schouten)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is report bp-349d05de-f9fb-499a-bdb6-6ef062150802. =============================================================
[Tracking Requested - why for this release]:
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?
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.
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.
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: 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.
Tracking for 42, see Comment 2. Regression.
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+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
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)
Since a few days now, the behaviour where tabs turn black shortly before they crash has gone away. However, all tabs still crash regularly.
(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)
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?
(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
Flags: needinfo?(bas)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: