Changeset 52829 in webkit

Timestamp:

Jan 5, 2010 3:07:21 PM (14 years ago)

Author:

Adam Roben

Message:

Make it safe to call IWebView::close when IWebView::initWithFrame hasn't been called

Part of <​http://webkit.org/b/32827> Crash when calling
IWebView::close, then releasing the WebView, without calling
DestroyWindow

Reviewed by Steve Falkenburg.

WebKit/win:

• WebView.cpp:

(WebView::close): Null-check m_page and m_preferences before using
them. They will be null if initWithFrame was never called.

WebKitTools:

Test that it's safe to call IWebView::close when IWebView::initWithFrame
hasn't been called

• WebKitAPITest/tests/WebViewDestruction.cpp:

(WebKitAPITest::CloseWithoutInitWithFrame): Calls IWebView::close
without ever calling IWebView::initWithFrame and tests that we don't
crash or leak.

Location:

trunk

Files:

• WebKit/win/ChangeLog (modified) (1 diff)
• WebKit/win/WebView.cpp (modified) (2 diffs)
• WebKitTools/ChangeLog (modified) (1 diff)
• WebKitTools/WebKitAPITest/tests/WebViewDestruction.cpp (modified) (1 diff)

trunk/WebKit/win/ChangeLog

@@ +1 @@
+2010-01-05  Adam Roben  <aroben@apple.com>
+
+        Make it safe to call IWebView::close when IWebView::initWithFrame
+        hasn't been called
+
+        Part of <http://webkit.org/b/32827> Crash when calling
+        IWebView::close, then releasing the WebView, without calling
+        DestroyWindow
+
+        Reviewed by Steve Falkenburg.
+
+        * WebView.cpp:
+        (WebView::close): Null-check m_page and m_preferences before using
+        them. They will be null if initWithFrame was never called.
+
 2010-01-05  Adam Roben  <aroben@apple.com>
 

trunk/WebKit/win/WebView.cpp

@@ -636 +636 @@
     removeFromAllWebViewsSet();
 
-    Frame* frame = m_page->mainFrame();
-    if (frame)
-        frame->loader()->detachFromParent();
+    if (m_page) {
+        if (Frame* frame = m_page->mainFrame())
+            frame->loader()->detachFromParent();
+    }
 
     if (m_mouseOutTracker) {
@@ -669 +670 @@
     notifyCenter->removeObserver(this, WebPreferences::webPreferencesChangedNotification(), static_cast<IWebPreferences*>(m_preferences.get()));
 
-    BSTR identifier = 0;
-    m_preferences->identifier(&identifier);
-
-    COMPtr<WebPreferences> preferences = m_preferences;
-    m_preferences = 0;
-    preferences->didRemoveFromWebView();
-    // Make sure we release the reference, since WebPreferences::removeReferenceForIdentifier will check for last reference to WebPreferences
-    preferences = 0;
-    if (identifier) {
-        WebPreferences::removeReferenceForIdentifier(identifier);
-        SysFreeString(identifier);
+    if (COMPtr<WebPreferences> preferences = m_preferences) {
+        BSTR identifier = 0;
+        preferences->identifier(&identifier);
+
+        m_preferences = 0;
+        preferences->didRemoveFromWebView();
+        // Make sure we release the reference, since WebPreferences::removeReferenceForIdentifier will check for last reference to WebPreferences
+        preferences = 0;
+        if (identifier) {
+            WebPreferences::removeReferenceForIdentifier(identifier);
+            SysFreeString(identifier);
+        }
     }
 

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-01-05  Adam Roben  <aroben@apple.com>
+
+        Test that it's safe to call IWebView::close when
+        IWebView::initWithFrame hasn't been called
+
+        Part of <http://webkit.org/b/32827> Crash when calling
+        IWebView::close, then releasing the WebView, without calling
+        DestroyWindow
+
+        Reviewed by Steve Falkenburg.
+
+        * WebKitAPITest/tests/WebViewDestruction.cpp:
+        (WebKitAPITest::CloseWithoutInitWithFrame): Calls IWebView::close
+        without ever calling IWebView::initWithFrame and tests that we don't
+        crash or leak.
+
 2010-01-05  Adam Roben  <aroben@apple.com>
 

trunk/WebKitTools/WebKitAPITest/tests/WebViewDestruction.cpp

@@ -103 +103 @@
 }
 
+TEST(WebViewDestruction, CloseWithoutInitWithFrame)
+{
+    COMPtr<IWebView> webView;
+    TEST_ASSERT(SUCCEEDED(WebKitCreateInstance(__uuidof(WebView), &webView)));
+
+    TEST_ASSERT(SUCCEEDED(webView->close()));
+
+    finishWebViewDestructionTest(webView, 0);
+}
+
 // Tests that releasing a WebView without calling IWebView::close or DestroyWindow doesn't leak. <http://webkit.org/b/33162>
 TEST(WebViewDestruction, NoCloseOrDestroyViewWindow)