Bug 53798

Summary: Assertion failure in WebInspectorProxy::platformClose closing main window when inspecting a popup window, or when running regression tests
Product: WebKit Reporter: Brian Weinstein <bweinstein>
Component: WebKit2Assignee: Brian Weinstein <bweinstein>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
[PATCH] Fix aroben: review+

Description Brian Weinstein 2011-02-04 12:22:31 PST
Assertion failure in WebInspectorProxy::platformClose closing main window when inspecting a popup window, or when running regression tests.


>    WebKit.dll!WebKit::WebInspectorProxy::platformClose()  Line 174 + 0x25 bytes    C++
    WebKit.dll!WebKit::WebInspectorProxy::invalidate()  Line 74    C++
    WebKit.dll!WebKit::WebPageProxy::close()  Line 240    C++
    WebKit.dll!WKPageClose(const OpaqueWKPage * pageRef=0x061f0f58)  Line 103    C++

ASSERT(m_inspectorWindow) is the failing assertion.

<rdar://problem/8814364>
Comment 1 Brian Weinstein 2011-02-04 12:26:51 PST
Created attachment 81264 [details]
[PATCH] Fix
Comment 2 Adam Roben (:aroben) 2011-02-04 12:28:36 PST
Comment on attachment 81264 [details]
[PATCH] Fix

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

> Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:178
> +    if (m_isVisible) {
> +        ASSERT(m_inspectorWindow);
> +        ASSERT(m_inspectorView);
> +    }

To get rid of the if in Release builds, you could do:

ASSERT(!m_isVisible || m_inspectorWindow);
ASSERT(!m_isVisible || m_inspectorView);

> Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:181
> +        ASSERT(::IsWindow(m_inspectorWindow));

Have you verified that this is true even when the inspector is docked?
Comment 3 Brian Weinstein 2011-02-04 12:31:52 PST
(In reply to comment #2)
> (From update of attachment 81264 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=81264&action=review
> 
> > Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:178
> > +    if (m_isVisible) {
> > +        ASSERT(m_inspectorWindow);
> > +        ASSERT(m_inspectorView);
> > +    }
> 
> To get rid of the if in Release builds, you could do:
> 
> ASSERT(!m_isVisible || m_inspectorWindow);
> ASSERT(!m_isVisible || m_inspectorView);

Fixed.

> 
> > Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:181
> > +        ASSERT(::IsWindow(m_inspectorWindow));
> 
> Have you verified that this is true even when the inspector is docked?

We don't support a docked inspector in WebKit2 yet, but I will make sure to check this when we do.

Thanks!
Comment 4 Brian Weinstein 2011-02-04 12:33:38 PST
Landed in r77655.