###!!! ASSERTION: Adding child where we already have a child? This will likely misbehave: 'Error', file c:/mozilla/docshell/shistory/src/nsSHEntry.cpp, line 528 STEPS TO REPRODUCE: 1. Load http://www.neothermic.com/ 2. Hit either the reload button on the toolbar or hit Ctrl+R SeaMonkey, winxp home, build from today, bfcache enabled (in case it matters)

This testcase shows the assertion on reload.

Hmm. So we're trying to add a new SHEntry when we call DoContent(), but of course there's one at that position already (since this is a history load). For iframes, the logic in nsDocShell::LoadURI deals with this: 703 // If the parent url, bypassed history or was loaded from 704 // history, pass on the parent's loadType to the new child 705 // frame too, so that the child frame will also 706 // avoid getting into history. 707 loadType = parentLoadType; so we might need to either refactor things a tad, or do something similar in nsObjectLoadingContent...

We also hit this bug running DOM Core Level 1 and/or Core Level 2 test suites. http://www.w3.org/DOM/Test/

WFM with the testcase in comment 1 (on Mac, with a recent debug build of Firefox).

I can reproduce with the testcase in bug 344216, though.

I got this at http://www.mibbit.com/ which doesn't appear to do any document.write(). Self build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2pre) Gecko/2007112705 Firefox/3.0b2pre

Moving to wanted list...please ren-nom if you disagree this should really block

I think this should really block; as a result of that assertion we end up with some pretty broken data structures in this case. And this really shouldn't be that hard to fix, per comment 2.

(In reply to comment #8) > I think this should really block; as a result of that assertion we end up with > some pretty broken data structures in this case. And this really shouldn't be > that hard to fix, per comment 2. > Hey Bz - just worried about timing here - since this bug is 8 years old do you think we really should block 1.9 for it?

> since this bug is 8 years old Uh... it is? It was filed in 2005, after 1.8 branched. It's a bug in code that's new in 1.9 (as in, this is a regression from 1.8). Am I just completely missing something?

(In reply to comment #10) > > since this bug is 8 years old > > Uh... it is? It was filed in 2005, after 1.8 branched. It's a bug in code > that's new in 1.9 (as in, this is a regression from 1.8). > > Am I just completely missing something? > Um nope. Reading too many bugs at once. Sorry bout that...

I can reproduce this with the latest Linux 3.0b3pre with deviantART's logout page http://www.deviantart.com/users/rockedout

Just reproduced this by logging into GMail.

removing the wanted-next+ flag as this really blocks 1.9.

Moving to JST since biesi and bz are busy with other stuff...

So there doesn't seem to be a good way to completely solve this that I can see. The easy way to get rid of this assertion in this particular case is to copy some of the code referenced in comment #2 into OnLoadingSite(), with the appropriate checks there (mLoadFlags == 0 and mCurrentURI == null, among others), we can propagate the parents load flags to the child docshell (through the object tag), but we really only want to do that for the cases where we're loading a URL (channel) from an object tag due to a reload. The code referenced in comment #2 does that by looking at the parents load flags and whether or not there's a session history entry for the child shell. I tried to do the same thing in the OnLoadingSite() code, and as I said earlier, I can make this assertion go away, but not w/o also affecting subsequent loads into the same docshell, or another child shell that's later on dynamically created for another object tag. In that case the parents load flags mean nothing, but I don't see a reliable way to tell that case apart from the "really reloading" case. What makes this even more complicated is that in the case where you hit the reload button, we'll have a session history entry for the child shell, but if you do a hard reload (shift+reload), there won't be a session history entry for the child shell in OnLoadingSite(), yet the assertion also fires in that case. The assertion in the shift+reload case can be fixed by looking for more specific load flags and ignoring whether a session history entry exists for the child shell in those cases, but that means we mess up for dynamically created object tags etc. So it seems like we need a way to tell whether a child shell load is happening due to the parent being reloaded, not whether or not the parent happened to be reloaded when it was loaded. Seems like that could be done by clearing mLoadFlags at the right spot, but even that wouldn't be enough as it wouldn't necessarily do the right thing if a page dynamically creates object tags that cause child shells to be created during loading of the parent document etc. So this gets tricky, it seems, unless I've missed something obvious. And this isn't exactly in our most robust code, and given the fact that this hasn't showed up anywhere else as a real problem I'm strongly questioning the need, or even desire, for this to block the release. Boris, what are your thoughts on this? Did I miss an obvious fix here? I for sure wouldn't want to touch this too much any more in this release cycle...

Hmm... are all those issues there for iframes too? I agree that if simply doing what iframes do is not straightforward it's better to not touch this at all and hope for the best.... If we can do exactly what iframes do (even if it's somewhat buggy, which it probably is), I think we should: at least it's a known quantity.

Given comment 17 and comment 18 should we just move this to the next release?

Boris, given the way loading using an object tag differs from loading using an [i]frame (i.e. the channel is already opened and data is starting to come in before we know what to do with the data), the whole loading sequence is different enough that the code we use for [i]frames doesn't directly apply to object tags, and thus I think I'd rather not touch this code. I'll stick what I have in this bug and maybe you can have a look and maybe you can convince me otherwise, but for now I'll mark this blocking-.

This fixes this bug, but has all the problems discussed above.

I also hit this assertion when reloading HTML documents that have SVG embedded. (e.g. this testcase) I'm guessing it's due to the same bug. I'm seeing this using a current mozilla-central debug build on Ubuntu Linux 8.04.

I have the same assertion with http://www.europcar.de see : https://bugzilla.mozilla.org/show_bug.cgi?id=450223

Do you think that this problem can hang the browser in final builds ? (Europcar website hang Firefox with final builds but not with Minefield) I search what can freeze firefox when i load europcar.de and the only thing that i see is this assertion.

I don't think this can lead to hangs, no.

Jeff, you probably want to file a bug on your hang, with steps to reproduce. Loading the site doesn't hang for me, for example.

Prerequisites : - you can reproduce the problem only if you are not behind a proxy. - Clean your cache,offline data and try to make a hard reload - Firefox 3.0.x and not Minefield - Random reproduction - Windows The browser begin to display and freeze. It's necessary next to ask to windows to force shutdown of Firefox. I've checkout Firefox source code and build it with debug symbols. The only problem I saw (except CSS problems and dirty code) is the assertion. Why the problem is only visible in final build and not in Minefield ? Simon GAUTHERAT (THALES)

Jeff, _please_ file a separate bug like I asked you to instead of hijacking this one.

Sorry I had not understand your response. I have already a ticket : https://bugzilla.mozilla.org/show_bug.cgi?id=450223 I just want to know if the hang is link to this assertion i search a workaround Sorry again

Olli, the stack of the assertion which is visible with the given testcase is the same as on bug 462076. The last patch which has been attached was a while back. So could we do the work in one bug? Johnny seems to be on vacation right now.

(In reply to comment #22) > Created attachment 331372 [details] > testcase 2 (using embedded SVG) WARNING: Adding child where we already have a child? This may misbehave: 'dyn', file /home/smaug/mozilla/hg/mozilla/docshell/shistory/src/nsSHEntry.cpp, line 641 Unfortunately there is still this.

I think I missed to handle <embed>.

Comment on attachment 487309 [details] [diff] [review] do the same what is done for <object> r=me

http://hg.mozilla.org/mozilla-central/rev/7c3d9919d2f9 http://hg.mozilla.org/mozilla-central/rev/65f41e98f739