Closed Bug 435065 Opened 17 years ago Closed 6 years ago

ASSERTION: Someone forgot to block scripts: 'aIsSafeToFlush == nsContentUtils::IsSafeToRunScript()' because tree views run arbitrary code while painting - viewing computed style in DOM Inspector

Categories

(Other Applications :: DOM Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: dholbert, Unassigned)

References

Details

(Keywords: assertion)

Attachments

(1 file)

STEPS TO REPRODUCE: 1. Install DOM Inspector extension. 2. Load any page (even about:blank) 3. Open DOM Inspector 4. Click the lower "HTML" element 5. In the right-hand dropdown, select "Computed Style" RESULTS: Many many copies of this assertion are printed: ###!!! ASSERTION: Someone forgot to block scripts: 'aIsSafeToFlush == nsContentUtils::IsSafeToRunScript()', file mozilla/layout/base/nsPresShell.cpp, line 4505 Additionally, every time I mouse-over the right pane when it's showing computed style, I get another copy of the assertion. Testing with a debug build of mozilla-central, which was up-to-date as of yesterday: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008052111 Minefield/3.1a1pre This may be a dupe of bug 434790 or bug 429962, but I figured I'd file a separate bug, since those ones don't mention DOM Inspector.
Version: Other Branch → Trunk
yeah filing a new bug is more following the rules than lame comments https://bugzilla.mozilla.org/show_bug.cgi?id=429962#c8
Ha, sorry Bernd, I overlooked that comment of yours. :) (Also I didn't know that DOMI was shorthand for DOM Inspector... learn something new every day!) Anyway -- yeah, let's keep this bug separate until it's determined to have the same cause as one of the other bugs mentioned in comment 0.
Same happens on Vista with latest trunk.
OS: Linux → All
Hardware: PC → All
Attached file stack (deleted) —
There are 334 assertions logged when doing the steps from comment 0. All of them looks like the stack in this attachment.
(In reply to comment #4) > Created an attachment (id=347000) [details] > stack > > There are 334 assertions logged when doing the steps from comment 0. All of > them looks like the stack in this attachment. This stack look also like on the one in Bug 429962
Only for the first frames but afterward it differs. I think it should be handled in its own bug.
Looks like someone is doing naughty things during painting, cc'ing some layout folks that might know what to do. The offending part is nsDocument::FlushPendingNotifications (nsdocument.cpp, line 6239) nsComputedDOMStyle::GetPropertyCSSValue (nscomputeddomstyle.cpp, line 320) nsComputedDOMStyle::GetPropertyValue (nscomputeddomstyle.cpp, line 293) NS_InvokeByIndex_P (xptcinvoke.cpp, line 102) XPCWrappedNative::CallMethod (xpcwrappednative.cpp, line 2405) ...JS stuff... nsXPCWrappedJS::CallMethod (xpcwrappedjs.cpp, line 564) PrepareAndDispatch (xptcstubs.cpp, line 114) SharedStub (xptcstubs.cpp, line 142) nsTreeBodyFrame::PaintText (nstreebodyframe.cpp, line 3534) nsTreeBodyFrame::PaintCell (nstreebodyframe.cpp, line 3279) nsTreeBodyFrame::PaintRow (nstreebodyframe.cpp, line 3078) nsTreeBodyFrame::PaintTreeBody (nstreebodyframe.cpp, line 2880) I.e. nsTreeBodyFrame::PaintText ends up calling into a JS component which then calls computerstyle.GetPropertyValue which flushes. The offending line in nsTreeBodyFrame::PaintText is likely the mView->GetCellText call. So I guess this is the DOM Inspector view implementation that calls back into gecko in ways it's not allowed to. Is this simply a DOM Inspector bug then? Or should we be able to deal with this by detecting the situation and not flushing?
In my opinion this is a DOM inspector bug. And more importantly, a complete f*ck-up when designing trees.
That is, calling script during painting, by design, means we just lose....
Yes, the root issue is that trees are evil. DOMI might be able to work around it by avoiding flushes.
Moving over to DOM Inspector then
Component: General → DOM Inspector
Product: Core → Other Applications
QA Contact: general → dom-inspector
I suppose we could have a flush-less internal way to get computed style, yeah...
Or DOMI could simply cache its data and not get computed style at all during the callback...
Yeah, it could eagerly compute all the data. I can live with that.
this isn't limited to domi. and changing domi wouldn't actually fix domi it affects all trees as soon as you use venkman (or any other debugger) to debug the treeview code. if someone really wants to fix a symptom instead of fixing the bug, i can file a new one for trees. the assertion makes debugging w/ venkman incredibly painful on windows. But i'd hope everyone agrees that this bug is really in trees. would it really be hard for trees to have their own two pass system: HandleEvent - PresShell::Paint - PaintTreeBody - --tree-clean - if tree-clean == 0 - mark tree dirty - post tree-update-event - if tree-clean > 0 - paint to image - nsTreeBodyFrame::PaintTreeBody - paint from image - tree-update-event - collect necessary information to paint - if new information doesn't match previous - post paint event to tree - tree-clean = 2
Summary: ASSERTION: Someone forgot to block scripts: 'aIsSafeToFlush == nsContentUtils::IsSafeToRunScript()' when viewing computed style in DOM Inspector → ASSERTION: Someone forgot to block scripts: 'aIsSafeToFlush == nsContentUtils::IsSafeToRunScript()' because tree views run arbitrary code while painting - viewing computed style in DOM Inspector
Sure, we could do something like that, but it will most likely result in visual regressions ... e.g. scrolling will leave some rows blank while script fills them in. We need more infrastructure to let script run at safe times. I know how to do that, I just never have time to work on it.
Should this still be under DOM Inspector, then?
Maybe Core -> "DOM: CSS Object Model" ?
Bulk close. This component is no longer supported or maintained. https://bugzilla.mozilla.org/show_bug.cgi?id=1499023
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INCOMPLETE
Bulk close. This component is no longer supported or maintained. https://bugzilla.mozilla.org/show_bug.cgi?id=1499023
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: