1260531 - nsSubDocumentFrame::Init call nsFrameLoader::Hide which leads to presentation destruction

Status: RESOLVED INVALID

(Core :: Layout, defect)

Description

[Timothy Nikkel (:tnikkel)]

Which is probably a bad idea during frame construction. nsSubDocumentFrame::Init also accesses |this| after the call, which could be deleted. And we should clear the detached views before exiting this function.

Fix is to move the hide call to be done async like the show call for the new frame loader.

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Comment 2

[Timothy Nikkel (:tnikkel)]

Found this while looking for potential causes of bug 724355 and bug 1259572. So maybe (hopefully?) this helps those bugs.

Comment 3

[Mats Palmgren (inactive)]

Could you CC me on bug 724355?

Comment 4

[Timothy Nikkel (:tnikkel)]

CC'd you on bug 724355 and bug 1259572.

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to Timothy Nikkel (:tnikkel) from comment #0)
> Which is probably a bad idea during frame construction.
> nsSubDocumentFrame::Init also accesses |this| after the call, which could be
> deleted. And we should clear the detached views before exiting this function.

"before exiting" -> "before calling an external function which can do lots of things"

Comment 6

[Mats Palmgren (inactive)]

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

Bug 724355 seems unrelated to this bug (it involves nsObjectFrame, not nsSubDocumentFrame).
Bug 1259572 might be related although I'm not convinced it has anything to do with Init.

(In reply to Timothy Nikkel (:tnikkel) from comment #0)
> Which is probably a bad idea during frame construction.

Why?  Scripts are blocked during frame construction.

> nsSubDocumentFrame::Init also accesses |this| after the call, which could be
> deleted. And we should clear the detached views before exiting this function.

Destroying a sub-shell and its frames/views should be completely safe from
the parent shell's (and its frames/views) POV.

I'm inclined to leave this code as is since it seems safer to me to destroy
the old shell/frames/views as soon as possible when we know we wont use them.

If you have any particular error-scenario in mind for bug 1259572, perhaps
we could add assertions/annotations to verify it first?

Comment 7

[Timothy Nikkel (:tnikkel)]

(In reply to Mats Palmgren (:mats) from comment #6)
> Bug 724355 seems unrelated to this bug (it involves nsObjectFrame, not
> nsSubDocumentFrame).

They hijacked the bug lower down in the comments to be about a crash in GetRootPresContext.

> 
> (In reply to Timothy Nikkel (:tnikkel) from comment #0)
> > Which is probably a bad idea during frame construction.
> 
> Why?  Scripts are blocked during frame construction.
> 
> > nsSubDocumentFrame::Init also accesses |this| after the call, which could be
> > deleted. And we should clear the detached views before exiting this function.
> 
> Destroying a sub-shell and its frames/views should be completely safe from
> the parent shell's (and its frames/views) POV.
> 
> I'm inclined to leave this code as is since it seems safer to me to destroy
> the old shell/frames/views as soon as possible when we know we wont use them.
> 
> If you have any particular error-scenario in mind for bug 1259572, perhaps
> we could add assertions/annotations to verify it first?

Nothing specific.