903340 - Abort: nsDisplayScrollLayer should always be defined 'hasCount' with new 1.1 e.me UI on b2g

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Gregor Wagner [:gwagner]]

MOZ_CRASH: Almost 100% reproducible when starting to type in the new e.me search field.

Program received signal SIGSEGV, Segmentation fault.
0x41f07786 in mozalloc_abort (msg=<value optimized out>) at /Users/Gregor/code/src/memory/mozalloc/mozalloc_abort.cpp:30
30	    MOZ_CRASH();
(gdb) bt
#0  0x41f07786 in mozalloc_abort (msg=<value optimized out>) at /Users/Gregor/code/src/memory/mozalloc/mozalloc_abort.cpp:30
#1  0x417bede8 in Abort (aSeverity=3, aStr=0x42057ab6 "nsDisplayScrollLayer should always be defined", aExpr=<value optimized out>, aFile=<value optimized out>, 
    aLine=3307) at /Users/Gregor/code/src/xpcom/base/nsDebugImpl.cpp:430
#2  NS_DebugBreak (aSeverity=3, aStr=0x42057ab6 "nsDisplayScrollLayer should always be defined", aExpr=<value optimized out>, aFile=<value optimized out>, 
    aLine=3307) at /Users/Gregor/code/src/xpcom/base/nsDebugImpl.cpp:417
#3  0x40a02fec in nsDisplayScrollLayer::GetScrollLayerCount (this=<value optimized out>) at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:3307
#4  0x40a0300c in nsDisplayScrollLayer::RemoveScrollLayerCount (this=0xceb) at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:3317
#5  0x40a03032 in nsDisplayScrollInfoLayer::ShouldFlattenAway (this=0xceb, aBuilder=0xbee19458) at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:3364
#6  0x40a08f18 in nsDisplayList::ComputeVisibilityForSublist (this=0x4712a8d0, aBuilder=0xbee19458, aVisibleRegion=<value optimized out>, 
    aListVisibleBounds=<value optimized out>, aAllowVisibleRegionExpansion=...) at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:1005
#7  0x40a09438 in nsDisplayWrapList::ComputeVisibility (this=0x4712a8a0, aBuilder=0xbee19458, aVisibleRegion=0xbee18bb8, aAllowVisibleRegionExpansion=...)
    at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:2733
#8  0x40a06190 in nsDisplayItem::RecomputeVisibility (this=0x4712a8a0, aBuilder=0xbee19458, aVisibleRegion=0xbee18bb8)
    at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:1505
#9  0x40a06258 in nsDisplayTransform::ComputeVisibility (this=0x4712a870, aBuilder=0xbee19458, aVisibleRegion=<value optimized out>, 
    aAllowVisibleRegionExpansion=<value optimized out>) at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:4041
#10 0x40a06190 in nsDisplayItem::RecomputeVisibility (this=0x4712a870, aBuilder=0xbee19458, aVisibleRegion=0xbee18e00)
    at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:1505
#11 0x409d0668 in mozilla::FrameLayerBuilder::DrawThebesLayer (aLayer=0x43dde0c0, aContext=0x45242550, aRegionToDraw=..., aRegionToInvalidate=..., 
    aCallbackData=0xbee19458) at /Users/Gregor/code/src/layout/base/FrameLayerBuilder.cpp:3229
#12 0x4183131c in mozilla::layers::ClientThebesLayer::PaintBuffer (this=0x43dde0c0, aContext=<value optimized out>, aRegionToDraw=..., aExtendedRegionToDraw=..., 
    aRegionToInvalidate=..., aDidSelfCopy=false) at /Users/Gregor/code/src/gfx/layers/client/ClientThebesLayer.cpp:149
#13 0x418317a2 in mozilla::layers::ClientThebesLayer::PaintThebes (this=0x43dde0c0) at /Users/Gregor/code/src/gfx/layers/client/ClientThebesLayer.cpp:92
#14 0x41831a5c in mozilla::layers::ClientThebesLayer::RenderLayer (this=0x43dde0c0) at /Users/Gregor/code/src/gfx/layers/client/ClientThebesLayer.cpp:123
---Type <return> to continue, or q <return> to quit---
#15 0x4182f86e in ClientContainerLayer::RenderLayer (this=0x44fa6400) at /Users/Gregor/code/src/gfx/layers/client/ClientContainerLayer.h:191
#16 0x418305f0 in mozilla::layers::ClientLayerManager::EndTransactionInternal (this=0x4424b100, 
    aCallback=0x409d03ad <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=<value optimized out>) at /Users/Gregor/code/src/gfx/layers/client/ClientLayerManager.cpp:176
#17 0x41830ee6 in mozilla::layers::ClientLayerManager::EndTransaction (this=0x4424b100, 
    aCallback=0x409d03ad <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0xbee19458, aFlags=mozilla::layers::LayerManager::END_NO_COMPOSITE) at /Users/Gregor/code/src/gfx/layers/client/ClientLayerManager.cpp:199
#18 0x40a06860 in nsDisplayList::PaintForFrame (this=<value optimized out>, aBuilder=0xbee19458, aCtx=<value optimized out>, aForFrame=<value optimized out>, 
    aFlags=13) at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:1190
#19 0x40a06a6c in nsDisplayList::PaintRoot (this=0xbee197f0, aBuilder=0xbee19458, aCtx=0x0, aFlags=13)
    at /Users/Gregor/code/src/layout/base/nsDisplayList.cpp:1051
#20 0x40a20eee in nsLayoutUtils::PaintFrame (aRenderingContext=<value optimized out>, aFrame=0x44aaa298, aDirtyRegion=<value optimized out>, 
    aBackstop=<value optimized out>, aFlags=772) at /Users/Gregor/code/src/layout/base/nsLayoutUtils.cpp:2126
#21 0x40a354bc in PresShell::Paint (this=0x40493630, aViewToPaint=<value optimized out>, aDirtyRegion=<value optimized out>, aFlags=1)
    at /Users/Gregor/code/src/layout/base/nsPresShell.cpp:5605
#22 0x40e4b666 in nsViewManager::ProcessPendingUpdatesForView (this=0x442c8430, aView=0x44a9d330, aFlushDirtyRegion=<value optimized out>)
    at /Users/Gregor/code/src/view/src/nsViewManager.cpp:410
#23 0x40e4b71c in nsViewManager::ProcessPendingUpdates (this=<value optimized out>) at /Users/Gregor/code/src/view/src/nsViewManager.cpp:1031
#24 0x40a401a6 in nsRefreshDriver::Tick (this=0x1620a30, aNowEpoch=2052385877, aNowTime=...) at /Users/Gregor/code/src/layout/base/nsRefreshDriver.cpp:1233
#25 0x40a4071a in mozilla::RefreshDriverTimer::TickDriver (aTimer=<value optimized out>, aClosure=<value optimized out>)
    at /Users/Gregor/code/src/layout/base/nsRefreshDriver.cpp:171
#26 mozilla::RefreshDriverTimer::Tick (aTimer=<value optimized out>, aClosure=<value optimized out>) at /Users/Gregor/code/src/layout/base/nsRefreshDriver.cpp:163
#27 mozilla::RefreshDriverTimer::TimerTick (aTimer=<value optimized out>, aClosure=<value optimized out>)
    at /Users/Gregor/code/src/layout/base/nsRefreshDriver.cpp:188

Comment 1

[Gregor Wagner [:gwagner]]

Also seen during scrolling in the email app.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Gregor Wagner [:gwagner] from comment #1)
> Also seen during scrolling in the email app.

\o/ It's sad but it makes me happy :)

Comment 3

[Gregor Wagner [:gwagner]]

Can we get some help here? This is a constant crash when we use e.me on trunk with a debug build.

Comment 4

[Gregor Wagner [:gwagner]]

Matt can we get some help here? We hit this fairly often with a debug build on the device. Basically just start typing in the everything.me search field on the homescreen or after a few sec if you start scrolling in the email app.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Gregor Wagner [:gwagner] from comment #3)
> Can we get some help here? This is a constant crash when we use e.me on
> trunk with a debug build.

