Last Comment Bug 336994 - Crash when window gets destroyed on SVGZoom event
: Crash when window gets destroyed on SVGZoom event
Status: RESOLVED FIXED
[sg:critical?]
: crash, fixed1.8.0.12, testcase, verified1.8.1.4
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: x86 Windows XP
: -- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug]
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-07 04:44 PDT by Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
Modified: 2009-04-24 11:10 PDT (History)
7 users (show)
dveditz: blocking1.8.1.4+
dveditz: blocking1.8.0.12+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase, crashes on load (1011 bytes, text/html)
2006-05-07 04:46 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details
Backtrace from debug build (5.18 KB, text/plain)
2006-05-07 04:46 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details
testcase2 (1014 bytes, text/html)
2006-05-07 04:50 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details
proposed patch (1.23 KB, patch)
2007-02-22 05:54 PST, Olli Pettay [:smaug]
tor: review+
tor: superreview+
dveditz: approval1.8.1.4+
dveditz: approval1.8.0.12+
Details | Diff | Review

Description Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-05-07 04:44:20 PDT
See upcoming testcase, which crashes Mozilla on load. Also crashes in Firefox1.5.0.3.
Marking security sensitive, just to be sure.

Talkback ID: TB18394613W
0x00000000
nsSVGSVGElement::DidModifySVGObservable   nsSVGValue::NotifyObservers   nsSVGValue::DidModify   XPTC_InvokeByIndex   XPCWrappedNative::CallMethod
Comment 1 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-05-07 04:46:16 PDT
Created attachment 221163 [details]
testcase, crashes on load

The iframe with the svg has this code:

<script xmlns="http://www.w3.org/1999/xhtml">
window.addEventListener('SVGZoom', doe, true);
function doe(e) {
var x= parent.document.getElementsByTagName('iframe')[0];
x.parentNode.removeChild(x);
}
setTimeout(doe2, 1000);

function doe2() {
document.documentElement.currentScale = 2;
}
</script>
</svg>
Comment 2 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-05-07 04:46:37 PDT
Created attachment 221164 [details]
Backtrace from debug build
Comment 3 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-05-07 04:50:55 PDT
Created attachment 221166 [details]
testcase2

Same crash seems to happen with SVGScroll event.
Comment 4 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2007-02-01 14:52:50 PST
This now got worksforme between 2007-01-04 and 2007-01-05:
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=2007-01-04+04&maxdate=2007-01-05+09&cvsroot=%2Fcvsroot
It seems that this was somehow fixed by bug 333078?
So I guess this bug can be resolved then, right?
Comment 5 Boris Zbarsky [:bz] 2007-02-01 17:50:04 PST
I don't think so.  If it's just something that's hidden because of when the cycle collector happens to run or not run, that's not really good enough.  In my opinion, of course.

In any case, the fix for this bug is obvious.  DO NOT hold a weak ref to an object you're dispatching an event on.  Just don't do it.  So fix nsSVGSVGElement::DidModifySVGObservable to hold a strong ref to the presshell.  ;)  And then probably audit the rest of the code that calls that method.
Comment 6 Olli Pettay [:smaug] 2007-02-22 05:54:27 PST
Created attachment 256027 [details] [diff] [review]
proposed patch

Keep strong ref to presShell.
Went quickly through the code and this really should be enough.
The patch applies to branch and trunk.
Comment 7 Olli Pettay [:smaug] 2007-02-22 12:13:55 PST
Checked in
Comment 8 Daniel Veditz [:dveditz] 2007-03-20 10:58:14 PDT
Comment on attachment 256027 [details] [diff] [review]
proposed patch

Is this patch appropriate for 1.8.0 as well? Firefox 1.5.0.x is also affected by this bug.
Comment 9 Daniel Veditz [:dveditz] 2007-03-21 15:38:23 PDT
Comment on attachment 256027 [details] [diff] [review]
proposed patch

approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers
Comment 10 Marcia Knous [:marcia - use ni] 2007-05-08 14:12:45 PDT
verified on the 1.8 branch using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.4pre) Gecko/2007050804 BonEcho/2.0.0.4pre. No crash for me, but i do still see the iframe in the page. Martijn attributes that to a test case error. Adding branch fixed keyword.
Comment 11 Bob Clary [:bc:] 2009-04-24 11:10:34 PDT
crash test landed
http://hg.mozilla.org/mozilla-central/rev/8334854afb84

Note You need to log in before you can comment on or make changes to this bug.