235405 - Mozilla crashes when trying to hide an iframe when iframe is in an element with fixed position [@ GetFrameFromLine ]

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Arnoud Berendsen]

User agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040204

Please open the testcase at the attached URL.

Description of the problem:
Each of the buttons there opens a div with an IFRAME in it. The left div is
positioned absolute, the right one has position:fixed. When the button to close
the fixed div is pressed, Mozilla crashes. (Also happens in Firebird).

Steps to reproduce:
1) Click the two buttons that say "Open it"
2) Click the left "Close it" button. Nothing special happens.
3) Click the right "Close it" button". Mozilla crashes.

Expected result:
Right div behaves like left div, and does not cause crash.

Reproducable: Always

Comment 1

[Olivier Cahagne]

confirming crash using FF 20040224 Win2k.

Comment 2

[Mats Palmgren (inactive)]

WFM, 2003-02-23-08 trunk Linux.  In a debug build I get:

###!!! ASSERTION: Focus events should not be getting thru when this is null!:
'shell', file nsEventStateManager.cpp, line 744

Comment 3

[Adam Hauner]

TB30649805W using 2004021913/1.7a/W2K

Comment 4

[Hermann Schwab]

TB30650312Z using 2004021913/1.7a/Win98SE

Comment 5

[Andreas Kunz]

This is what .NET 2003 gives me as stack.
Please have a look at it and remove the stackwanted keyword if this stack is ok.

>	gklayout.dll!nsContainerFrame::GetFrameForPointUsing(nsIPresContext *
aPresContext=0x03089878, const nsPoint & aPoint={...}, nsIAtom *
aList=0x00000000, nsFramePaintLayer aWhichLayer=eFramePaintLayer_Overlay, int
aConsiderSelf=0, nsIFrame * * aFrame=0x03debc44)  Line 328 + 0x19	C++
 	gklayout.dll!nsBlockFrame::GetFrameForPoint(nsIPresContext *
aPresContext=0x03089878, const nsPoint & aPoint={...}, nsFramePaintLayer
aWhichLayer=eFramePaintLayer_Overlay, nsIFrame * * aFrame=0x03debc44)  Line
5645 + 0x1a	C++
 	gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x02fd7110, nsGUIEvent *
aEvent=0x0012c7d0, nsEventStatus * aEventStatus=0x0012c610, int aForceHandle=1,
int & aHandled=1)  Line 5924 + 0x20	C++
 	gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x02fd7110, nsGUIEvent
* aEvent=0x0012c7d0, int aCaptured=0)  Line 2225	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012c7d0,
nsEventStatus * aStatus=0x0012c73c)  Line 2010 + 0x14	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012c7d0)  Line 79	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012c7d0,
nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1064 + 0xa	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012c7d0) 
Line 1085	C++
 	gkwidget.dll!nsWindow::DispatchFocus(unsigned int aEventType=105, int
isMozWindowTakingFocus=1)  Line 5401 + 0xf	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=7, unsigned int
wParam=592164, long lParam=0, long * aRetValue=0x0012cc44)  Line 4150 + 0x17	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x00090926, unsigned int
msg=7, unsigned int wParam=592164, long lParam=0)  Line 1346 + 0x1b	C++
 	user32.dll!77d13a50() 	
 	user32.dll!77d13b1f() 	
 	user32.dll!77d144f5() 	
 	user32.dll!77d14525() 	
 	ntdll.dll!77f65da3() 	
 	user32.dll!77d1a17d() 	
 	gkwidget.dll!nsWindow::Destroy()  Line 1643 + 0x10	C++
 	gklayout.dll!nsViewManager::RemoveChild(nsIView * aChild=)  Line 2527	C++

Comment 6

[Asa Dotzler [:asa]]

Attachment: stack trace of crash from winXP

Comment 7

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

Is there a way we can prevent events from being processed in this nested event loop?

Comment 8

[Adam Hauner]

Should be bug 237961 dupe of this (one of incidents is TB2034X)?

Comment 9

[Dan M]

David: what nested event loop? I can't find one in the stack. It seems a fairly
straightforward crash accessing deleted frames:

GetFrameFromLine(const nsRect & aLineArea, const nsPoint & aTmp,
  nsLineList_iterator & aLine, nsIPresContext * 0x031fa878,
  nsFramePaintLayer eFramePaintLayer_Overlay,
  nsIFrame * * 0x032344fc) line 5728 + 25 bytes
