Closed Bug 608191 Opened 9 years ago Closed 9 years ago

"ASSERTION: EnsureReflowFlushAndPaint() called with no docshell!" with prompt()

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- -

People

(Reporter: jruderman, Assigned: Natch)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file testcase
###!!! ASSERTION: EnsureReflowFlushAndPaint() called with no docshell!: 'mDocShell', file dom/base/nsGlobalWindow.cpp, line 4298
nsGlobalWindow::EnsureReflowFlushAndPaint [dom/base/nsGlobalWindow.cpp:4300]
nsGlobalWindow::Prompt [dom/base/nsGlobalWindow.cpp:4584]
NS_InvokeByIndex_P
...
Uh.... that assert is bogus, no?  Or the callsite needs to be checking for a docshell.
(In reply to comment #2)
> Uh.... that assert is bogus, no?  Or the callsite needs to be checking for a
> docshell.

Yes, it's bogus. See bug 623447. We bail anyhow the same way in ::GetTop, where we anyhow check if there is no docshell. These need to be fixed.
(In reply to comment #3)
> (In reply to comment #2)
> > Uh.... that assert is bogus, no?  Or the callsite needs to be checking for a
> > docshell.
> 
> Yes, it's bogus. See bug 623447. We bail anyhow the same way in ::GetTop, where
> we anyhow check if there is no docshell. These need to be fixed.

Wait, scratch that. I'm not sure I understand this. The docshell goes away after the call to GetTop? Is this still happening?
Ok, so I'm thinking this might just be due to the fact that AreDialogsBlocked isn't forwarded to the outer window (as isn't Prompt). So there's probably a docshell on the outer window but not on the inner. Does that make any sense?
(Oh, and GetTop is forwarded, so there's no check on mDocShell for the inner window).
> The docshell goes away after the call to GetTop?

What call to GetTop?

> So there's probably a docshell on the outer window but not on the inner.

This statement is correct.
(In reply to comment #7)
> > The docshell goes away after the call to GetTop?
> 
> What call to GetTop?

In Prompt there's a call to AreDialogsBlocked. In AreDialogsBlocked there is a call to GetTop, which checks if there is a docshell. If there isn't we return null and AreDialogsBlocked returns true which should cause us to bail with an error from Prompt.

> > So there's probably a docshell on the outer window but not on the inner.
> 
> This statement is correct.

Then that seems to be the problem. EnsureReflowFlushAndPaint is not forwarded. In Alert and Confirm that means it's acting on the outer window since Alert and Confirm are both forwarded. With Prompt that means it's acting on the inner window. It won't get caught in GetTop since GetTop is forwarded.
Also, I can't see how this wouldn't happen on 1.9.2, can someone confirm if it happens there as well?
status2.0: --- → ?
Sounds like we should forward Prompt (and maybe EnsureReflowFlushAndPaint, or just add an assert in it that it's running on the outer window).
Attached patch like so?Splinter Review
Had to add the forwarding to ShowModalDialog too.
Assignee: nobody → highmind63
Attachment #502023 - Flags: review?(bzbarsky)
This assertion was added in bug 115352 when the function was changed from accepting a docshell as a parameter to using mDochShell. I'm not sure if this is a serious assertion in this case, we don't crash in a release build because we check for mDocShell anyhow and return early. However, that means that we don't flush layout before going into modal mode. Again, in this particular case, I don't know if that's bad.

To clarify this, I'm requesting blocking.
blocking2.0: --- → ?
Not doing this particular flush is, iirc, a spoofing vector.
This goes too far back for me, the EnsureReflowFlushAndPaint originated in bug 77002. What kind of spoofing is possible? AFAICT the worry is that some layout values won't be correct (e.g. .width and .height).
Making it look like the prompt is from a different website from the one it's actually from, by posing it during paint suppression.
(In reply to comment #15)
> Making it look like the prompt is from a different website from the one it's
> actually from, by posing it during paint suppression.

I don't see how that can be done with this testcase, however it can be easily done with onunload="prompt()" and the user closing the tab...
Not blocking.
blocking2.0: ? → -
Comment on attachment 502023 [details] [diff] [review]
like so?

Imo, yes.
Attachment #502023 - Flags: review?(bzbarsky) → review+
Attachment #502023 - Flags: approval2.0?
Attachment #502023 - Flags: approval2.0? → approval2.0+
Pushed: http://hg.mozilla.org/mozilla-central/rev/47cbe0ba759c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
status2.0: ? → ---
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.