Load the indicated URL and use shift-reload. You'll see the left hand side (the SVG) display partially and disappear. At the same time these couple of assertions show up in the console: ###!!! ASSERTION: Unexpected child of document element containing block: 'mDocElementContainingBlock->GetFirstChild(nsnull) == nsnull', file /home/tor/src/moz-trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9261 Break: at file /home/tor/src/moz-trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9261 ###!!! ASSERTION: already have a child frame: 'mFrames.IsEmpty()', file /home/tor/src/moz-trunk/mozilla/layout/generic/nsHTMLFrame.cpp, line 286 Break: at file /home/tor/src/moz-trunk/mozilla/layout/generic/nsHTMLFrame.cpp, line 286 Loading the SVG by itself (right click on the area, select "show only this frame") works fine.

is this a trunk-only regression?

Yes, trunk only.

This worskforme in a Firefox build pulled at MOZ_CO_DATE="Sun Oct 16 21:52:07 CDT 2005" Did this regress since then?

OK, I just retested with a Linux Firefox build pulled at MOZ_CO_DATE="Wed Oct 19 00:30:35 CDT 2005", clean profile. No assertions other than the known session history one.

Bracketing the regression narrowed it down to the checkin for bug 288064 for my tree, which went in at 19:50 CDT 16-Oct-2005.

Copy paste error from bonsai - went in at 17:50 CDT.

Hmm... That fix is definitely in my tree, and I'm not seeing the assert, as I said... Are there any interesting changes to the tree that's having the problem?

No, those trees were stock pulls. Were you using shift-reload? It seems to load ok if it pulls from the cache.

This also seems to affect SVG in an object tag (e.g. http://169.237.230.40/ins-peak.php?section=500RF&ins=MDD&result=PD) Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051019 Firefox/1.6a1 Fresh build, no special options. Noticed in the same time frame (Friday 2005/10/15 works, Monday 2005/10/17 does not). Sorry, but it's not a debug build, so don't have console output.

The constructor for nsHTMLObjectElement::nsHTMLObjectElement doesn't set nsObjectLoadingContent.mType (which defaults to eType_Loading). Backing out the change to line 525 of nsObjectLoadingContent.cpp fixes things.

Ah, ok. I managed to reproduce this. The key is that if you actually break on the session history assertion, this one is not hit. Jeremy, thanks for a testcase that doesn't involve session history! > The constructor for nsHTMLObjectElement::nsHTMLObjectElement doesn't set > nsObjectLoadingContent.mType (which defaults to eType_Loading). That's correct. We want that. ;)

<sigh>. So the sequence of events here is the following: 1) We start the load. We're in the LOADING state. 2) The data starts coming in. We transition out of the LOADING state (and our type changes to "document"). This posts a frame reconstruct event for the content node. 3) We start parsing the data and building up the DOM. 4) The frame reconstruct fires. This recreates the subdocument frame, and calls InitialReflow() on the presshell inside the new subdocument frame (via nsSubDocumentFrame::ShowDocShell calling nsDocShell::SetVisibility calling DocumentViewerImpl::Show calling DocumentViewerImpl::InitPresentationStuff). We construct frames for whatever part of the tree has been parsed so far. 5) Parsing finishes, and the XML content sink notifies on all the content. Including content that's already been parsed on. So the real fundamental problem is that the XML sink is not incremental and can't deal with layout trying to happen before it's done parsing... We have a number of bugs on various consequences of this. So short-term we could just stop doing the LOADING state for objects. But you can get the same results by setting display on the iframe, of course. Or suddenly making it floating while the SVG is loading. Or anything else that messes with the rendering while we're parsing the XML. So we should really find a better way to do this. Is this the time to bite the bullet and make XML notify on the partially-parsed document? Or should we look for something else?

I think that the subframe call InitialReflow() on its contained document. This is causing us badness in xslt since we end up performing initial reflow on the source document and then crash once the result document is done and we try to replace ths source.

mmm, maybe we need to create subdocumentframes in ObjectURIChanged even if aNotify=false?

