Closed Bug 377960 Opened 17 years ago Closed 16 years ago

Crash [@ nsGenericHTMLElement::GetFormControlFrameFor][@ DoDeletingFrameSubtree], using select and iframe

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Platform:
x86
Windows XP
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: martijn.martijn, Assigned: dholbert)

References

Details

(Keywords: crash, testcase, Whiteboard: [sg:critical?][duplicate of 399852?])

Crash Data

Attachments

(3 files)

testcase
555 bytes, text/html
Details
reduced testcase for mac crash
271 bytes, text/html
Details
stacktrace for reduce testcase crash
12.43 KB, text/plain
Details
Attached file testcase — 
This testcas looks rather similar to bug 377938, but I get different stacktraces, so I'm filing this as a different for now.

The testcase only sometimes crashes. 
To reproduce, both the select and the iframe have to be visible after onload.
Then, click inside the iframe.
If it doesn't crash, press the 'Go' button and try again.

Talkback ID: TB31322388Q
DoDeletingFrameSubtree  [mozilla/layout/base/nscssframeconstructor.cpp, line 9274]
DeletingFrameSubtree  [mozilla/layout/base/nscssframeconstructor.cpp, line 9355]
nsCSSFrameConstructor::ContentRemoved  [mozilla/layout/base/nscssframeconstructor.cpp, line 9546]
nsCSSFrameConstructor::RecreateFramesForContent  [mozilla/layout/base/nscssframeconstructor.cpp, line 11103]
nsCSSFrameConstructor::RestyleElement  [mozilla/layout/base/nscssframeconstructor.cpp, line 9974]
0x02b13698

Talkback ID: TB31322403H
nsGenericHTMLElement::GetFormControlFrameFor  [mozilla/content/html/content/src/nsgenerichtmlelement.cpp, line 1618]
nsGenericHTMLElement::GetFormControlFrame  [mozilla/content/html/content/src/nsgenerichtmlelement.h, line 263]
nsGenericHTMLFormElement::GetDesiredIMEState  [mozilla/content/html/content/src/nsgenerichtmlelement.cpp, line 2608]
nsIMEStateManager::GetNewIMEState  [mozilla/content/events/src/nsimestatemanager.cpp, line 242]
nsIMEStateManager::OnChangeFocus  [mozilla/content/events/src/nsimestatemanager.cpp, line 118]
nsEventStateManager::SetContentState  [mozilla/content/events/src/nseventstatemanager.cpp, line 3909]
nsGenericHTMLFormElement::SetFocusAndScrollIntoView  [mozilla/content/html/content/src/nsgenerichtmlelement.cpp, line 2885]
nsHTMLSelectElement::SetFocus  [mozilla/content/html/content/src/nshtmlselectelement.cpp, line 1631]
nsHTMLInputElement::Focus  [mozilla/content/html/content/src/nshtmlinputelement.cpp, line 1148]
XPCWrappedNative::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2247]
I can't get the testcase to crash, but I see the following assertion
(which probably should be NS_WARNING)
###!!! ASSERTION: Focus events should not be getting thru when this is null!: 'shell', file /home/smaug/mozilla/mozilla_cvs/mozilla/content/events/src/nsEventStateManager.cpp, line 1021
Can you make a testcase that doesn't use Math.random()?
No, unfortunately, I wasn't able to to get a testcase without Math.random(), thus far.
I got a different stack in a debug build, looks like

>	nsTypedSelection::selectFrames() Line 4954	C++
 	nsTypedSelection::selectFrames() Line 5066	C++
 	nsTypedSelection::AddRange() Line 5720	C++
 	nsEventStateManager::MoveCaretToFocus() Line 5020	C++
 	nsGenericHTMLElement::SetElementFocus() Line 3081	C++
 	nsGenericHTMLElement::Focus() Line 3108	C++
 	nsGenericHTMLElementTearoff::Focus() Line 197	C++
 	NS_InvokeByIndex() Line 102	C++
 	XPCWrappedNative::CallMethod() Line 2247	C++
 	XPC_WN_CallMethod() Line 1464	C++
  ... lots, lots, more ...

crashes on a deleted frame.
Flags: wanted1.8.1.x-
Flags: wanted1.8.0.x-
Whiteboard: [sg:critical?]
dbaron/vlad, can you help figure out the security risk on this one?
Flags: blocking1.9+
Dbaron, please take a look at this one.
Assignee: events → dbaron
I got a crash sometimes on Mac.  This testcase doesn't use Math.random() and it's pretty reliable.  Just load the testcase and click the <select> to crash.  I don't know whether this is the same bug as the Windows crash Martijn saw.

#6  0x90b41a93 in __cxa_pure_virtual ()
#7  0x054f39b4 in nsCocoaWindow::Show (this=0x3d9dbce0, bState=1) at /Users/jruderman/trunk/mozilla/widget/src/cocoa/nsCocoaWindow.mm:495
#8  0x17f88e8e in nsView::SetVisibility (this=0x3d9dbc80, aVisibility=nsViewVisibility_kShow) at /Users/jruderman/trunk/mozilla/view/src/nsView.cpp:459
...
My testcase is suspiciously similar to the testcase in bug 377938.
This is still crashing with a trunk build from yesterday on windowsXP.
http://crash-stats.mozilla.com/report/index/440467f0-2e4e-11dc-a818-001a4bd46e84

The testcase that is crashing on mac doesn't crash for me (unsurprisingly).
Assignee: dbaron → jwalden+bmo
Waldo, have you had time to look at this more?  
I'll look into this on Monday or Tuesday.
Still crashes for me with a trunk build from 2007-11-01.
Assignee: jwalden+bmo → Olli.Pettay
Priority: -- → P2
Can't reproduce on linux.
Sorry, because I can't reproduce this, it is hard to make a patch.
Assignee: Olli.Pettay → nobody
QA Contact: ian → events
this looks like a layout bug, no?
Assignee: nobody → dholbert
Component: Event Handling → Layout
QA Contact: events → layout
If it's relevant, I get this assertion on pageload:

###!!! ASSERTION: Adding child where we already have a child?  This will likely misbehave: 'Error', file /scratch/work/builds/trunk.07-11-20.11-42/mozilla/docshell/shistory/src/nsSHEntry.cpp, line 595

This assertion has several bugs filed: bug 307241, bug 314457, bug 344216, bug 396836
Status: NEW → ASSIGNED
Here's a stacktrace of the "reduced testcase for mac crash".

From stepping through the crashing code, it looks like the "GetPrimaryFrameFor" call at
 http://mxr.mozilla.org/seamonkey/source/content/html/content/src/nsGenericHTMLElement.cpp#1576
is returning an already-destroyed frame.

Then we call "CallQueryInterface" on that destroyed frame, and we hit a null pointer and crash.

The destroyed primary-frame is for a nsHTMLSelectElement.
Depends on: 399852
Comment on attachment 290278 [details]
stacktrace for reduce testcase crash

>#3  0x0a27dcb0 in nsHTMLSelectElement::PreHandleEvent (this=0x30ab9380, aVisitor=@0xbfffba80) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/content/html/content/src/nsHTMLSelectElement.cpp:1458
>#4  0x0a229aec in nsEventTargetChainItem::PreHandleEvent (this=0x34d2620, aVisitor=@0xbfffba80) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/content/events/src/nsEventDispatcher.cpp:186
>#5  0x0a22a770 in nsEventDispatcher::Dispatch (aTarget=0x30ab9380, aPresContext=0x30a5ea80, aEvent=0xbfffbf80, aDOMEvent=0x0, aEventStatus=0xbfffbd20, aCallback=0xbfffbb90) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/content/events/src/nsEventDispatcher.cpp:436
>#6  0x09f0b754 in PresShell::HandleEventInternal (this=0x3341c00, aEvent=0xbfffbf80, aView=0x30abbdf0, aStatus=0xbfffbd20) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/layout/base/nsPresShell.cpp:5802
>#7  0x09f0b078 in PresShell::HandleEvent (this=0x3341c00, aView=0x30abbdf0, aEvent=0xbfffbf80, aEventStatus=0xbfffbd20) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/layout/base/nsPresShell.cpp:5616
>#8  0x0a3a33b0 in nsViewManager::HandleEvent (this=0x30a5fca0, aView=0x30abbdf0, aPoint={x = 178437272, y = 53316176}, aEvent=0xbfffbf80, aCaptured=0) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/view/src/nsViewManager.cpp:1296
>#9  0x0a3a317c in nsViewManager::DispatchEvent (this=0x30a5fca0, aEvent=0xbfffbf80, aStatus=0xbfffbe90) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/view/src/nsViewManager.cpp:1252
>#10 0x0a39a0f4 in HandleEvent (aEvent=0xbfffbf80) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/view/src/nsView.cpp:168
>#11 0x074ba0e0 in nsChildView::DispatchEvent (this=0x30abbe50, event=0xbfffbf80, aStatus=@0xbfffbf70) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/widget/src/cocoa/nsChildView.mm:1347
>#12 0x074bc3f0 in -[ChildView sendFocusEvent:] (self=0x30abbef0, _cmd=0x152cf80, eventType=105) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/widget/src/cocoa/nsChildView.mm:2085
>#13 0x074c2b50 in -[ChildView becomeFirstResponder] (self=0x30abbef0, _cmd=0x90a87804) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/widget/src/cocoa/nsChildView.mm:4163
>#14 0x9384bf40 in -[NSWindow makeFirstResponder:] ()
>#15 0x074b7950 in nsChildView::TearDownView (this=0x30abf830) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/widget/src/cocoa/nsChildView.mm:484
>#16 0x074b7bb0 in nsChildView::Destroy (this=0x30abf830) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/widget/src/cocoa/nsChildView.mm:551
>#17 0x0a39a5f0 in nsView::~nsView (this=0x30abf7c0, __in_chrg=3) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/view/src/nsView.cpp:255
>#18 0x0a39a368 in nsView::~nsView (this=0x30abf7c0) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/view/src/nsView.cpp:200
>#19 0x0a39a878 in nsIView::Destroy (this=0x30abf7c0) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/view/src/nsView.cpp:297
>#20 0x09f451ac in nsFrame::Destroy (this=0x34d5ea0) at /Users/dholbert/builds/all/trunk.07-11-26.08-38/mozilla/layout/generic/nsFrame.cpp:505

Should this be happening?  Seems like we shouldn't be able to do this...
Maybe fixed by patch on bug 399852, as dependency indicates.
Whiteboard: [sg:critical?] → [sg:critical?][duplicate of 399852?]
This is now worksforme, using today's trunk build. It was still crashing using 2008-02-16 build, so this was fixed by bug 399852.

If someone is still seeing crashes on Mac (there was some talk of that), a new bug should be filed for it.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
dholbert, I don't see a <select> when I load your testcase in a Mac trunk debug build.
(In reply to comment #21)
> dholbert, I don't see a <select> when I load your testcase in a Mac trunk debug
> build.

I'm assuming you're talking about attachment 267504 [details], which you actually posted, not me. :)

Since the testcase relies on "onfocus", you need to not have the page body un-focused at page-load time.  Try these steps:
 1. Load the URL.
      -->get blank page.
 2. Click in location or search bar, to focus that
 3. Click reload button.
      --> Get a <select> overlaid on an iframe

Using those steps, this is actually worksforme on Mac *before* the 2/19 landing of bug 399852, using this nightly build:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008021304 Minefield/3.0b4pre

So maybe something else fixed this on mac.  I'm going to do a bit of investigation to figure out what fixed it.

> Using those steps, this is actually worksforme on Mac *before* the...

Sorry -- I should've said "Using those steps, and then clicking on the <select>", which is what used to trigger the crash, I think.
(In reply to comment #22)
> Using those steps, this is actually worksforme on Mac *before* the 2/19 landing
> of bug 399852

> So maybe something else fixed this on mac.  I'm going to do a bit of
> investigation to figure out what fixed it.

So, firstly: I can reproduce this crash on Mac OS 10.4.11, but not on 10.5.1.

Secondly, on Mac OS 10.4.11, the crash seems to have become fixed between these nightlies:
  2008-01-17-04
  2008-01-18-04

  Bonsai link:  http://tinyurl.com/ynwsv8
Initially, the only checkin I see on Bonsai that looks related is for bug 323740, but that just adds a null-check, so I'm not sure how that'd fix this bug.
Flags: in-testsuite?
dholbert: daniel do you still crash on mac 10.4 ? 

i cannot reproduce this crash on 10.5.2 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050614 Minefield/3.0pre
I no longer have my 10.4 machine, so I can't test this on 10.4 anymore.
Group: core-security
Crash Signature: [@ nsGenericHTMLElement::GetFormControlFrameFor] [@ DoDeletingFrameSubtree]
Crash tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d44e8ca18d0
Crash Signature: [@ nsGenericHTMLElement::GetFormControlFrameFor] [@ DoDeletingFrameSubtree] → [@ nsGenericHTMLElement::GetFormControlFrameFor] [@ DoDeletingFrameSubtree]
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: