If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Crash [@ nsSplitterFrameInner::SetPreferredSize] with this testcase when resizing using xul:splitter

VERIFIED FIXED

Status

()

Core
Layout
--
critical
VERIFIED FIXED
12 years ago
10 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: roc)

Tracking

(4 keywords)

Trunk
regression, testcase, verified1.8.0.13, verified1.8.1.5
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

12 years ago
See upcoming testcase, it crashes when using the splitter.

Talkback ID: TB10384493H

nsSplitterFrameInner::SetPreferredSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSplitterFrame.cpp,
line 1056]
nsSplitterFrameInner::AdjustChildren 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSplitterFrame.cpp,
line 1026]
nsSplitterFrameInner::AdjustChildren 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSplitterFrame.cpp,
line 980]
nsSplitterFrameInner::MouseDrag 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSplitterFrame.cpp,
line 581]
nsSplitterFrame::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSplitterFrame.cpp,
line 480]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6039]
PresShell::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 5850]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp,
line 2545]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp,
line 2237]
HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsView.cpp, line
174]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1060]
nsWindow::DispatchMouseEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 5793]
ChildWindow::DispatchMouseEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 6044]
nsWindow::WindowProc 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1249]
USER32.dll + 0x27b17 (0x77d37b17)
USER32.dll + 0x2cdce (0x77d3cdce)
USER32.dll + 0x4435 (0x77d14435)
USER32.dll + 0x9611 (0x77d19611)
nsAppStartup::Run 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 162]
main 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp,
line 61]
kernel32.dll + 0x1eb69 (0x77e5eb69)
(Reporter)

Comment 1

12 years ago
Created attachment 198922 [details]
testcase

Hmm, seems like a regression, this doesn't crash Mozilla1.7.
(Reporter)

Comment 2

12 years ago
Ok, doesn't crash with 2004-08-09 build, but crashes with 2004-08-10 build, so
that leads me to think this could be a regression from bug 230170.
Keywords: regression
I don't really know how to fix this, offhand.  The problem is that the splitter
creates this list of frames in mChildInfosBefore when it gets a mousedown.  Then
in mousedrag it does things with them.  But in this case, the frame gets
destroyed after the mousedown (when we flush out pending restyles because
someone wants a box object) and so we get:

(gdb) fram 0
#0  0xb660157b in nsSplitterFrameInner::SetPreferredSize (this=0x840f1e8, 
    aState=@0xbfffe650, aChildBox=0x8913894, aOnePixel=14, aIsHorizontal=1, 
    aSize=0xbfffe640)
    at ../../../../../mozilla/layout/xul/base/src/nsSplitterFrame.cpp:1056
1056      aChildBox->GetMargin(margin);
(gdb) p/x *aChildBox
$4 = {<nsISupports> = {_vptr.nsISupports = 0x0}, mRect = {x = 0xdddddddd, 
    y = 0xdddddddd, width = 0xdddddddd, height = 0xdddddddd}, mContent =
0xdddddddd, 
  mStyleContext = 0xdddddddd, mParent = 0xdddddddd, mNextSibling = 0xdddddddd, 
  mState = 0xdddddddd}

Which is not happy, as you might imagine.

You could probably get the same result in a build from before bug 230170 got
fixed by doing the style change off a 0-msec timeout....
Keywords: helpwanted
OS: Windows XP → All
Hardware: PC → All
A simple band-aid approach is just to have the nsSplitterInfos store a strong
reference to the content objects for the children, and get the box from those
objects every time.
Assignee: nobody → roc
Created attachment 199134 [details] [diff] [review]
fix as described
Attachment #199134 - Flags: superreview?(bzbarsky)
Attachment #199134 - Flags: review?(bzbarsky)
Comment on attachment 199134 [details] [diff] [review]
fix as described

Seems reasonable, but are we sure we always get the mouseup (eg if the mouseup
happens outside the window)?  I guess we're capturing?
Attachment #199134 - Flags: superreview?(bzbarsky)
Attachment #199134 - Flags: superreview+
Attachment #199134 - Flags: review?(bzbarsky)
Attachment #199134 - Flags: review+
I think it doesn't *really* matter if we don't. The only effect will be that we
keep the content alive longer than we otherwise would. We'll still destroy the
frame eventually and that will drop the reference.
OK.  Sounds good.
checked into trunk. We may want to consider this for the branch on the chance
that it's exploitable.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Crash is verified FIXED using SeaMonkey 1.1a;Mozilla/5.0 (Windows; U; Windows NT
5.1; en-US; rv:1.9a1) Gecko/20051012 Mozilla/1.0.
Status: RESOLVED → VERIFIED
Blocks: 230170

Updated

11 years ago
Duplicate of this bug: 380856
Blocks: 380856
Comment on attachment 199134 [details] [diff] [review]
fix as described

Seeking branch approval --- the patch applies fine and fixes the crash.
Attachment #199134 - Flags: approval1.8.1.5?
Attachment #199134 - Flags: approval1.8.0.13?
Comment on attachment 199134 [details] [diff] [review]
fix as described

approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Attachment #199134 - Flags: approval1.8.1.5?
Attachment #199134 - Flags: approval1.8.1.5+
Attachment #199134 - Flags: approval1.8.0.13?
Attachment #199134 - Flags: approval1.8.0.13+
Checked into branches.
Keywords: fixed1.8.0.13, fixed1.8.1.5

Comment 15

10 years ago
Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070710 BonEcho/2.0.0.5pre and the attached "testcase". I crashed in a 2007-07-02 build (without the patch), but don't crash in today's build.
Keywords: fixed1.8.1.5, helpwanted → verified1.8.1.5
Verified fixed using Thunderbird version 1.5.0.13 (20070809) on MacIntel, by setting the start page to the testcase URL. Resizing did not crash it.
Keywords: fixed1.8.0.13 → verified1.8.0.13
You need to log in before you can comment on or make changes to this bug.