874391 - Crash [@ nsNativeTheme::IsDarkBackground(nsIFrame*)] when opening DOM Inspector/Thunderbird

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[:Gijs (he/him)]

This happened on my own build, and the OS X crash dialog says:


Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000

With a stack starting:

0   XUL                           	0x00000001021d98e4 nsNativeTheme::IsDarkBackground(nsIFrame*) + 20 (nsNativeTheme.cpp:685)
1   XUL                           	0x00000001021a8325 nsNativeThemeCocoa::DrawWidgetBackground(nsRenderingContext*, nsIFrame*, unsigned char, nsRect const&, nsRect const&) + 9029 (nsNativeThemeCocoa.mm:2310)
2   XUL                           	0x00000001016319b2 nsCSSRendering::PaintBackgroundWithSC(nsPresContext*, nsRenderingContext&, nsIFrame*, nsRect const&, nsRect const&, nsStyleContext*, nsStyleBorder const&, unsigned int, nsRect*, int) + 386 (nsCSSRendering.cpp:2490)
3   XUL                           	0x00000001016317b8 nsCSSRendering::PaintBackground(nsPresContext*, nsRenderingContext&, nsIFrame*, nsRect const&, nsRect const&, unsigned int, nsRect*, int) + 280 (nsCSSRendering.cpp:1572)
4   XUL                           	0x000000010164af04 nsDisplayBackgroundImage::Paint(nsDisplayListBuilder*, nsRenderingContext*) + 228 (nsDisplayList.cpp:2115)
5   XUL                           	0x0000000101611c98 mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*) + 2408 (FrameLayerBuilder.cpp:3314)
6   XUL                           	0x0000000102723663 mozilla::layers::ThebesLayerOGL::RenderLayer(int, nsIntPoint const&) + 339 (BaseRect.h:44)
7   XUL                           	0x000000010271971c mozilla::layers::ContainerLayerOGL::RenderLayer(int, nsIntPoint const&) + 1564 (LayerManagerOGL.h:461)
8   XUL                           	0x000000010271e85f mozilla::layers::LayerManagerOGL::Render() + 1231 (LayerManagerOGL.cpp:837)
9   XUL                           	0x000000010271e101 mozilla::layers::LayerManagerOGL::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) + 545 (LayerManagerOGL.cpp:566)
10  XUL                           	0x00000001016479d6 nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) const + 1462 (nsDisplayList.h:306)
11  XUL                           	0x00000001016473fc nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) const + 172 (GeckoProfilerImpl.h:233)
12  XUL                           	0x0000000101667c44 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) + 2900 (nsLayoutUtils.cpp:2093)
13  XUL                           	0x0000000101682a4b PresShell::Paint(nsView*, nsRegion const&, unsigned int) + 907 (nsPresShell.cpp:5593)
14  XUL                           	0x0000000101b9f733 nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) + 627 (nsRegion.h:75)
15  XUL                           	0x000000010168b7a3 nsRefreshDriver::Tick(long long, mozilla::TimeStamp) + 3107 (nsISupportsImpl.h:260)
16  XUL                           	0x000000010168c4a8 mozilla::RefreshDriverTimer::Tick() + 248 (nsTArray.h:363)
17  XUL                           	0x00000001026839e2 nsTimerImpl::Fire() + 402 (nsTimerImpl.cpp:562)
18  XUL                           	0x0000000102683add nsTimerEvent::Run() + 29 (nsAutoPtr.h:879)

followed by some other less relevant frames... Based on the first frame, blaming bug 865806.

This is 100% reproducible for me just by starting nightly and opening DOM Inspector using the shortcut (Cmd+shift+I).

Comment 1

[:Gijs (he/him)]

Attachment: Patch

This is the dumb nullcheck way. I'm not sure this is the Right fix, but it sure fixes the crashes I'm seeing...

Comment 2

[Ludovic Hirlimann [:Usul]]

This is crashing Thunderbird in startup for me right now.

Comment 3

[Mark Banner (:standard8)]

A very similar issue is affecting Thunderbird as well:

bp-70519f39-00ae-4d55-ae9e-118e32130521

Comment 4

[:Gijs (he/him)]

Pushed directly to central to get this fixed ASAP: https://hg.mozilla.org/mozilla-central/rev/c96b50b62e75

Comment 5

[Stephen A Pohl [:spohl] (OOO until 9/9)]

Comment on attachment 752133 [details] [diff] [review]
Patch

Review of attachment 752133 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/xpwidgets/nsNativeTheme.cpp
@@ +686,2 @@
>    while (!scrollFrame && aFrame) {
> +    scrollFrame = aFrame->GetScrollTargetFrame();

Would there be any point in keeping track of the last scroll target frame that we found, or are we only interested in it when the root frame can get a scroll target frame?

Comment 6

[Stephen A Pohl [:spohl] (OOO until 9/9)]

Not sure if need-info is necessary here, but figured it couldn't hurt.

Comment 7

[(no longer active)]

Thanks for the quick fix, Gijs!

(In reply to Stephen Pohl [:spohl] from comment #5)
> Comment on attachment 752133 [details] [diff] [review]
> Patch
> 
> Review of attachment 752133 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/xpwidgets/nsNativeTheme.cpp
> @@ +686,2 @@
> >    while (!scrollFrame && aFrame) {
> > +    scrollFrame = aFrame->GetScrollTargetFrame();
> 
> Would there be any point in keeping track of the last scroll target frame
> that we found, or are we only interested in it when the root frame can get a
> scroll target frame?

We bail out of the loop when the first scroll target frame is found.  We're not really keeping track of the last one.

Comment 8

[Stephen A Pohl [:spohl] (OOO until 9/9)]

Duh, of course. :-) Thanks.

Comment 9

[(not currently active) Ted Mielczarek]

I triggered new nightly builds on Gijs' revision because this is crashing me constantly.

Comment 10

[David Teller [:Yoric] - still alive but not very active]

Apparently, I have the same crashes without attempting to open DOMi: https://crash-stats.mozilla.com/report/index/bp-28dbcc71-cbad-4551-b8db-a18212130521

Comment 11

[Stephen A Pohl [:spohl] (OOO until 9/9)]

Comment on attachment 752133 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 865806
User impact if declined: Frequent crashes.
Testing completed (on m-c, etc.): Has been on m-c for a few days and was confirmed to fix the bug.
Risk to taking this patch (and alternatives if risky): low (simple null-check)
String or IDL/UUID changes made by this patch: none

Comment 12

[:Gijs (he/him)]

Comment on attachment 752133 [details] [diff] [review]
Patch

Cancelling approval request in favour of the request on bug 865806 for a patch combining the original fix with this crashfix.

Comment 13

[Paul Silaghi, QA [:pauly]]

No crashes in the crashstats in the last 4 weeks.