Closed Bug 877085 Opened 7 years ago Closed 7 years ago

crash in nsGfxScrollFrameInner::GetNondisappearingScrollbarWidth @ nsNativeThemeCocoa::GetMinimumWidgetSize

Categories

(Core :: Layout, defect, critical)

24 Branch
x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla24
Tracking Status
firefox23 --- verified
firefox24 --- verified

People

(Reporter: scoobidiver, Assigned: spohl)

References

Details

(Keywords: crash, regression, Whiteboard: [lion-scrollbars+])

Crash Data

Attachments

(1 file, 1 obsolete file)

It first showed up in 24.0a1/20130528. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a39263b0c896&tochange=c190422547ed
It's likely a regression from bug 869314.

Signature 	nsNativeThemeCocoa::GetMinimumWidgetSize(nsRenderingContext*, nsIFrame*, unsigned char, nsIntSize*, bool*) More Reports Search
UUID	bc24da13-f131-465a-9a24-c8f7a2130529
Date Processed	2013-05-29 02:05:03
Uptime	27706
Last Crash	4.0 days before submission
Install Age	7.7 hours since version was first installed.
Install Time	2013-05-28 17:34:24
Product	Firefox
Version	24.0a1
Build ID	20130528030942
Release Channel	nightly
OS	Mac OS X
OS Version	10.8.2 12C3103
Build Architecture	amd64
Build Architecture Info	family 6 model 58 stepping 9
Crash Reason	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address	0x0
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x 166GL Layers? GL Context? GL Context+ GL Layers+ 
Processor Notes 	sp-processor06_phx1_mozilla_com_16597:2012; exploitability tool: ERROR: unable to analyze dump
EMCheckCompatibility	True
Adapter Vendor ID	0x8086
Adapter Device ID	0x 166

Frame 	Module 	Signature 	Source
0 	XUL 	nsNativeThemeCocoa::GetMinimumWidgetSize 	widget/cocoa/nsNativeThemeCocoa.mm:1783
1 	libobjc.A.dylib 	lookUpMethod 	
2 	XUL 	nsGfxScrollFrameInner::GetNondisappearingScrollbarWidth 	layout/generic/nsGfxScrollFrame.cpp:954
3 	XUL 	nsNativeTheme::IsWidgetStyled 	widget/xpwidgets/nsNativeTheme.cpp:330
4 	XUL 	_ZThn112_N17nsHTMLScrollFrame32GetNondisappearingScrollbarWidthEP13nsPresContext 	layout/generic/nsGfxScrollFrame.h:493
5 	XUL 	nsComboboxControlFrame::GetIntrinsicWidth 	layout/forms/nsComboboxControlFrame.cpp:742
6 	XUL 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2727

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsNativeThemeCocoa%3A%3AGetMinimumWidgetSize%28nsRenderingContext*%2C+nsIFrame*%2C+unsigned+char%2C+nsIntSize*%2C+bool*%29
Assignee: nobody → spohl.mozilla.bugs
Whiteboard: [lion-scrollbars+]
Attached patch Patch (obsolete) — Splinter Review
GetParentScrollbarFrame crashes when aFrame is NULL. We don't need to have a valid frame to retrieve the scrollbar width for non-disappearing scrollbars. This patch avoids any dependency on aFrame.
Attachment #755413 - Flags: review?(smichaud)
> We don't need to have a valid frame to retrieve the scrollbar width
> for non-disappearing scrollbars.

Why is this?  

If it's because non-disappearing scrollbars are currently never
"small", is that a good enough reason?
Status: NEW → ASSIGNED
(In reply to Steven Michaud from comment #2)
> > We don't need to have a valid frame to retrieve the scrollbar width
> > for non-disappearing scrollbars.
> 
> Why is this?  
> 
> If it's because non-disappearing scrollbars are currently never
> "small", is that a good enough reason?

We're interested in finding out what the width of the traditional scrollbars was in order to display a dropmarker in non-native styled comboboxes. As far as I can tell, these comboboxes always had the regular (not small) scrollbar width.
I'd feel more comfortable if, for non-disappearing scrollbars, you defaulted to kThemeMetricScrollBarWidth if aFrame is NULL.  You should also add an XXX comment saying you're not sure it's necessary to check scrollbarFrame->StyleDisplay()->mAppearance at all in this case, and why.
Attached patch PatchSplinter Review
Attachment #755413 - Attachment is obsolete: true
Attachment #755413 - Flags: review?(smichaud)
Attachment #755480 - Flags: review?(smichaud)
Comment on attachment 755480 [details] [diff] [review]
Patch

Looks good to me.

Another question is why aFrame is sometimes NULL in this code, where it (apparently) never was before.  *That* may be the real bug.  But I wouldn't pursue it unless these crashes are reproducible.
Attachment #755480 - Flags: review?(smichaud) → review+
https://hg.mozilla.org/mozilla-central/rev/7220b0615ba6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Scoobidiver, could you verify that this crash signature disappears in the coming days? Once confirmed I'm going to uplift bug 869314 and this bug here to aurora simultaneously. Thanks!
Flags: needinfo?(scoobidiver)
Flags: needinfo?(scoobidiver)
Flags: needinfo?(scoobidiver)
If I'm reading the crash stats correctly I think this crash has disappeared.
(In reply to Stephen Pohl [:spohl] from comment #9)
> Scoobidiver, could you verify that this crash signature disappears in the
> coming days?
There have been no crashes since 24.0a1/20130602. Crashes in 24.0a1/20130531 and 20130601 (post-patch) are in Nightly UX.
Status: RESOLVED → VERIFIED
Flags: needinfo?(scoobidiver)
I'll land this on aurora with the combined patch in bug 869314, once the aurora approval has gone through.
(In reply to Robert Strong [:rstrong] (do not email) from comment #13)
> Landed in a combined patch in bug 869314
on mozilla-aurora
(In reply to Scoobidiver from comment #11)
> There have been no crashes since 24.0a1/20130602.

Could you please check if it's a similar story for Firefox 23?
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #15)
> Could you please check if it's a similar story for Firefox 23?
There have never been crashes in 23.0a2 because the patch of bug 869314 landed in Aurora with this bug's fix.
You need to log in before you can comment on or make changes to this bug.