Closed Bug 537157 Opened 11 years ago Closed 11 years ago
Crash [@ Get
CSSComputed Value] with animate in svg in binding
See testcase. http://crash-stats.mozilla.com/report/index/c749df82-a453-4c17-9f31-7e2402091229 0 xul.dll GetCSSComputedValue content/smil/nsSMILCSSProperty.cpp:68 1 xul.dll nsSMILCSSProperty::GetBaseValue content/smil/nsSMILCSSProperty.cpp:107 2 xul.dll nsSMILAnimationFunction::WillReplace content/smil/nsSMILAnimationFunction.cpp:347 3 xul.dll nsSMILCompositor::ComposeAttribute content/smil/nsSMILCompositor.cpp:157
Confirmed. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20091229 Minefield/3.7a1pre
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
OS: Windows 7 → All
Version: unspecified → Trunk
In a debug build: ###!!! ABORT: actively animated document w/ no window: 'win', file /Users/jruderman/central/content/smil/nsSMILCSSProperty.cpp, line 66
So, there's apparently a "helper" nsXMLDocument that gets created for the XBL binding. This isn't a data document, so when we call nsDocument::GetAnimationController on it, it happily (lazily) creates an animation controller, with which we can register animations. But this helper document doesn't ever get a window, so we die as soon as we try to sample an animation of a CSS property. (because those currently assume they have a window, which they use for looking up their "base value" (the computed value) We can avoid this by just adding a null-check for mWindow to the initial "immediate return" cases, at the beginning of nsDocument::GetAnimationController.
Rrgh - sorry, I hit 'tab+enter' or something while I'd just started still typing that previous comment. I was going to say: smaug pointed out a few useful things in IRC: - We can check for this case in nsDocument::GetAnimationController by checking mLoadedAsInteractiveData (though we need to move that flag up to nsDocument from nsXMLDocument, in order to do this) - We don't actually need the window pointer in the first place, to look up the computed value. We can just look up the presShell & call NS_NewComputedDOMStyle. (which is all that the window's computed-style accessors do anyway) Patch to accomplish the above coming up...
This WIP patch accomplishes the second point from my previous comment (ditching the window pointer in favor of a null-checked PresShell & a call to NS_NewComputedDOMStyle). This turns this bug's crash into warning-spam (with the warning added in this patch being spammed once per sample). After I've got the mLoadedAsInteractiveData check, that will prevent us from animating at all, and that will fix the warning-spam.
This version adds the mLoadedAsInteractiveData check. This also removes the "LookupComputedStyleFor" convenience method (and related code) from dom/base/ns*Window*. This method was added in bug 474049, and its only caller is removed in this bug's patch. To remove it, I just directly reverted bug 474049's changes to ns*Window*, with these commands: > hg diff -r da6d67c2c140 -r 0187a51241b7 dom/base/nsGlobalWindow.* | patch -p1 -R > hg diff -r da6d67c2c140 -r 0187a51241b7 dom/base/nsPIDOMWindow.h | patch -p1 -R
Attachment #420159 - Attachment is obsolete: true
Attachment #420186 - Flags: superreview?(roc) → superreview+
Comment on attachment 420186 [details] [diff] [review] fix v1 You should update the IID of nsPIDOMWindow
Attachment #420186 - Flags: review?(Olli.Pettay) → review+
Thanks -- IID fixed in this version. Carrying forward r+sr.
Pushed with crashtest: http://hg.mozilla.org/mozilla-central/rev/15c10445598c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Crash Signature: [@ GetCSSComputedValue]
You need to log in before you can comment on or make changes to this bug.