What happens on non-debug build?

Comment 6

[Gregor Wagner [:gwagner]]

I think this is a debug only assertion but in optimize builds I see bug 908381 when I use e.me. Maybe they are related.

Comment 7

[Matt Woodrow (:mattwoodrow)]

Sorry, I have no idea about the scroll layer code.

The comment above the assertion looks like it has some hints on how to diagnose the issue.

Comment 8

[Gregor Wagner [:gwagner]]

Milan, can you find an owner here?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I can't reproduce this on my device running mozilla-central.

Greg, can you reproduce your crash in a debug build, running with
adb shell "export MOZ_DUMP_PAINT_LIST=1; b2g.sh"
and attach the output here?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: logging patch

Trying with this logging patch would also be helpful.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The patch in bug 898444 might just possibly help too. And should be applicable to 1.1 too.

Comment 12

[Gregor Wagner [:gwagner]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> Created attachment 812297 [details] [diff] [review]
> logging patch
> 
> Trying with this logging patch would also be helpful.

This patch is empty :)

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: real logging patch

Comment 14

[Gregor Wagner [:gwagner]]

Logcat for gecko: http://pastebin.mozilla.org/3179500

And adb shell "export MOZ_DUMP_PAINT_LIST=1; b2g.sh":
http://pastebin.mozilla.org/3179503

I also applied the patch from bug 898444 but it still crashed.

Comment 15

[Gregor Wagner [:gwagner]]

Attachment: logging for b2g

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm.

In
+  printf_stderr("nsDisplayScrollInfoLayer::ShouldFlattenAway removing property for %p\n", mScrolledFrame);
can you also dump 'this'?

Looks like the MOZ_DUMP_PAINT_LIST=1 didn't succeed in dumping the data. Did you do it in a debug build?

Comment 17

[Gregor Wagner [:gwagner]]

Attachment: log.txt

now with:
-  printf_stderr("nsDisplayScrollInfoLayer::ShouldFlattenAway removing property for %p\n", mScrolledFrame);
+  printf_stderr("nsDisplayScrollInfoLayer::ShouldFlattenAway removing property for %p, this: %p\n", mScrolledFrame, this);

Comment 18

[Gregor Wagner [:gwagner]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> Hmm.
> 
> In
> +  printf_stderr("nsDisplayScrollInfoLayer::ShouldFlattenAway removing
> property for %p\n", mScrolledFrame);
> can you also dump 'this'?
> 
> Looks like the MOZ_DUMP_PAINT_LIST=1 didn't succeed in dumping the data. Did
> you do it in a debug build?

It is a debug build. Some export MOZ_DUMP_PAINT_LIST=1; b2g.sh doesn't work for me.

Comment 19

[Gregor Wagner [:gwagner]]

Attachment: log2.txt.zip

I had to modify the dump code a little bit so it works on b2g.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

nsDisplayScrollInfoLayer::ShouldFlattenAway gets called twice on the same display item. This shouldn't happen and I can't tell why it does.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: possible fix

Does this fix it?

Comment 22

[Gregor Wagner [:gwagner]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #21)
> Created attachment 816500 [details] [diff] [review]
> possible fix
> 
> Does this fix it?

Yes I don't hit the assertion any more with this patch. I will do some more testing but it looks good!

Comment 23

[Timothy Nikkel (:tnikkel)]

Comment on attachment 816500 [details] [diff] [review]
possible fix

This seems fine but did you ever figure out why ShouldFlatten is getting called twice? That we might be missing a bigger problem nags at me here.

Comment 24

[Gregor Wagner [:gwagner]]

I did some more testing and it looks good. No more assertions!

Comment 25

[Gregor Wagner [:gwagner]]

Can we land this patch?

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes.

Comment 27

[Gregor Wagner [:gwagner]]

Thanks!

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/683b5927e3fe

The remaining question is if we should uplift to 1.2.
Can you comment on the risks?

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I believe this is very low risk.

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/683b5927e3fe

Comment 30

[Ed Morley [:emorley]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9e0f57276693