> I think that the subframe call InitialReflow() on its contained document. Yes, it is. See item 4 in comment 12. > This is causing us badness in xslt There's xslt involved? I suppose if you have SVG disabled... I would say that if there is XSLT attached to the document we should simply not put the root element in the document until after the transform happens. Or something. We don't want to be rendering a partial source document when XSLT is involved, in any case. > create subdocumentframes in ObjectURIChanged even if aNotify=false? How would that help?

ah, nevermind... it wouldn't.

So i missed a word in my last comment :) It should say: I think that the subframe *shouldn't* call InitialReflow() on its contained document. The reason i mentioned xslt is just like you say, we don't want to do layout of half a source xslt document, so just teaching the xml sink to do incremental layout isn't enough. I'm not super exited about not putting the content in the document until it is loaded. We have to put it in there anyway before the transformation since otherwise we wouldn't be transforming the proper document. One idea I had back in the day was to let the subframe check if the document was still being loaded, and if so not make the call. Though I really don't understand why the call should ever be needed, doesn't the sink take care of that?

> I think that the subframe *shouldn't* call InitialReflow() on its contained > document. What if you have a display:none iframe and show it? Shouldn't that call InitialReflow() after it creates the presshell? I believe this is why we're doing it... > I'm not super exited about not putting the content in the > document until it is loaded. For XSLT, I think this is exactly the thing to change. We know up front whether we have to XSLT, and if we do, don't put the content into the document until right before we start the transform. That way the transform source can never be laid out. > Though I really don't understand why the call should ever be needed, doesn't > the sink take care of that? InitialReflow is only called on presshells, not documents. If the load finishes in a display:none iframe (so no presshell) and then we show the iframe, where should the InitialReflow call come from? That's why it's needed....

Don't we need to do reflow in case style information is queried on an document in a display:none iframe? Though I guess we could do the reflow at that time. I'm still not convinced that it's a good idea to start reflow under the sink if the document is being loaded. Seems like at best would be a perf problem if we start layout too soon, and at worst cause the sink to blow? Continued the xslt discussion in bug 182460 comment 39 since it's not really related to this bug.

> Don't we need to do reflow in case style information is queried on an document > in a display:none iframe? What style information? Layout-dependent stuff like offset*, etc just returns 0 for nodes in such documents. getComputedStyle throws. > I'm still not convinced that it's a good idea to start reflow under the sink > if the document is being loaded. I'm not sure what you mean by "under the sink"... Back to this bug, the other option is to not construct the subdocument frame for loading stuff. Or something. For objects with a standby attribute we want to show that instead, but almost no objects have one...

> What style information? Layout-dependent stuff like offset*, etc just returns > 0 for nodes in such documents. getComputedStyle throws. Right, is that what we want to do? I guess if people really needed the style information they should not set display:none on the iframe. > I'm not sure what you mean by "under the sink"... While the sink is loading stuff into the document and expects to be the one calling InitialReflow on it. My point is that the sink is having enough trouble with notifications and such as it is, without having to worry about that someone might do initial reflow at any point in time. > Back to this bug, the other option is to not construct the subdocument frame > for loading stuff. Or something. For objects with a standby attribute we > want to show that instead, but almost no objects have one... Hmm.. we'd need to show a partially loaded iframe though, no? Maybe we could just avoid calling InitialReflow if the document is in the process of being loaded, and rely on the sink to do it?

> While the sink is loading stuff into the document and expects to be the one > calling InitialReflow on it. Frankly, I think the fact that the sink is what calls InitialReflow is not a very good setup. There should be no real reason for the sink to even know that presshells exist... > Hmm.. we'd need to show a partially loaded iframe though, no? Yeah. I was sort of focusing on <object>/<embed> there... > Maybe we could just avoid calling InitialReflow if the document is in the > process of being loaded, and rely on the sink to do it? We could, yes. Again, that would mean that we'd try to paint the iframe when there is nothing inside it. I suppose we could handle that gracefully. So sounds like we need an API on nsIDocument that indicates whether the document is ready to be presented, and a way for the sink to affect this answer (either a setter on nsIDocument or a getter on the sink).

So is this better now that we have incremental XML?

tor? Is this still a problem? The codepath I describe in comment 12 should work correctly now, I believe.

ping? Is this still a problem?

The only assertion we get these days for this testcase is from session history ("###!!! ASSERTION: Adding child where we already have a child? This will likely misbehave...").

Sounds like a worksforme...