JSDOMWindowShell.cpp: ASSERT(!m_window || domWindow.get() != m_window->impl())

Fix assertion when running debug build of Mac platform DRT.

Created attachment 105387 [details]
Patch

[Darin Adler]

Comment on attachment 105387 [details]
Patch

It makes the assertion go away, but it is correct?

The assertion checks that setWindow() is not called with the same window that's already set.

This patch makes the code not do that. I do not know the context of this code to tell you more.

[Darin Adler]

(In reply to comment #3)
> I do not know the context of this code to tell you more.

Exactly. We need a reviewer who does.

[Geoffrey Garen]

+Alexey, since he added the ASSERT.

[Alexey Proskuryakov]

Comment on attachment 105387 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105387&action=review

I'm confused. Is there something special that one needs to do to hit this assertion? Mac debug build is how I'm running DRT, and I haven't seen it fire.

> Source/WebCore/bindings/js/ScriptController.cpp:193
>          windowShell->window()->willRemoveFromWindowShell();
> -        windowShell->setWindow(m_frame->domWindow());
> +        if (m_frame->domWindow() != windowShell->impl())
> +            windowShell->setWindow(m_frame->domWindow());

This can not be quite correct, because we tell JSDOMWindow that it will be removed from window shell, but don't fulfill the promise.

I would need to see how the assertion fails (and then probably consult with Geoff) to suggest a better approach, but it's likely that we should avoid calling clearWindowShell altogether.

[Alexey Proskuryakov]

> avoid calling clearWindowShell altogether

(in the case that makes the assertion fail).

I encountered this invoking:

DumpRenderTree --pixel-tests /full/path/to/WebKit/LayoutTests/fast/repaint/background-scaling.html

[Alexey Proskuryakov]

I cannot reproduce this. Could you please attach a stack trace?

I've updated my checkout and rebuild and am no longer seeing this. Sorry for the noise.

For the record, the original stack trace was:


ASSERTION FAILED: !m_window || domWindow.get() != m_window->impl() /WebKit/Source/WebCore/bindings/js/JSDOMWindowShell.cpp(62) : void WebCore::JSDOMWindowShell::setWindow(WTF::PassRefPtr<WebCore::DOMWindow>)
1   WebCore::JSDOMWindowShell::setWindow(WTF::PassRefPtr<WebCore::DOMWindow>)
2   WebCore::ScriptController::clearWindowShell(bool)
3   WebCore::Frame::pageDestroyed()
4   WebCore::FrameLoader::detachFromParent()
5   -[WebView(WebPrivate) _close]
6   -[WebView close]
7   testStringByEvaluatingJavaScriptFromString()
8   dumpRenderTree(int, char const**)
9   main
10  start