Bug 32827

Summary: Crash when calling IWebView::close, then releasing the WebView, without calling DestroyWindow
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: WebKit APIAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: sfalken, webkit.review.bot
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
Patch
none
Make it safe to call IWebView::close when IWebView::initWithFrame hasn't been called
sfalken: review+
Make IWebView::close and destroying a WebView's HWND optional for WebKit clients sfalken: review+

Adam Roben (:aroben)
Reported 2009-12-21 10:15:36 PST
Created attachment 45337 [details] Patch WebKit clients can currently cleanly get rid of a WebView in two ways. A) 1) DestroyWindow(webViewHWND) 2) webView->Release() B) 1) webView->close() 2) DestroyWindow(webViewHWND) (can be swapped with (1)) 3) webView->Release() We'd like clients to be able to get rid of a WebView just by releasing the last reference to it. This patch gets us a little closer to that by removing step B2 above (though calling DestroyWindow in this case is harmless). A future patch will make steps A1 and B1 unnecessary, as well. Fixes <rdar://problem/7374218> Crash in WebView::updateActiveState when closing "Welcome to iTunes" window Reviewed by NOBODY (OOPS!). * WebView.cpp: (WebView::~WebView): Call setIsBeingDestroyed() so that we won't be ref'd by our WndProc, which would result in this destructor being re-entered. (WebView::close): Moved the call to revokeDragDrop here... (WebView::WebViewWndProc): ...from here. This is important in order to release the reference that OLE holds while we're registered as a drop target. Otherwise, clients that call IWebView::close but not DestroyWindow would leak the WebView. * WebView.h: (WebView::setIsBeingDestroyed): (WebView::isBeingDestroyed): Made these private, and added a comment about what isBeingDestroyed() now means. --- 3 files changed, 52 insertions(+), 5 deletions(-)
Attachments
Patch (4.38 KB, patch)
2009-12-21 10:15 PST, Adam Roben (:aroben)
no flags
Make it safe to call IWebView::close when IWebView::initWithFrame hasn't been called (4.44 KB, patch)
2010-01-05 11:04 PST, Adam Roben (:aroben)
sfalken: review+
Make IWebView::close and destroying a WebView's HWND optional for WebKit clients (5.97 KB, patch)
2010-01-05 12:02 PST, Adam Roben (:aroben)
sfalken: review+
Adam Roben (:aroben)
Comment 1 2009-12-21 10:17:34 PST
WebKit Review Bot
Comment 2 2009-12-21 10:19:06 PST
style-queue ran check-webkit-style on attachment 45337 [details] without any errors.
Adam Roben (:aroben)
Comment 3 2009-12-22 11:27:20 PST
Comment on attachment 45337 [details] Patch I'm going to try tackling this a different way.
Adam Roben (:aroben)
Comment 4 2010-01-05 11:04:57 PST
Created attachment 45912 [details] Make it safe to call IWebView::close when IWebView::initWithFrame hasn't been called
WebKit Review Bot
Comment 5 2010-01-05 11:10:02 PST
style-queue ran check-webkit-style on attachment 45912 [details] without any errors.
Adam Roben (:aroben)
Comment 6 2010-01-05 12:02:30 PST
Created attachment 45916 [details] Make IWebView::close and destroying a WebView's HWND optional for WebKit clients
Eric Seidel (no email)
Comment 7 2010-01-05 13:56:52 PST
Comment on attachment 45912 [details] Make it safe to call IWebView::close when IWebView::initWithFrame hasn't been called Looks sane to me too.
Adam Roben (:aroben)
Comment 8 2010-01-05 15:10:08 PST
Adam Roben (:aroben)
Comment 9 2010-01-05 15:10:31 PST
Note You need to log in before you can comment on or make changes to this bug.