Jesse Ruderman Reporter
	Description • 17 years ago

###!!! ASSERTION: Why are we calculating with a dirty list?: '!aList->IsDirty()', file /Users/jruderman/trunk/mozilla/layout/base/nsCounterManager.cpp, line 50

###!!! ASSERTION: Counter updates will be lost: '!mCountersDirty', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 10200


Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xe814ec8f

Thread 0 Crashed:
0   libgklayout.dylib        	0x19718dcd nsCounterList::SetScope(nsCounterNode*) + 185 (nsCounterManager.cpp:137)
1   libgklayout.dylib        	0x1971929f nsCounterList::RecalcAll() + 61 (nsCounterManager.cpp:176)
2   libgklayout.dylib        	0x19719387 RecalcDirtyLists(nsAString_internal const&, nsCounterList*, void*) + 37 (nsCounterManager.cpp:274)
3   libgklayout.dylib        	0x19d6c345 nsBaseHashtable<nsStringHashKey, nsAutoPtr<nsCounterList>, nsCounterList*>::s_EnumReadStub(PLDHashTable*, PLDHashEntryHdr*, unsigned, void*) + 81 (nsBaseHashtable.h:327)
4   libxpcom_core.dylib      	0x012f5764 PL_DHashTableEnumerate + 191 (pldhash.c:724)
5   libgklayout.dylib        	0x19d6c000 nsBaseHashtable<nsStringHashKey, nsAutoPtr<nsCounterList>, nsCounterList*>::EnumerateRead(PLDHashOperator (*)(nsAString_internal const&, nsCounterList*, void*), void*) const + 128 (nsBaseHashtable.h:188)
6   libgklayout.dylib        	0x19719447 nsCounterManager::RecalcAll() + 41 (nsCounterManager.cpp:281)
7   libgklayout.dylib        	0x196ee43c nsCSSFrameConstructor::RecalcQuotesAndCounters() + 80 (nsCSSFrameConstructor.cpp:10199)
8   libgklayout.dylib        	0x1974db0f PresShell::FlushPendingNotifications(mozFlushType) + 315 (nsPresShell.cpp:4370)
9   libgklayout.dylib        	0x19798529 nsGfxScrollFrameInner::AsyncScrollPortEvent::Run() + 63 (nsGfxScrollFrame.cpp:1902)
10  libxpcom_core.dylib      	0x013557ea nsThread::ProcessNextEvent(int, int*) + 508 (nsThread.cpp:483)
...

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 1 • 17 years ago

So the problem is that nsSVGUseElement calls TriggerReclone() on any mutation observed in the subtree rooted by its target.  Including mutations in anonymous content (like counter list updates).  In this case, what that means is that frames that are using counters are blown away during re-resolution of the counter list... which is not a good idea, generally.  Good thing we have asserts about it.

I'd not like to see what happens with the current setup if <use> is pointed at something that contains a <textarea> and the user makes the mistake of typing in it.  ;)

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 2 • 17 years ago

The question is whether it's what we need to be doing for <use>...

Perhaps a better solution would be to make the reclone async in some way?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 3 • 17 years ago

Oh, as a note the z-index makes the green square not paint for me (on trunk).  Is that expected?

	tor
	Comment 4 • 17 years ago

(In reply to comment #2)
> Created an attachment (id=264205) [details]
> This fixes things for me
> 
> The question is whether it's what we need to be doing for <use>...
> 
> Perhaps a better solution would be to make the reclone async in some way?

Ideally I'd like to avoid cloning altogether and reuse the content nodes and just have a different frame tree atop them.  However I don't know how to do this or even if gecko has that functionality.

	tor
	Comment 5 • 17 years ago

(In reply to comment #3)
> Oh, as a note the z-index makes the green square not paint for me (on trunk). 
> Is that expected?

No, not expected.

Seems somewhat similar to 369850 - will reappear if resized vertically.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 6 • 17 years ago

Yeah, you can't really have multiple frames in the same presshell for the same content node and have it work right with dynamic updates at the moment...

So what _is_ the expected behavior when <use> points to a <html:textarea> and the user edits the textarea?  Or in general when it points to an <svg:foreignObject> with stateful stuff inside and state changes in a way not captured by the DOM?

	chris hofmann
	Comment 7 • 17 years ago

peterv, can your sr?

	Peter Van der Beken [:peterv]
	Comment 8 • 17 years ago

(In reply to comment #1)
> So the problem is that nsSVGUseElement calls TriggerReclone() on any mutation
> observed in the subtree rooted by its target.  Including mutations in anonymous
> content (like counter list updates).

Could you explain the fix a bit? It seems to only reclone when those mutations are in the same anonymous tree as the use element?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 9 • 17 years ago

The fix is basically to only reclone when the mutation is happening to some node X such that one can get from X to the <use> and from the <use> to X via DOM APIs.  For the typical (no <use> as XBL anon content) case, this is basically enforcing that neither is anonymous (which is what we want per spec).  If a <use> is XBL anon content, this is enforcing that X is anon content from the same binding.

Of course it's not clear how <use> and XBL should interact to start with, so....

For sure we don't want to reclone if the thing being changed is native anonymous, so we could check for that instead, I suppose.  But what I put up makes more sense to me.

	Peter Van der Beken [:peterv]
	Comment 10 • 17 years ago

Comment on attachment 264205 [details] [diff] [review]
This fixes things for me

Makes sense then.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 11 • 17 years ago

Checked in.

	Daniel Veditz [:dveditz]
	Comment 12 • 17 years ago

The testcase does not crash for me in a FF2.0.0.4 debug build, although I get the two assertions mentioned in comment 0.

Is this a trunk-only problem? Mac only? (filed that way, but I don't see anything Mac-specific in the fix.)

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 13 • 17 years ago

Do we support <use> on the branch?  If so, we need to fix this there too.

And this is not platform-specific.

	Robert Longson [:longsonr]
	Comment 14 • 17 years ago

(In reply to comment #13)
> Do we support <use> on the branch?  If so, we need to fix this there too.
> 
> And this is not platform-specific.
> 

We do support <use> on the branch however the update mechanism for <use> elements is different. On the trunk we use nsIMutationObserver, on the branch we use nsIDOMMutationListener.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 15 • 17 years ago

smaug, will that see the anonymous content mutations on branch?

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 16 • 17 years ago

Mutation events don't propagate from native anonymous content on branch anymore. Does that break something in 1.8 svg:use?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 17 • 17 years ago

No, it just means branch is not susceptible to this bug.  Good to know.

	Jesse Ruderman Reporter
	Comment 18 • 17 years ago

Crashtest checked in.

	Carsten Book [:Tomcat]
	Comment 19 • 16 years ago

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050614 Minefield/3.0pre - no crash on testcase.

Note: when using the Testcase i get:###!!! ASSERTION: Someone forgot to block scripts: 'aIsSafeToFlush == nsContentUtils::IsSafeToRunScript()' 

Bug 429962 covers this Assertion