591433 - "ASSERTION: SHEntry already contains viewer"

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: SHEntry already contains viewer: '!aViewer || !mContentViewer', file docshell/shistory/src/nsSHEntry.cpp, line 240

###!!! ASSERTION: Uh, mDocument doesn't match the current inner window document!: '!GetCurrentInnerWindow() || GetCurrentInnerWindow()->GetExtantDocument() == mDocument', file dom/base/nsGlobalWindow.cpp, line 1629

Comment 1

[Jesse Ruderman]

Attachment: stacks

Comment 2

[Boris Zbarsky [:bzbarsky]]

Regression from smaug's changes?

Comment 3

[Jesse Ruderman]

Other testcases (similar in appearance and all difficult to reduce) trigger other SHEntry-related assertions, and some of them sound pretty scary.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

I'll look at this.

But other assertions need new bugs.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #2)
> Regression from smaug's changes?

No. This happens on 1.9.2 too.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

We shouldn't try to load anything to a docshell which isn't anymore in the
docshell tree.
The problem here is that the document (or its container) which keeps the iframe
alive is already disconnected from the parent docshell.
But we don't tear down all the child docshells in other way than setting
the treeOwner to null.

The testcase should work as crashtest, which asserts without the patch.

I pushed this to tryserver.

Comment 7

[Jesse Ruderman]

> The testcase should work as crashtest, which asserts without the patch.

On the other hand, the testcase is kind of insane.  What it actually tests is timing-dependent.  Is it possible to make a simpler one, now that you know what's going on?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

I don't see anything "insane" in the testcase.

Comment 9

[Jesse Ruderman]

Well, it's complex and full of timeouts.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 473489 [details] [diff] [review]
patch

r=me

Comment 11

[Johnny Stenback (:jst)]

Not blocking, but please land asap.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/85c59446991a

I'll push still the testcase.