Closed
Bug 1247380
Opened 9 years ago
Closed 9 years ago
45.0b4 crash spike in gfxContext::PushGroupAndCopyBackground at address 0x0
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: kairo, Assigned: bas.schouten)
References
Details
(Keywords: crash, crashreportid, Whiteboard: gfx-noted)
Crash Data
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
jrmuizel
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details |
[Tracking Requested - why for this release]:
This bug was filed from the Socorro interface and is
report bp-84a86876-d189-4320-af90-091ce2160210.
=============================================================
Stack Trace:
0 xul.dll gfxContext::PushGroupAndCopyBackground(gfxContentType, float, mozilla::gfx::SourceSurface*, mozilla::gfx::Matrix const&) gfx/thebes/gfxContext.cpp
1 xul.dll mozilla::layers::BasicLayerManager::PushGroupForLayer(gfxContext*, mozilla::layers::Layer*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&) gfx/layers/basic/BasicLayerManager.cpp
All those stacks are only those two frames, and have an address of 0x0.
This crash signature spiked in 45.0b4 and is 6.8% of total crashes (and 4.8% of non-e10s crashes) - while it was only 1.1% of non-e10s crashes in 45.0b3.
Comment 1•9 years ago
|
||
Bas, is this related to your push group / pop group stuff?
Flags: needinfo?(bas)
Whiteboard: gfx-noted,crash
Comment 2•9 years ago
|
||
The spike isn't limited to b4, b3(/2/1) shows the same spike.
e10s hit with tipple crashes compared to non-e10s for this.
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Mason Chang [:mchang] from comment #1)
> Bas, is this related to your push group / pop group stuff?
No, I don't think it is, I'll have a patch up here soon though.
Flags: needinfo?(bas)
Assignee | ||
Comment 4•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/34495/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34495/
Attachment #8718301 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 5•9 years ago
|
||
This fix is a little speculative as I don't know what causes this and I can't reproduce it locally, by guess would be something like a device loss. But even if it's something else I'm fine with losing subpixel AA instead of crashing. This implements that. On trunk this bug is superceeded by native pushlayer. I'll prepare a patch for beta and request uplift once this has been r+'ed.
Updated•9 years ago
|
Assignee: nobody → bas
Status: NEW → ASSIGNED
Keywords: crashreportid
Whiteboard: gfx-noted,crash → gfx-noted
Tracking for 46 since this is a top crash. It doesn't appear to affect 46 or 47.
Comment 7•9 years ago
|
||
Comment on attachment 8718301 [details]
MozReview Request: Bug 1247380: Only copy the background if we can succesfully get a snapshot. r=jrmuizel
As dvander points out on IRC Snapshot() doesn't seem to have the ability to return null.
Attachment #8718301 -
Flags: review?(jmuizelaar) → review-
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8718301 [details]
MozReview Request: Bug 1247380: Only copy the background if we can succesfully get a snapshot. r=jrmuizel
Cairo does. We've dealt with a bunch of cairo surface state errors in the past (one of them was when using certain fonts for some reason iirc). This is presumably why this crash signature spiked, we've made a number of places inside the Cairo Moz2D code return nullptr more aggressively when there's state issues recently.
Attachment #8718301 -
Flags: review- → review?(jmuizelaar)
Assignee | ||
Comment 9•9 years ago
|
||
To make people happier with some additional information, I believe this was a regression from http://hg.mozilla.org/mozilla-central/rev/6c49bb716593. Before that change CreateSimilarDT would return a nullptr when the surface was in an error state, this would cause us to crash in gfxContext::PushNewDT.
Comment 10•9 years ago
|
||
Comment on attachment 8718301 [details]
MozReview Request: Bug 1247380: Only copy the background if we can succesfully get a snapshot. r=jrmuizel
https://reviewboard.mozilla.org/r/34495/#review31275
Attachment #8718301 -
Flags: review?(jmuizelaar) → review+
Comment 11•9 years ago
|
||
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8718301 [details]
MozReview Request: Bug 1247380: Only copy the background if we can succesfully get a snapshot. r=jrmuizel
Approval Request Comment
[Feature/regressing bug #]: Technically not a regression, changed stack from a fix for the PushNewDT crash.
[User impact if declined]: More crashes.
[Describe test coverage new/current, TreeHerder]: None, since this code is no longer used on 46 or higher
[Risks and why]: Very low, just a nullcheck
[String/UUID change made/needed]: None
Attachment #8718301 -
Flags: approval-mozilla-beta?
Comment 13•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 14•9 years ago
|
||
control-no-addons 10.94%, experiment-no-addons 33.77%. (low samples, experiment just started.)
Hints to me that it is possibly an advert.
Comment 15•9 years ago
|
||
Comment on attachment 8718301 [details]
MozReview Request: Bug 1247380: Only copy the background if we can succesfully get a snapshot. r=jrmuizel
Fix a top crash, taking it.
Should be in 45 beta 6.
Attachment #8718301 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 16•9 years ago
|
||
has problems with beta uplift:
Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r b2b9363f3edc
grafting 328530:b2b9363f3edc "Bug 1247380: Only copy the background if we can succesfully get a snapshot. r=jrmuizel"
merging gfx/thebes/gfxContext.cpp
warning: conflicts while merging gfx/thebes/gfxContext.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Flags: needinfo?(bas)
Reporter | ||
Comment 17•9 years ago
|
||
This is also the #1 Top Crash Score in 44.0.2 release.
Assignee | ||
Comment 18•9 years ago
|
||
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #17)
> This is also the #1 Top Crash Score in 44.0.2 release.
Hrm, it seems the cause of this got uplifted in bug 1107792. In theory this just 'moves' a crash to another signature rather than causing a new one though. I'll land this patch on beta now. If it works it could be worth uplifting if there is going to be another dot release. But it should be noted this is not a regression.
Flags: needinfo?(bas)
Assignee | ||
Comment 19•9 years ago
|
||
Comment 20•9 years ago
|
||
(In reply to Bas Schouten (:bas.schouten) from comment #19)
> https://hg.mozilla.org/releases/mozilla-beta/rev/01b9ef7f3dce
setting flags
Updated•9 years ago
|
Blocks: e10s-crashes
Updated•9 years ago
|
tracking-e10s:
--- → ?
You need to log in
before you can comment on or make changes to this bug.
Description
•