Bug 30009

Summary: Web Inspector: close inspector client view on InspectorController::close API call.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 26549    
Attachments:
Description Flags
patch
timothy: review-
patch with comments addressed
timothy: review-
patch timothy: review+

Description Pavel Feldman 2009-10-02 02:23:42 PDT
In order to run batch web inspector layout tests (and not affect subsequent tests) we should close inspector client's view upon InspectorController::close API call.
Comment 1 Pavel Feldman 2009-10-02 04:32:03 PDT
Created attachment 40508 [details]
patch
Comment 2 Timothy Hatcher 2009-10-02 11:08:59 PDT
Comment on attachment 40508 [details]
patch


>  Page* WebInspectorClient::createPage()
>  {
> -    if (m_webView)
> -        return core(m_webView.get());
> -
>      ASSERT(!m_hwnd);
>  
>      registerWindowClass();


That !m_hwnd ASSERT will fire the second time.
Comment 3 Pavel Feldman 2009-10-03 11:44:35 PDT
Created attachment 40580 [details]
patch with comments addressed

I still need to test it on non-Mac platforms manually.
Comment 4 Timothy Hatcher 2009-10-03 16:10:24 PDT
Comment on attachment 40580 [details]
patch with comments addressed

I think you will need:

    if (m_hwnd)
        ::DestroyWindow(m_hwnd);


Also:

    if (inspectorView)
        delete inspectorView;
Comment 5 Pavel Feldman 2009-10-05 07:16:30 PDT
Created attachment 40631 [details]
patch

Done destroying the window. Why do you think delete inspectorView is necessary? (I don't find this code in the destructor...)
Comment 6 Timothy Hatcher 2009-10-05 09:09:19 PDT
(In reply to comment #5)
> Created an attachment (id=40631) [details]
> patch
> 
> Done destroying the window. Why do you think delete inspectorView is necessary?
> (I don't find this code in the destructor...)

I missed the fact that it uses a spart pointer.
Comment 7 Pavel Feldman 2009-10-06 08:49:08 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorController.cpp
	M	WebKit/gtk/ChangeLog
	M	WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp
	M	WebKit/mac/ChangeLog
	M	WebKit/mac/WebCoreSupport/WebInspectorClient.mm
	M	WebKit/qt/ChangeLog
	M	WebKit/qt/WebCoreSupport/InspectorClientQt.cpp
	M	WebKit/win/ChangeLog
	M	WebKit/win/WebCoreSupport/WebInspectorClient.cpp
Committed r49190