108940 - Crash on every page with position:fixed - Trunk [@ nsView::SetVisibility]

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect)

Description

[Matthew Mastracci]

Any page using position:fixed is crashing.  :(

One example, w3c fixed menu: www.w3.org/Style/Examples/007/menus.html

Talkback IDs:
TB37708287E
TB37708179Z
TB37708147W
TB37708011M
TB37707982X

Comment 1

[Boris Zbarsky [:bzbarsky]]

Over to viewmanager:

#0  nsViewManager::GetMouseEventGrabber (this=0x0) at nsViewManager.h:386
#1  0x420b40dd in nsView::SetVisibility (this=0x88e1070, 
    aVisibility=nsViewVisibility_kHide) at nsView.cpp:674
#2  0x420c847d in nsZPlaceholderView::nsZPlaceholderView (this=0x88e1070, 
    aParent=0x87a9858) at nsViewManager.h:102
#3  0x420bfb86 in nsViewManager::InsertZPlaceholder (this=0x8136348,
aParent=0x87a9858, 
    aChild=0x88c65b0, aZIndex=1) at nsViewManager.cpp:2021
#4  0x41d87780 in nsHTMLContainerFrame::CreateViewForFrame (aPresContext=0x87abbc8, 
    aFrame=0x88c01a8, aStyleContext=0x88c0114, aContentParentFrame=0x88bfb0c,
aForce=1)
    at nsHTMLContainerFrame.cpp:619
#5  0x41e37ad3 in nsCSSFrameConstructor::ConstructFrameByDisplayType
(this=0x88ed178, 
    aPresShell=0x88eb170, aPresContext=0x87abbc8, aState=@0xbfffe604, 
    aDisplay=0x88c0148, aContent=0x87aa920, aParentFrame=0x88bfb0c, 
    aStyleContext=0x88c0114, aFrameItems=@0xbfffe6cc) at
nsCSSFrameConstructor.cpp:6110


(gdb) frame 0
#0  nsViewManager::GetMouseEventGrabber (this=0x0) at nsViewManager.h:386
386       nsView* GetMouseEventGrabber() const { return mMouseGrabber; }
(gdb) p mMouseGrabber
Cannot access memory at address 0x34

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: full stack

Comment 3

[Kevin McCluskey (gone)]

Attachment: Patch to check for null viewmanger when setting visibility on the view

Comment 4

[Kevin McCluskey (gone)]

The problem is the nsZPlaceholderView contructor calls nsView::SetVisibility to
initialize it's visibility. SetVisibility was accessing it's viewmanager member
variable which hadn't been set yet.

Comment 5

[Marc Attinasi]

Comment on attachment 56940 [details] [diff] [review]
Patch to check for null viewmanger when setting visibility on the view

sr=attinasi - but wouldn't it be better to juggle the order of calls around so that the viewManager *is* set before the SetVisibility call is made, and then ASSERT that mViewManager is not null?

Comment 6

[Kevin McCluskey (gone)]

The call to SetVisibility is done in the constructor to set it's initial value.
The viewmanager member variable is not set until the nsView::init method is
called. I could explicitly set the visibility member of the view instead of
calling the SetVisibility method, but I thought it would be better just protect
the SetVisibility against having a null viewmanager.

Comment 7

[Boris Zbarsky [:bzbarsky]]

With that patch the crash goes away but is replaced with continuous assertions:

###!!! ASSERTION: mViewManager is null!: 'mViewManager', file nsView.cpp, line 632
###!!! Break: at file nsView.cpp, line 632

see eg http://web.mit.edu/bzbarsky/www/

Also, could you not just test mViewManager instead of |nsnull != mViewManager| ?

Comment 8

[Boris Zbarsky [:bzbarsky]]

bug 108934 is another crash in which mViewManager is null.  Combined with the
assertions, that makes me think there is a deeper problem here somewhere...

Comment 9

[Marc Attinasi]

Should we try backing out Robert's changes from last night (locally)?
http://bonsai.mozilla.org/showcheckins.cgi?person=roc%2B%25cs.cmu.edu&treeid=SeaMonkey

Comment 10

[Kevin McCluskey (gone)]

I backed out Robert's changes from last night locally and this crash and the
crash in 108934 go away:

cvs update -j3.80 -j3.79 mozilla/view/src/nsViewManager.h
cvs update -j3.210 -j3.209 mozilla/view/src/nsViewManager.cpp
cvs update -j3.57 -j3.56 mozilla/view/src/nsView.h
cvs update -j3.142 -j3.141 mozilla/view/src/nsView.cpp
cvs update -j3.57 -j3.56 mozilla/view/src/nsScrollingView.h
cvs update -j3.144 -j3.143 mozilla/view/src/nsScrollingView.cpp
cvs update -j3.9 -j3.8 mozilla/view/src/nsScrollPortView.h
cvs update -j3.22 -j3.21 mozilla/view/src/nsScrollPortView.cpp
cvs update -j3.56 -j3.55 mozilla/view/public/nsIViewManager.h
cvs update -j3.60 -j3.59 mozilla/view/public/nsIView.h

It may be caused by the part of the patch that removes the implementations for
the nsIView methods that ZPlaceholderFrame's used to have and uses the nsView's
implementations instead.

Comment 11

[Kevin McCluskey (gone)]

Reassigning to Robert to give him a chance to look at this.

Comment 12

[Kevin McCluskey (gone)]

*** Bug 108934 has been marked as a duplicate of this bug. ***

Comment 13

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

Urk.

Comment 14

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

Thanks for backing this out. I am an idiot.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Um... it's not backed out.  Kevin just _tried_ backing it out in his tree.  This
bug is still present as of right now on the trunk.

Comment 16

[Boris Zbarsky [:bzbarsky]]

*** Bug 109038 has been marked as a duplicate of this bug. ***

Comment 17

[Adam Hauner]

Are bugs 109055 and 109057 dupes of this?

Comment 18

[Boris Zbarsky [:bzbarsky]]

*** Bug 109057 has been marked as a duplicate of this bug. ***

Comment 19

[R.K.Aa.]

dup of bug 108651?

Comment 20

[Kevin McCluskey (gone)]

*** Bug 109068 has been marked as a duplicate of this bug. ***

Comment 21

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

Oh. Wow.

Alright, let me fix it pronto.

Comment 22

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

Attachment: Fix up nsZPlaceholderView creation and destruction

This is it ... no crash, no assertion failures. Ready for review.

Comment 23

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

That patch fixes all testcases in all dups so far.

Comment 24

[Marc Attinasi]

Comment on attachment 57083 [details] [diff] [review]
Fix up nsZPlaceholderView creation and destruction

sr=attinasi

Comment 25

[Kevin McCluskey (gone)]

Comment on attachment 57083 [details] [diff] [review]
Fix up nsZPlaceholderView creation and destruction

r=[e-mail]

Comment 26

[Jay Patel [:jay]]

Adding Trunk [@ nsView::SetVisibility] for tracking, this is currently a
topcrasher with MozillaTrunk builds.

Comment 27

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.6

Comment 28

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

Fix checked in.

Comment 29

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

*** Bug 109097 has been marked as a duplicate of this bug. ***

Comment 30

[Katsuhiko Momoi]

*** Bug 109206 has been marked as a duplicate of this bug. ***

Comment 31

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 109270 has been marked as a duplicate of this bug. ***