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)
Tracking
()
RESOLVED
FIXED
mozilla42
People
(Reporter: gcp, Assigned: bas.schouten)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mattwoodrow
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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•9 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.
Comment 4•9 years ago
|
||
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•9 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•9 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•9 years ago
|
||
Comment 8•9 years ago
|
||
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.
Assignee | ||
Comment 10•9 years ago
|
||
Attachment #8643535 -
Attachment is obsolete: true
Attachment #8643535 -
Flags: review?(matt.woodrow)
Attachment #8643928 -
Flags: review?(matt.woodrow)
Updated•9 years ago
|
Attachment #8643928 -
Flags: review?(matt.woodrow) → review+
Comment 11•9 years ago
|
||
Comment 12•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment 13•9 years ago
|
||
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•9 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•9 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•9 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 17•9 years ago
|
||
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?
Updated•9 years ago
|
status-firefox41:
--- → affected
Assignee | ||
Comment 18•9 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)
Comment 21•9 years ago
|
||
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
Comment 22•9 years ago
|
||
(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
Updated•9 years ago
|
Flags: needinfo?(bas)
You need to log in
before you can comment on or make changes to this bug.
Description
•