Changeset 51510 in webkit

Timestamp:

Nov 30, 2009 1:41:03 PM (14 years ago)

Author:

Adam Roben

Message:

Fix double-free of BSTRs passed to WebNavigationData::createInstance

WebFrameLoaderClient::updateGlobalHistory was converting
WebCore::Strings to WebCore::BStrings, then passing them to
WebNavigationData::createInstance. But the latter function takes BSTR
parameters and adopts them into WebCore::BStrings. So the end result
was that two WebCore::BStrings would end up freeing each underlying
BSTR.

The fix is to only convert to WebCore::BString inside
WebNavigationData.

Fixes <​http://webkit.org/b/31998> <rdar://problem/7383452> REGRESSION
(r49564): Crash in updateGlobalHistory when running Javascript iBench
test

I couldn't find a way to reproduce this in DumpRenderTree.

Reviewed by Steve Falkenburg.

• WebCoreSupport/WebFrameLoaderClient.cpp:

(WebFrameLoaderClient::updateGlobalHistory): Pass WebCore::Strings to
WebNavigationData::createInstance.

• WebNavigationData.cpp:

(WebNavigationData::WebNavigationData):
(WebNavigationData::createInstance):

• WebNavigationData.h:

Changed to take const WebCore::String&s instead of BSTRs and to
convert the Strings to BStrings at this level.

Location:

trunk/WebKit/win

Files:

• ChangeLog (modified) (1 diff)
• WebCoreSupport/WebFrameLoaderClient.cpp (modified) (1 diff)
• WebNavigationData.cpp (modified) (3 diffs)
• WebNavigationData.h (modified) (1 diff)

trunk/WebKit/win/ChangeLog

@@ +1 @@
+2009-11-30  Adam Roben  <aroben@apple.com>
+
+        Fix double-free of BSTRs passed to WebNavigationData::createInstance
+
+        WebFrameLoaderClient::updateGlobalHistory was converting
+        WebCore::Strings to WebCore::BStrings, then passing them to
+        WebNavigationData::createInstance. But the latter function takes BSTR
+        parameters and adopts them into WebCore::BStrings. So the end result
+        was that two WebCore::BStrings would end up freeing each underlying
+        BSTR.
+
+        The fix is to only convert to WebCore::BString inside
+        WebNavigationData.
+
+        Fixes <http://webkit.org/b/31998> <rdar://problem/7383452> REGRESSION
+        (r49564): Crash in updateGlobalHistory when running Javascript iBench
+        test
+
+        I couldn't find a way to reproduce this in DumpRenderTree.
+
+        Reviewed by Steve Falkenburg.
+
+        * WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebFrameLoaderClient::updateGlobalHistory): Pass WebCore::Strings to
+        WebNavigationData::createInstance.
+
+        * WebNavigationData.cpp:
+        (WebNavigationData::WebNavigationData):
+        (WebNavigationData::createInstance):
+        * WebNavigationData.h:
+        Changed to take const WebCore::String&s instead of BSTRs and to
+        convert the Strings to BStrings at this level.
+
 2009-11-30  Steve Falkenburg  <sfalken@apple.com>
 

trunk/WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp

@@ -501 +501 @@
 
     if (historyDelegate) {
-        BString url(loader->urlForHistory());
-        BString title(loader->title());
-        BString redirectSource(loader->clientRedirectSourceForHistory());
         COMPtr<IWebURLResponse> urlResponse(AdoptCOM, WebURLResponse::createInstance(loader->response()));
         COMPtr<IWebURLRequest> urlRequest(AdoptCOM, WebMutableURLRequest::createInstance(loader->originalRequestCopy()));
         
         COMPtr<IWebNavigationData> navigationData(AdoptCOM, WebNavigationData::createInstance(
-            url, title, urlRequest.get(), urlResponse.get(), loader->substituteData().isValid(), redirectSource));
+            loader->urlForHistory(), loader->title(), urlRequest.get(), urlResponse.get(), loader->substituteData().isValid(), loader->clientRedirectSourceForHistory()));
 
         historyDelegate->didNavigateWithNavigationData(webView, navigationData.get(), m_webFrame);

trunk/WebKit/win/WebNavigationData.cpp

@@ -28 +28 @@
 #include "WebNavigationData.h"
 
-#include <WebCore/BString.h>
-using WebCore::BString;
+using namespace WebCore;
 
 // IUnknown -------------------------------------------------------------------
@@ -63 +62 @@
 // WebNavigationData -------------------------------------------------------------------
 
-WebNavigationData::WebNavigationData(BSTR url, BSTR title, IWebURLRequest* request, IWebURLResponse* response, bool hasSubstituteData, BSTR clientRedirectSource)
+WebNavigationData::WebNavigationData(const String& url, const String& title, IWebURLRequest* request, IWebURLResponse* response, bool hasSubstituteData, const String& clientRedirectSource)
     : m_refCount(0)
+    , m_url(url)
+    , m_title(title)
     , m_request(request)
     , m_response(response)
     , m_hasSubstituteData(hasSubstituteData)
+    , m_clientRedirectSource(clientRedirectSource)
 
 {
     gClassCount++;
     gClassNameCount.add("WebNavigationData");
-
-    m_url.adoptBSTR(url);
-    m_title.adoptBSTR(title);
-    m_clientRedirectSource.adoptBSTR(clientRedirectSource);
 }
 
@@ -84 +82 @@
 }
 
-WebNavigationData* WebNavigationData::createInstance(BSTR url, BSTR title, IWebURLRequest* request, IWebURLResponse* response, bool hasSubstituteData, BSTR clientRedirectSource)
+WebNavigationData* WebNavigationData::createInstance(const String& url, const String& title, IWebURLRequest* request, IWebURLResponse* response, bool hasSubstituteData, const String& clientRedirectSource)
 {
     WebNavigationData* instance = new WebNavigationData(url, title, request, response, hasSubstituteData, clientRedirectSource);

trunk/WebKit/win/WebNavigationData.h

@@ -34 +34 @@
 class WebNavigationData : public IWebNavigationData {
 public:
-    static WebNavigationData* createInstance(BSTR, BSTR, IWebURLRequest*, IWebURLResponse*, bool, BSTR);
+    static WebNavigationData* createInstance(const WebCore::String& url, const WebCore::String& title, IWebURLRequest*, IWebURLResponse*, bool hasSubstituteData, const WebCore::String& clientRedirectSource);
 private:
-    WebNavigationData(BSTR url, BSTR title, IWebURLRequest*, IWebURLResponse*, bool hasSubstituteData, BSTR clientRedirectSource);
+    WebNavigationData(const WebCore::String& url, const WebCore::String& title, IWebURLRequest*, IWebURLResponse*, bool hasSubstituteData, const WebCore::String& clientRedirectSource);
     ~WebNavigationData();