281424 - [FIX]Crash in nsIView::GetOffsetTo [@ nsIView::GetOffsetTo 3412f9d4]

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P1)

Description

[Alex Sirota (iosart)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050207 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050207 Firefox/1.0+

In the FoxyTunes extension (http://www.foxytunes.org) there is a button (the
button with a musical note) which when hovered over displays a popup.
In the latest Firefox nightlies this causes a crash. 
The crash can be easily reproduced if you hover over the button several times.

Reproducible: Always

Steps to Reproduce:
1. Install FoxyTunes 1.1 from http://www.foxytunes.org
2. Hover over the 'info' button for a few times


Actual Results:  
Browser crash

Comment 1

[Mike Connor [:mconnor]]

currently #3 on the talkback topcrasher list.

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=+nsIView%3A%3AGetOffsetTo+3412f9d4&vendor=All&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

I'm pretty certain that this is the wrong component.

Comment 2

[Alex Sirota (iosart)]

Attachment: minimal test case extension

Created a minimal (anything less doesn't cause a crash for me) test case.
1. Install the testcase.xpi extension
2. A button with a question mark will appear on the status bar
3. Hover over this button to get the crash

The things that cause the crash (removing one of them causes the crash to stop
happening):
1. A button that has an onmouseover handler
2. This handler calls showPopup on some popup element
3. This popup element has a onpopupshowing handler
4. In that handler get the "directory_service" service
5. On top of that the button has the following CSS style:
   - by default it has opacity of 0.99
   - on hover it has opacity of 1

It is of course possible that many additional scenarios will lead to the same
crash.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Does it have to be an extension?  Or will a file:// XUL page (which requests
UniversalXPConnect privileges so it can get the service) reproduce it too?  A
XUL page version of the testcase would make it a lot easier to debug this...

Comment 4

[Frank Wein [:mcsmurf]]

There is also another (easier way IMHO) to reproduce this, also it doesn't crash
all the time:
1. Open http://www.heise.de/newsticker/meldung/56281 (or another page with a
link, i used this)
2. Click on the link "Novell" in the article (first line)
3. When the new window appears, the site begins to load (Transfering from ....)
and the progress bar already appeared, press ESC to cancel the page load.

Comment 5

[Frank Wein [:mcsmurf]]

This regressed between 2004-02-03-06 and 2004-02-04-06, Bonsai link:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-02-03+05%3A00%3A00&maxdate=2005-02-04+07%3A00%3A00&cvsroot=%2Fcvsroot

Comment 6

[Frank Wein [:mcsmurf]]

Make that 2005- :)

Comment 7

[Frank Wein [:mcsmurf]]

Possible fallout from Bug 268352, that checkin looks suspicious to me?

Comment 8

[Alex Sirota (iosart)]

(In reply to comment #3)
> Does it have to be an extension?  Or will a file:// XUL page (which requests
> UniversalXPConnect privileges so it can get the service) reproduce it too?  A
> XUL page version of the testcase would make it a lot easier to debug this...

I have only been able to reproducte this in an overlay. 
Why would a XUL page make it easier to debug than an extension (maybe I can
provide a better test case)?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Because I'm not willing to install extensions.

Comment 10

[Martijn Wargers (dead)]

Attachment: css file for testcase

Comment 11

[Martijn Wargers (dead)]

Attachment: js file for testcase

Comment 12

[Martijn Wargers (dead)]

Attachment: Testcase

This testcase is derived from the "minimal test case extension"
It crashes for me when I hover over the "?"

Comment 13

[Martijn Wargers (dead)]

It doesn't crash for me with the build where I've backed out the first patch
from bug 268352 ("Fix coordinate systems").

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

The problem is that event dispatch can destroy aView, so we can't use it after
we've called HandleDOMEvent, say...  Note that I'm assuming that
GetCurrentEventFrame() will return null any time after the presshell is
destroyed, so we won't hit the code where we adjust the offset when there is no
viewmanager.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Nominating for 1.8b1 blocking, since this is pretty easy to trigger and we have
a fix in hand....

Comment 16

[Ryuichi KUBUKI]

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

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 174063 [details] [diff] [review]
Fix

I think we should take this for beta... It's a pretty safe crash fix for what
may turn out to be a common crash that's a recent regression...

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 174063 [details] [diff] [review]
Fix

a=asa for 1.8b checkin

Comment 19

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.8b1.

Comment 20

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b) Gecko/20050214 Firefox/1.0+

confirming FIXED using latest beast with patch