814398 - ISimpleDOMNode::get_computedStyleForProperties may flush layout during tree update

Status: NEW

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

the stack is:

 	xul.dll!RealBreak()  Line 457	C++
 	xul.dll!Break(const char * aMsg)  Line 556	C++
 	xul.dll!NS_DebugBreak_P(unsigned int aSeverity, const char * aStr, const char * aExpr, const char * aFile, int aLine)  Line 415 + 0xc bytes	C++
 	xul.dll!PresShell::FlushPendingNotifications(mozFlushType aType)  Line 3778 + 0x2d bytes	C++
 	xul.dll!nsDocument::FlushPendingNotifications(mozFlushType aType)  Line 6099	C++
 	xul.dll!nsDocument::FlushPendingNotifications(mozFlushType aType)  Line 6088	C++
 	xul.dll!nsComputedDOMStyle::GetPropertyCSSValue(const nsAString_internal & aPropertyName, nsIDOMCSSValue * * aReturn)  Line 483	C++
 	xul.dll!nsComputedDOMStyle::GetPropertyValue(const nsAString_internal & aPropertyName, nsAString_internal & aReturn)  Line 259 + 0x2a bytes	C++
>	xul.dll!mozilla::a11y::sdnAccessible::get_computedStyleForProperties(unsigned short aNumStyleProperties, unsigned char aUseAlternateView, wchar_t * * aStyleProperties, wchar_t * * aStyleValues)  Line 250 + 0x3f bytes	C++
 	FsDomNodeFirefox.dll!65e88f8c() 	
 	FsDomNodeFirefox.dll!65e8b40c() 	
 	FsDomNodeFirefox.dll!65e81fc7() 	
 	FsDomNodeFirefox.dll!65e9a5d6() 	
 	FsDomNodeFirefox.dll!65e81a09() 	
 	FsDomNodeFirefox.dll!65e8629a() 	
 	ntdll.dll!777bf8b1() 	
 	KernelBase.dll!753d149d() 	
 	FsDomSrv.dll!64b84a9d() 	
 	ntdll.dll!777bfb81() 	
 	KernelBase.dll!753d0fa2() 	
 	FsDomSrv.dll!64b85081() 	
 	FsDomSrv.dll!64b84d39() 	
 	FsDomSrv.dll!64b57b12() 	
 	FsDomSrv.dll!64b5f7e2() 	
 	mozjs.dll!js::Interpret(JSContext * cx, js::StackFrame * entryFrame, js::InterpMode interpMode)  Line 3799 + 0xf bytes	C++

stack doesn't show where they call into us from but it seems NotificationController::WillRefresh has bunch of places where we fire events while eRefreshProcessingForUpdate flag is set. That could be just assertion, no real problem.

Comment 1

[alexander :surkov (:asurkov)]

there's another one (probably related):

 NS_ASSERTION(nsCSSPseudoElements::PseudoElementContainsElements(
                     topWithPseudoElementData->GetPseudo()),
                   "we should be in a pseudo-element that is expected to "
                   "contain elements");

http://mxr.mozilla.org/mozilla-central/source/layout/style/nsComputedDOMStyle.cpp#521

stack:

 	xul.dll!Break(const char * aMsg)  Line 556	C++
 	xul.dll!NS_DebugBreak_P(unsigned int aSeverity, const char * aStr, const char * aExpr, const char * aFile, int aLine)  Line 415 + 0xc bytes	C++
 	xul.dll!nsComputedDOMStyle::GetPropertyCSSValue(const nsAString_internal & aPropertyName, nsIDOMCSSValue * * aReturn)  Line 524 + 0x33 bytes	C++
 	xul.dll!nsComputedDOMStyle::GetPropertyValue(const nsAString_internal & aPropertyName, nsAString_internal & aReturn)  Line 259 + 0x2a bytes	C++
 	xul.dll!mozilla::a11y::sdnAccessible::get_computedStyleForProperties(unsigned short aNumStyleProperties, unsigned char aUseAlternateView, wchar_t * * aStyleProperties, wchar_t * * aStyleValues)  Line 250 + 0x3f bytes	C++
 	FsDomNodeFirefox.dll!65c68f8c() 	

Boris, do you know what this exception is about?

Comment 2

[Boris Zbarsky [:bzbarsky]]

That's supposed to be ensuring that our element is in fact in a situation that requires taking a slow computed style path.

What did topWithPseudoElementData->GetPseudo() return in this case?

Comment 3

[Trevor Saunders (:tbsaunde)]

> stack doesn't show where they call into us from but it seems
> NotificationController::WillRefresh has bunch of places where we fire events
> while eRefreshProcessingForUpdate flag is set. That could be just assertion,
> no real problem.

yeah, I think the assert is probably too general, if we're calling out to other peoples code we should probably assume they will do any crazy thing.  So I think we should
1. make sure we don't assume things about the world when we come back from fireing an event (atleast on windows)
2. make the assert only cover what happens during code we control.