<--
  if (aLineArea.Contains(aTmp)) {
    nsIFrame* kid = aLine->mFirstChild;
    PRInt32 n = aLine->GetChildCount();
    while (--n >= 0) {
      nsIFrame *hit;
      nsresult rv = kid->GetFrameForPoint(aPresContext, aTmp, aWhichLayer, &hit);
      *** crash is here. kid has been deleted.
-->
nsBlockFrame::GetFrameForPointUsing(nsIPresContext * 0x031fa878,
  const nsPoint & aPoint, nsIAtom * 0x00000000,
  nsFramePaintLayer eFramePaintLayer_Overlay, int 0, nsIFrame * * 0x032344fc)
  line 5804 + 29 bytes
<--
    for (line_iterator line = mLines.begin();
         line != line_end;
         ++line) {
    *** same mLines being deleted (below)
      nsRect lineArea = line->GetCombinedArea();
      ...
      GetFrameFromLine(lineArea, tmp, line, aPresContext, aWhichLayer, aFrame);
      ...
    }
-->
nsBlockFrame::GetFrameForPoint(nsBlockFrame * const 0x036e0a48,
  nsIPresContext * 0x031fa878, const nsPoint & aPoint,
  nsFramePaintLayer eFramePaintLayer_Overlay, nsIFrame * * 0x032344fc)
  line 5838 + 26 bytes
PresShell::HandleEvent(PresShell * const 0x032344b4, nsIView * 0x036ece08,
  nsGUIEvent * 0x0012c9c8, nsEventStatus * 0x0012c800, int 1, int & 1)
  line 5851 + 32 bytes
nsViewManager::HandleEvent(nsView * 0x036ece08, nsGUIEvent * 0x0012c9c8, int 0)
  line 2235
...OS dispatches focus to Window's container window
...destroy Window
...destroy View
...destroy Frame
nsBlockFrame::Destroy(nsBlockFrame * const 0x036e0a48,
  nsIPresContext * 0x031fa878) line 300 + 16 bytes
<--
    nsLineBox::DeleteLineList(aPresContext, mLines);
    *** while deleting mLines, we'll access it (above)
-->
nsAreaFrame::Destroy(nsAreaFrame * const 0x036e0a48,
  nsIPresContext * 0x031fa878) line 156
...start destroying frames
nsCSSFrameConstructor::ContentRemoved(nsIPresContext * 0x031fa878,
  nsIContent * 0x031e9868, nsIContent * 0x03318d90, int 7, int 0)
  line 9431 + 67 bytes
nsCSSFrameConstructor::RecreateFramesForContent(nsIPresContext * 0x031fa878,
  nsIContent * 0x03318d90) line 11264 + 31 bytes
...notifications go flying
CSS2PropertiesTearoff::SetDisplay(CSS2PropertiesTearoff * const 0x02f6ff4c,
  const nsAString & {...}) line 339 + 51 bytes
...set <div> style.display="none"

Comment 10

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

Look at the stacks in comment 5 and comment 6 -- frame destruction is leading to
view destruction which leads to widget destruction which calls
nsWindow::WindowProc which dispatches a focus event which accesses the frame
hierarchy that's in the middle of being deleted.

Comment 11

[Dan M]

I came to the same conclusion; see the annotated stack in comment 9. I'm still
not seeing a nested event loop. Oh! Well there's an event being processed. But
there's no loop. I think it would be bad to try to suppress the OS's attempt to
reestablish focus as the focused window is destroyed.

Seems to me the thing to do is armour up nsBlockFrame so it doesn't step through
mLines as mLines is being destroyed, no matter what. That's why I cc:ed a bunch
of Frame guys. A second possibility would perhaps be to explicitly set focus on
the outermost window before the chain of destruction begins.

Comment 12

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

The thing is, Windows seems to be the only platform that does this type of thing
-- and this is definitely not the first crash of this type that I've seen.  It
would be nice if we could just avoid having to worry about this stuff.

Comment 13

[Dan M]

Windows isn't being very obnoxious. It's just focusing the first container of a
focused window immediately after that focused window is destroyed. In this case,
the window given focus is a perfectly valid window. It's just that its idjit
focus handler tackles the message by stepping off into its list of deleted
frames, looking for a nonexistent subframe to take focus.

At the widget level, nothing is amiss. In this case by luck or design the window
getting focus is the innermost one that won't be destroyed. Without outside
help, it can't know that anything is the matter. Therefore any processing delay
would likely happen at a layer of abstraction within the widget. Once again I
find myself at the same conclusion, that the Frame needs to know about the state
of its subframes, and just keep focus to itself in this case.

But a general mechanism to force Windows to, say, post destruction
activate/focus messages, rather than sending them, so Mozilla would wait to
process them until its next pass through the event loop, presumably once this
chain of destruction has had a chance to run its complete course? Not that I
know of. Maybe someone else does. I fear the sort of subtle focus problems such
a thing might introduce. (Heh. Or it could fix a bunch. But it'd be different,
and something angry and smelly has *got* to fall out of that tree when you shake
it.)

If you could get the knowledge to a window that its contents are in a state of
flux, it might work to catch and repost activate and focus messages until it
settles down. But I think it'd be safer to instead code Mozilla to handle inline
notifications like this.

Comment 14

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

Easiest quick fix is to have nsBlockFrame::Destroy start by calling
ClearLineCursor().

Comment 15

[Boris Zbarsky [:bzbarsky]]

Robert, that sounds reasonable to me.  Could someone test that that fixes the crash?

Comment 16

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

I can't reproduce the crash, so I can't verify the fix. But I'll attach the
patch in case someone else wants to try.

Comment 17

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

Attachment: fix?

I guess this might not be enough. It might be possible for event handling to
reinstate the line cursor during nsBlockFrame::Destroy().

Comment 18

[Dan M]

No help; the patch does not stop the crash. If it's any help, SetupLineCursor is
never called after clicking the button to undisplay the div, before the crash.

Comment 19

[Asa Dotzler [:asa]]

I don't see this showing up in talkback (other than the few people explicitely
testing this bug).  Is this likely to be a high-profile crasher? 

Comment 20

[Asa Dotzler [:asa]]

is bug 242620 the same as this? The stack traces look very close.

Comment 21

[Frank Wein [:mcsmurf]]

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

Comment 22

[Martijn Wargers (dead)]

Is this still an issue? The testcase seems to work for me (no crash) in 2006-01-14 trunk build on windows.

Comment 23

[José Jeria]

Also WFM with latest trunk build on XP

Comment 24

[Martijn Wargers (dead)]

Ok, marking WFM then.

Comment 25

[Arnoud Berendsen]

This still crashes Firefox 1.5 final. I will test a Trunk build if/when I can

Comment 26

[Arnoud Berendsen]

WFM on the Trunk/WinXP as well. 

Comment 27

[Martijn Wargers (dead)]

So this got fixed between 2005-08-22 and 2005-08-23:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-08-22+06%3A00%3A00&maxdate=2005-08-23+08%3A00%3A00&cvsroot=%2Fcvsroot
So maybe fixed by bug 296036?