888458 - [10.8] crash in -[ChildView nativeDirtyRegionWithBoundingRect:]

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Scoobidiver (away)]

It first showed up in 24.0a1/201360622. The regression range might be (startup crash):
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7ba8c86f1a56&tochange=cea75ce9a559
It's likely a regression from bug 875335 that landed in the previous regression range.

Signature 	-[ChildView nativeDirtyRegionWithBoundingRect:] More Reports Search
UUID 	16afc9ec-583c-4c20-a4d4-638ff2130628
Date Processed	2013-06-28 18:53:15.384931
Uptime	6
Last Crash	35 seconds before submission
Install Age 	6 since version was first installed.
Install Time 	2013-06-28 18:53:05
Product 	Firefox
Version 	25.0a1
Build ID 	20130628031149
Release Channel 	nightly
OS 	Mac OS X
OS Version 	10.8.4 12E55
Build Architecture 	x86
Build Architecture Info 	GenuineIntel family 6 model 23 stepping 6 | None
Crash Reason 	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address 	0x7fffffff
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x94c8GL Layers! GL Context? GL Context+ GL Layers+ 
Processor Notes 	sp-processor05_phx1_mozilla_com_4696:2012; exploitability tool: ERROR: unable to analyze dump

Crashing Thread
Frame 	Module 	Signature 	Source
0 	XUL 	-[ChildView nativeDirtyRegionWithBoundingRect:] 	widget/cocoa/nsChildView.mm
1 	XUL 	-[ChildView viewWillDraw] 	widget/cocoa/nsChildView.mm
2 		@0x1700fd20 	
3 	AppKit 	AppKit@0x19d0b0 	
4 	CoreFoundation 	CoreFoundation@0x933b2 	
5 	CoreFoundation 	CoreFoundation@0x92f36 	
6 	CoreFoundation 	CoreFoundation@0x92e45 	
7 	AppKit 	AppKit@0x19cdfb 	
8 	AppKit 	AppKit@0x19d0b0 	
9 	CoreFoundation 	CoreFoundation@0x933b2 	
10 	CoreFoundation 	CoreFoundation@0x92f36 	
11 	CoreFoundation 	CoreFoundation@0x92e45 	
12 	AppKit 	AppKit@0x19cdfb 	
13 	AppKit 	AppKit@0x19c5a4 	
14 	AppKit 	AppKit@0x1670f8 	
15 	AppKit 	AppKit@0x166d3d 	
16 	AppKit 	AppKit@0x21f619 	
17 	AppKit 	AppKit@0x21f0b6 	
18 	AppKit 	AppKit@0x21ebcd 	
19 	AppKit 	AppKit@0x21e734 	
20 	AppKit 	AppKit@0x217338 	
21 	XUL 	nsCocoaWindow::Show(bool) 	widget/cocoa/nsCocoaWindow.mm

More reports at:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=-[ChildView+nativeDirtyRegionWithBoundingRect%3A]

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Note that all the crashes are in 32-bit mode.

And yes, I crash on startup running today's mozilla-central nightly with a clean profile on OS X 10.8.3.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Interestingly, I don't crash with the same STR on OS X 10.7.5.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

I find the following regression range:

firefox-2013-06-20-03-10-10-mozilla-central
firefox-2013-06-21-03-10-51-mozilla-central

Which translates into:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8ea92aeab783&tochange=7ba8c86f1a56

And yes, this does include the patches for bug 875335.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I've been trying to narrow down the regression range I gave in comment #3.  But none of the custom builds I do reproduce the crash.

I suppose it's a matter of figuring out exactly what .mozconfig file to use, but that's turning out to be a lot more difficult than I expected.

Comment 5

[Markus Stange [:mstange]]

Oh. I think replacing [NSView focusView] by self should fix this. [NSView focusView] is not guaranteed to be non-null during viewWillDraw, and since count is not initialized, calling getRectsBeingDrawn on nil will not update its valuee.

Comment 6

[Markus Stange [:mstange]]

Attachment: fix

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 769274 [details] [diff] [review]
fix

Looks reasonable.

You *could* also fix the crashes by doing this:

-   NSInteger count;
+   NSInteger count = 0;

Note also that -[NSRect getRectsBeingDrawn:...] probably doesn't work outside a call to -[NSView drawRect:].

But it seems to make better sense to get such a list from 'self' rather than from [NSView focusView] (which might return some other NSView altogether).

Comment 8

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d731a339374

Comment 9

[Markus Stange [:mstange]]

(In reply to Steven Michaud from comment #7)
> You *could* also fix the crashes by doing this:
> 
> -   NSInteger count;
> +   NSInteger count = 0;

That would fix the crash, yes. But it wouldn't actually give us the rects we're interested in.
I've actually noticed a bug on 10.9 where we didn't redraw the window buttons in drawintitlebar windows on hover. For some reason I could only reproduce the bug in a nightly and not in a self-compiled build, but I hope this patch will fix it.

> Note also that -[NSRect getRectsBeingDrawn:...] probably doesn't work
> outside a call to -[NSView drawRect:].

I assume you mean NSView instead of NSRect. As far as I know, getRectsBeingDraw will work inside drawRect and inside viewWillDraw. The docs for viewWillDraw say:

> A subclass’s implementation of viewWillDraw can use the existing NSView
> getRectsBeingDrawn:count: method to obtain a list of rectangles that bound the
> affected area, enabling it to restrict its efforts to that area.

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6d731a339374

Comment 11

[Markus Stange [:mstange]]

(In reply to Markus Stange [:mstange] from comment #9)
> I've actually noticed a bug on 10.9 where we didn't redraw the window
> buttons in drawintitlebar windows on hover. For some reason I could only
> reproduce the bug in a nightly and not in a self-compiled build, but I hope
> this patch will fix it.

I've confirmed that this patch fixes that bug, too.

Comment 12

[Scoobidiver (away)]

It's #1 top crasher in 24.0a2 on Mac OS X.

Comment 13

[Markus Stange [:mstange]]

Comment on attachment 769274 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 875335
User impact if declined: topcrash
Testing completed (on m-c, etc.): 1 day on mozilla-central
Risk to taking this patch (and alternatives if risky): extremely low
String or IDL/UUID changes made by this patch: none

Comment 14

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #9)

>> Note also that -[NSRect getRectsBeingDrawn:...] probably doesn't
>> work outside a call to -[NSView drawRect:].
>
> I assume you mean NSView instead of NSRect.

Yes :-)

> As far as I know, getRectsBeingDraw will work inside drawRect and
> inside viewWillDraw. The docs for viewWillDraw say: ...

You're right.  I stand corrected.

Comment 16

[Markus Stange [:mstange]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/299263db2764

Comment 17

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 24 beta 2 on Mac OS X 10.8 in 32-bit mode (Firefox does not crash on startup or after startup).

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Firefox/24.0
Build ID: 20130812173056

Comment 18

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 25.0a2 on Mac OS X 10.8 in 32-bit mode - Firefox does not start on startup or after startup.

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0
Build ID: 20130912004004