[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
aroben at apple.com
aroben at apple.com
Thu Apr 8 00:55:07 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 317b26872ebb9bd473fab32c21e97ef135b219d0
Author: aroben at apple.com <aroben at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Jan 5 23:07:33 2010 +0000
Make IWebView::close and destroying a WebView's HWND optional for WebKit clients
WebView will now take care of these operations itself when its last
reference is released, if they haven't already been done.
IWebView::close now also destroys the WebView's HWND. All WebKit
clients were already performing these operations in succession anyway,
or were attempting to by calling IWebView::close then destroying the
WebView's host window (which actually resulted in the WebView's HWND
leaking, and the crash in the below bug).
Fixes <http://webkit.org/b/32827> Crash when calling IWebView::close,
then releasing the WebView, without calling DestroyWindow
Fixes a few WebViewDestruction tests, too.
Reviewed by Steve Falkenburg.
* WebView.cpp:
(WebView::~WebView): Don't try to destroy m_viewWindow here. That
should already have happened. Assert that this is the case.
(WebView::close): If m_viewWindow isn't already being destroyed,
destroy it now. Moved the call to revokeDragDrop() here from our
WM_DESTROY handler because it needs to be done before m_viewWindow is
nulled out.
(WebView::WebViewWndProc): Removed call to revokeDragDrop() that
close() now performs.
(WebView::Release): If our last reference is being released, call
close() so that clients don't have to. (It's harmless to call close()
multiple times.) We do this here instead of in the destructor because
close() can cause AddRef() and Release() to be called, and calling
those from within the destructor leads to double-destruction.
(WebView::setHostWindow): Removed an unnecessary (and now harmful)
null-check.
(WebView::revokeDragDrop): Changed an assertion into a run-time check,
since this will now sometimes be called when m_viewWindow hasn't been
created yet. Changed the IsWindow call to a null-check because we
never hold onto a destroyed m_viewWindow.
(WebView::windowAncestryDidChange): If we don't have a view window,
stop tracking changes to our parent's active state.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52830 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKit/win/ChangeLog b/WebKit/win/ChangeLog
index 67a3ff6..ac88cf2 100644
--- a/WebKit/win/ChangeLog
+++ b/WebKit/win/ChangeLog
@@ -1,5 +1,49 @@
2010-01-05 Adam Roben <aroben at apple.com>
+ Make IWebView::close and destroying a WebView's HWND optional for
+ WebKit clients
+
+ WebView will now take care of these operations itself when its last
+ reference is released, if they haven't already been done.
+
+ IWebView::close now also destroys the WebView's HWND. All WebKit
+ clients were already performing these operations in succession anyway,
+ or were attempting to by calling IWebView::close then destroying the
+ WebView's host window (which actually resulted in the WebView's HWND
+ leaking, and the crash in the below bug).
+
+ Fixes <http://webkit.org/b/32827> Crash when calling IWebView::close,
+ then releasing the WebView, without calling DestroyWindow
+
+ Fixes a few WebViewDestruction tests, too.
+
+ Reviewed by Steve Falkenburg.
+
+ * WebView.cpp:
+ (WebView::~WebView): Don't try to destroy m_viewWindow here. That
+ should already have happened. Assert that this is the case.
+ (WebView::close): If m_viewWindow isn't already being destroyed,
+ destroy it now. Moved the call to revokeDragDrop() here from our
+ WM_DESTROY handler because it needs to be done before m_viewWindow is
+ nulled out.
+ (WebView::WebViewWndProc): Removed call to revokeDragDrop() that
+ close() now performs.
+ (WebView::Release): If our last reference is being released, call
+ close() so that clients don't have to. (It's harmless to call close()
+ multiple times.) We do this here instead of in the destructor because
+ close() can cause AddRef() and Release() to be called, and calling
+ those from within the destructor leads to double-destruction.
+ (WebView::setHostWindow): Removed an unnecessary (and now harmful)
+ null-check.
+ (WebView::revokeDragDrop): Changed an assertion into a run-time check,
+ since this will now sometimes be called when m_viewWindow hasn't been
+ created yet. Changed the IsWindow call to a null-check because we
+ never hold onto a destroyed m_viewWindow.
+ (WebView::windowAncestryDidChange): If we don't have a view window,
+ stop tracking changes to our parent's active state.
+
+2010-01-05 Adam Roben <aroben at apple.com>
+
Make it safe to call IWebView::close when IWebView::initWithFrame
hasn't been called
diff --git a/WebKit/win/WebView.cpp b/WebKit/win/WebView.cpp
index f1f2d4e..da6772a 100644
--- a/WebKit/win/WebView.cpp
+++ b/WebKit/win/WebView.cpp
@@ -354,18 +354,13 @@ WebView::~WebView()
{
deleteBackingStore();
- // <rdar://4958382> m_viewWindow will be destroyed when m_hostWindow is destroyed, but if
- // setHostWindow was never called we will leak our HWND. If we still have a valid HWND at
- // this point, we should just destroy it ourselves.
- if (!isBeingDestroyed() && ::IsWindow(m_viewWindow))
- ::DestroyWindow(m_viewWindow);
-
// the tooltip window needs to be explicitly destroyed since it isn't a WS_CHILD
if (::IsWindow(m_toolTipHwnd))
::DestroyWindow(m_toolTipHwnd);
ASSERT(!m_page);
ASSERT(!m_preferences);
+ ASSERT(!m_viewWindow);
WebViewCount--;
gClassCount--;
@@ -646,6 +641,18 @@ HRESULT STDMETHODCALLTYPE WebView::close()
m_mouseOutTracker.set(0);
}
+ revokeDragDrop();
+
+ if (m_viewWindow) {
+ // We can't check IsWindow(m_viewWindow) here, because that will return true even while
+ // we're already handling WM_DESTROY. So we check !isBeingDestroyed() instead.
+ if (!isBeingDestroyed())
+ DestroyWindow(m_viewWindow);
+ // Either we just destroyed m_viewWindow, or it's in the process of being destroyed. Either
+ // way, we clear it out to make sure we don't try to use it later.
+ m_viewWindow = 0;
+ }
+
setHostWindow(0);
setDownloadDelegate(0);
@@ -1910,7 +1917,6 @@ LRESULT CALLBACK WebView::WebViewWndProc(HWND hWnd, UINT message, WPARAM wParam,
case WM_DESTROY:
webView->setIsBeingDestroyed();
webView->close();
- webView->revokeDragDrop();
break;
case WM_GESTURENOTIFY:
handled = webView->gestureNotify(wParam, lParam);
@@ -2280,6 +2286,14 @@ ULONG STDMETHODCALLTYPE WebView::Release(void)
{
ASSERT(!m_deletionHasBegun);
+ if (m_refCount == 1) {
+ // Call close() now so that clients don't have to. (It's harmless to call close() multiple
+ // times.) We do this here instead of in our destructor because close() can cause AddRef()
+ // and Release() to be called, and if that happened in our destructor we would be destroyed
+ // more than once.
+ close();
+ }
+
ULONG newRef = --m_refCount;
if (!newRef) {
#if !ASSERT_DISABLED
@@ -3060,8 +3074,7 @@ HRESULT STDMETHODCALLTYPE WebView::setHostWindow(
m_hostWindow = window;
- if (m_viewWindow)
- windowAncestryDidChange();
+ windowAncestryDidChange();
return S_OK;
}
@@ -4941,7 +4954,9 @@ HRESULT WebView::registerDragDrop()
HRESULT WebView::revokeDragDrop()
{
- ASSERT(::IsWindow(m_viewWindow));
+ if (!m_viewWindow)
+ return S_OK;
+
return ::RevokeDragDrop(m_viewWindow);
}
@@ -5369,7 +5384,15 @@ HRESULT STDMETHODCALLTYPE WebView::inspector(IWebInspector** inspector)
HRESULT STDMETHODCALLTYPE WebView::windowAncestryDidChange()
{
- HWND newParent = findTopLevelParent(m_hostWindow);
+ HWND newParent;
+ if (m_viewWindow)
+ newParent = findTopLevelParent(m_hostWindow);
+ else {
+ // There's no point in tracking active state changes of our parent window if we don't have
+ // a window ourselves.
+ newParent = 0;
+ }
+
if (newParent == m_topLevelParent)
return S_OK;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list