Closed Bug 1247949 Opened 8 years ago Closed 8 years ago

crash in mozilla::ContainerState::SetupScrollingMetadata rising in 45 with Todoist addon

Categories

(Core :: Layout, defect)

45 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox45 --- affected
firefox46 --- fixed
firefox47 --- fixed
firefox-esr45 - fixed

People

(Reporter: philipp, Assigned: mstange)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-1b3dcdac-976e-4d0e-954f-6558a2160211.
=============================================================
0 	xul.dll 	mozilla::ContainerState::SetupScrollingMetadata(mozilla::NewLayerEntry*) 	layout/base/FrameLayerBuilder.cpp
1 	xul.dll 	mozilla::ContainerState::PostprocessRetainedLayers(mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits>*) 	layout/base/FrameLayerBuilder.cpp
2 	xul.dll 	mozilla::ContainerState::Finish(unsigned int*, mozilla::LayerManagerData*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, nsDisplayList*, bool&) 	layout/base/FrameLayerBuilder.cpp
3 	xul.dll 	mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) 	layout/base/FrameLayerBuilder.cpp
4 	xul.dll 	nsDisplayOpacity::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) 	layout/base/nsDisplayList.cpp
5 	xul.dll 	mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*) 	layout/base/FrameLayerBuilder.cpp
6 	xul.dll 	mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) 	layout/base/FrameLayerBuilder.cpp
7 	xul.dll 	nsIFrame::GetSkipSides(nsHTMLReflowState const*) 	layout/generic/nsFrame.cpp

this signature seems to go up in 45 builds and correlation data & user comments would suggest that it is related to the todoist addon from https://addons.mozilla.org/firefox/addon/todoist/ (~20k users)

the timing of this occurring in 45.0a1 builds would suggest that this may have been triggered by the landing of the patch from bug 1222880.
Looks like we get a null AGR here instead of hitting the container agr. We should probably just check for a null agr in the terminate condition of the for loop. Agree Markus?
Flags: needinfo?(mstange)
For 45, sure. We're already doing it on 46 (checking for a null scroll clip).
Flags: needinfo?(mstange)
It's too late for 45 most likely.
Is this fixed in 46 then?

can we update the status-firefox* flags if so?
yes, just by looking at crash stats this looks fixed in 46.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Hi, I don't know if it is the right place to put it, but my company is using firefox esr and we still have the same problem (although with a custom add-on).
Has this bug been fixed on esr (now 45.2.0) as well as the normal firefox version?


crash report https://crash-stats.mozilla.com/report/index/3c240327-1c91-4f83-a974-f898a2160706

More : the custom add-on are just for the company.. But it uses the panel API https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/panel to open links.. this is when crashes happen (but not on all links).

Let me know if you have information, or if you need more data.
[Tracking Requested - why for this release]:
Crash, regression, affecting Firefox ESR 45.  (per comment 3, comment 7, & per "firefox45: affected" status).

Sounds like we could backport a tiny/trivial fix to ESR45, based on comment 1. tn / mstange, what do you think?
Flags: needinfo?(tnikkel)
(ESR45 is still supported for a good while; the next ESR is version 52.)

(In reply to Markus Stange [:mstange] from comment #2)
> We're already doing it on 46 (checking for a null scroll clip).

Was this Firefox-46-era change bug 1147673, or another bug?  (Since this bug is marked FIXED with no patches landed, we should ideally update it to be dependent on whatever bug we expect was the fix.)
Flags: needinfo?(mstange)
Yes, it was bug 1147673.
Depends on: 1147673
Flags: needinfo?(mstange)
Attached patch patch for esr45Splinter Review
Assignee: nobody → mstange
Flags: needinfo?(tnikkel)
Attachment #8774068 - Flags: review?(tnikkel)
Attachment #8774068 - Flags: review?(dholbert)
Attachment #8774068 - Flags: review?(tnikkel) → review+
Attachment #8774068 - Flags: review?(dholbert)
Comment on attachment 8774068 [details] [diff] [review]
patch for esr45

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: crashes in some situations, apparently more likely with Todoist addon
Fix Landed on Version: 46, but it was a different fix
Risk to taking this patch (and alternatives if risky): virtually zero. just a null check
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8774068 - Flags: approval-mozilla-esr45?
Comment on attachment 8774068 [details] [diff] [review]
patch for esr45

In ESR, we only take stability patches for the 2 first releases of the cycle. Here, we are past that and we only take sec-high & sec-critical fixes.
However, as it is really low risk and we already shipped several releases with the fix, it impacts at least an ESR 45 user, let's take it.
Attachment #8774068 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
Crash volume for signature 'mozilla::ContainerState::SetupScrollingMetadata':
 - nightly (version 50): 0 crashes from 2016-06-06.
 - aurora  (version 49): 7 crashes from 2016-06-07.
 - beta    (version 48): 12 crashes from 2016-06-06.
 - release (version 47): 18 crashes from 2016-05-31.
 - esr     (version 45): 592 crashes from 2016-04-07.

Crash volume on the last weeks:
            W. N-1  W. N-2  W. N-3  W. N-4  W. N-5  W. N-6  W. N-7
 - nightly       0       0       0       0       0       0       0
 - aurora        0       0       0       7       0       0       0
 - beta          1       6       1       1       2       1       0
 - release       4       6       3       2       2       1       0
 - esr          46      75      63      52      39      49      81

Affected platforms: Windows, Mac OS X, Linux
Probably a different issue.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: