Evil xul testcase, tree:hover{display:inline;}, crashes Mozilla

VERIFIED FIXED

Status

()

Core
Widget: Win32
--
critical
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: Ere Maijala (slow))

Tracking

({crash, regression, testcase})

Trunk
x86
Windows XP
crash, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

13 years ago
See upcoming testcase. 
When hovering over the content area, Mozilla crashes.
This seems a recent regression, 2005-10-17 build doesn't crash 2005-10-18 build does crash.
I've also tried a 1.5 branch build of 2005-10-23, it doesn't crash.
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-10-17+06%3A00%3A00&maxdate=2005-10-18+13%3A00%3A00&cvsroot=%2Fcvsroot
I've no idea which bug could be responsible
(Reporter)

Comment 1

13 years ago
Created attachment 201442 [details]
testcase
(Reporter)

Comment 2

13 years ago
Created attachment 201443 [details]
backtrace

This is the backtrace from the assertion I get prior to the crash:
###!!! ASSERTION: no scroll bar: 'mScrollbar', file c:/mozilla/mozilla/layout/xu
l/base/src/tree/src/nsTreeBodyFrame.cpp, line 782
Break: at file c:/mozilla/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.c
pp, line 782

After that, the backtrace of the sigsegv follows.
> I've no idea which bug could be responsible

Based on the crash stack, bug 307678.  This doesn't crash on Linux.
Assignee: nobody → win32
Blocks: 307678
Component: Layout → Widget: Win32
QA Contact: layout → ian
(Assignee)

Updated

13 years ago
Assignee: win32 → emaijala
(Assignee)

Comment 4

13 years ago
Created attachment 201543 [details] [diff] [review]
Patch v1

Use only weak refs to nsWindow in mouse trailer to avoid all kinds of problems such as re-entry to nsWindow destructor.
Attachment #201543 - Flags: review?(roc)
(Assignee)

Comment 5

13 years ago
Comment on attachment 201543 [details] [diff] [review]
Patch v1

Oops. A very flawed patch here...
Attachment #201543 - Attachment is obsolete: true
Attachment #201543 - Flags: review?(roc)
(Assignee)

Comment 6

13 years ago
Created attachment 201546 [details] [diff] [review]
Patch v1.1

Second try. Use weak refs but don't use NS_IF_RELEASE to get there to avoid nullifying the pointers.
Attachment #201546 - Flags: review?(roc)
+  // We're not going to hold a reference to the window. nsWindow just
+  // needs to do the right thing (nullify this if necessary) in its destructor.
+  if (topWin) {
+    topWin->Release();
+  }
   if (mHoldMouseWindow != topWin && mTimer) {
     // Make sure TimerProc is fired at least once for the old window
     TimerProc(nsnull, nsnull);
   }
-  NS_IF_RELEASE(mHoldMouseWindow);
   mHoldMouseWindow = topWin;

Is it possible for the TimerProc call to cause topWin to be destroyed? If so this would leave a dangling reference in mHoldMouseWindow.
(Assignee)

Comment 8

13 years ago
Comment on attachment 201546 [details] [diff] [review]
Patch v1.1

I have to assume it's possible (and it is of course). Which leads to a situation I'm not sure how to handle: if we don't hold the ref, we'll have a dangling pointer, if we do, we might hold the last ref to it and cause a horrible loop when releasing it.

Roc, how about just backing out the patch from bug 307678 and going for TrackMouseEvent?
Attachment #201546 - Attachment is obsolete: true
Attachment #201546 - Flags: review?(roc)
Yeah, I think that's the way to go on trunk. And we won't do anything about 307678 for branch, OK?
(Assignee)

Comment 10

13 years ago
Created attachment 201595 [details] [diff] [review]
Backout patch

Yes, unless someone finds a critical problem with that version too. 

This is a patch to backout the patch of bug 307678.
Attachment #201595 - Flags: superreview?(roc)
Attachment #201595 - Flags: review?(roc)
Attachment #201595 - Flags: superreview?(roc)
Attachment #201595 - Flags: superreview+
Attachment #201595 - Flags: review?(roc)
Attachment #201595 - Flags: review+
(Assignee)

Comment 11

13 years ago
Patch checked in. Work will continue in bug 312566.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Depends on: 312566
Resolution: --- → FIXED
Verified FIXED using build 2005-11-07-10 of SeaMonkey trunk on Windows XP; no crash.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.