Bug 30009 - Web Inspector: close inspector client view on InspectorController::close API call.
Summary: Web Inspector: close inspector client view on InspectorController::close API ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks: 26549
  Show dependency treegraph
 
Reported: 2009-10-02 02:23 PDT by Pavel Feldman
Modified: 2009-10-06 08:49 PDT (History)
1 user (show)

See Also:


Attachments
patch (7.49 KB, patch)
2009-10-02 04:32 PDT, Pavel Feldman
timothy: review-
Details | Formatted Diff | Diff
patch with comments addressed (7.42 KB, patch)
2009-10-03 11:44 PDT, Pavel Feldman
timothy: review-
Details | Formatted Diff | Diff
patch (7.52 KB, patch)
2009-10-05 07:16 PDT, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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