[webkit-changes] [149121] trunk/Source/WebKit2

Thu, 25 Apr 2013 10:57:20 -0700

Title: [149121] trunk/Source/WebKit2
Revision
149121
Author
[email protected]
Date
2013-04-25 10:59:07 -0700 (Thu, 25 Apr 2013)

Log Message

Thread safety issues in WKCustomProtocol.
<rdar://problem/13247304> and https://bugs.webkit.org/show_bug.cgi?id=115185

Reviewed by Alexey Proskuryakov.

* Shared/Network/CustomProtocols/CustomProtocolManager.h:

* Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:
(WebKit::CustomProtocolManager::addCustomProtocol): Protect m_customProtocolMap with its mutex.
(WebKit::CustomProtocolManager::removeCustomProtocol): Ditto.
(WebKit::CustomProtocolManager::didFailWithError):
(WebKit::CustomProtocolManager::didLoadData):
(WebKit::CustomProtocolManager::didReceiveResponse):
(WebKit::CustomProtocolManager::didFinishLoading):
(WebKit::CustomProtocolManager::protocolForID): Protect m_customProtocolMap with its mutex, and return
  a RetainPtr instead of a raw pointer.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (149120 => 149121)


--- trunk/Source/WebKit2/ChangeLog	2013-04-25 17:48:58 UTC (rev 149120)
+++ trunk/Source/WebKit2/ChangeLog	2013-04-25 17:59:07 UTC (rev 149121)
@@ -1,3 +1,22 @@
+2013-04-25  Brady Eidson  <[email protected]>
+
+        Thread safety issues in WKCustomProtocol.
+        <rdar://problem/13247304> and https://bugs.webkit.org/show_bug.cgi?id=115185
+
+        Reviewed by Alexey Proskuryakov.
+
+        * Shared/Network/CustomProtocols/CustomProtocolManager.h:
+
+        * Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:
+        (WebKit::CustomProtocolManager::addCustomProtocol): Protect m_customProtocolMap with its mutex.
+        (WebKit::CustomProtocolManager::removeCustomProtocol): Ditto.
+        (WebKit::CustomProtocolManager::didFailWithError):
+        (WebKit::CustomProtocolManager::didLoadData):
+        (WebKit::CustomProtocolManager::didReceiveResponse):
+        (WebKit::CustomProtocolManager::didFinishLoading):
+        (WebKit::CustomProtocolManager::protocolForID): Protect m_customProtocolMap with its mutex, and return
+          a RetainPtr instead of a raw pointer.
+
 2013-04-25  Jer Noble  <[email protected]>
 
         Further build fixes: correct two more misnames of WebProcessShim.dyld.

Modified: trunk/Source/WebKit2/Shared/Network/CustomProtocols/CustomProtocolManager.h (149120 => 149121)


--- trunk/Source/WebKit2/Shared/Network/CustomProtocols/CustomProtocolManager.h	2013-04-25 17:48:58 UTC (rev 149120)
+++ trunk/Source/WebKit2/Shared/Network/CustomProtocols/CustomProtocolManager.h	2013-04-25 17:59:07 UTC (rev 149121)
@@ -32,6 +32,7 @@
 #include "NetworkProcessSupplement.h"
 #include "WebProcessSupplement.h"
 #include <wtf/HashSet.h>
+#include <wtf/Threading.h>
 #include <wtf/text/WTFString.h>
 
 #if PLATFORM(MAC)
@@ -96,7 +97,11 @@
 #if PLATFORM(MAC)
     typedef HashMap<uint64_t, RetainPtr<WKCustomProtocol> > CustomProtocolMap;
     CustomProtocolMap m_customProtocolMap;
-    WKCustomProtocol *protocolForID(uint64_t customProtocolID);
+    Mutex m_customProtocolMapMutex;
+    
+    // WKCustomProtocol objects can be removed from the m_customProtocolMap from multiple threads.
+    // We return a RetainPtr here because it is unsafe to return a raw pointer since the object might immediately be destroyed from a different thread.
+    RetainPtr<WKCustomProtocol> protocolForID(uint64_t customProtocolID);
 #endif
 };

Modified: trunk/Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm (149120 => 149121)


--- trunk/Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm	2013-04-25 17:48:58 UTC (rev 149120)
+++ trunk/Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm	2013-04-25 17:59:07 UTC (rev 149121)
@@ -150,12 +150,14 @@
 void CustomProtocolManager::addCustomProtocol(WKCustomProtocol *customProtocol)
 {
     ASSERT(customProtocol);
+    MutexLocker locker(m_customProtocolMapMutex);
     m_customProtocolMap.add(customProtocol.customProtocolID, customProtocol);
 }
 
 void CustomProtocolManager::removeCustomProtocol(WKCustomProtocol *customProtocol)
 {
     ASSERT(customProtocol);
+    MutexLocker locker(m_customProtocolMapMutex);
     m_customProtocolMap.remove(customProtocol.customProtocolID);
 }
     
@@ -176,50 +178,52 @@
 
 void CustomProtocolManager::didFailWithError(uint64_t customProtocolID, const WebCore::ResourceError& error)
 {
-    WKCustomProtocol *protocol = protocolForID(customProtocolID);
+    RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     if (!protocol)
         return;
-    
-    [[protocol client] URLProtocol:protocol didFailWithError:error.nsError()];
-    removeCustomProtocol(protocol);
+
+    [[protocol.get() client] URLProtocol:protocol.get() didFailWithError:error.nsError()];
+    removeCustomProtocol(protocol.get());
 }
 
 void CustomProtocolManager::didLoadData(uint64_t customProtocolID, const CoreIPC::DataReference& data)
 {
-    WKCustomProtocol *protocol = protocolForID(customProtocolID);
+    RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     if (!protocol)
         return;
     
-    [[protocol client] URLProtocol:protocol didLoadData:[NSData dataWithBytes:(void*)data.data() length:data.size()]];
+    [[protocol.get() client] URLProtocol:protocol.get() didLoadData:[NSData dataWithBytes:(void*)data.data() length:data.size()]];
 }
 
 void CustomProtocolManager::didReceiveResponse(uint64_t customProtocolID, const WebCore::ResourceResponse& response, uint32_t cacheStoragePolicy)
 {
-    WKCustomProtocol *protocol = protocolForID(customProtocolID);
+    RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     if (!protocol)
         return;
     
-    [[protocol client] URLProtocol:protocol didReceiveResponse:response.nsURLResponse() cacheStoragePolicy:static_cast<NSURLCacheStoragePolicy>(cacheStoragePolicy)];
+    [[protocol.get() client] URLProtocol:protocol.get() didReceiveResponse:response.nsURLResponse() cacheStoragePolicy:static_cast<NSURLCacheStoragePolicy>(cacheStoragePolicy)];
 }
 
 void CustomProtocolManager::didFinishLoading(uint64_t customProtocolID)
 {
-    WKCustomProtocol *protocol = protocolForID(customProtocolID);
+    RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     if (!protocol)
         return;
     
-    [[protocol client] URLProtocolDidFinishLoading:protocol];
-    removeCustomProtocol(protocol);
+    [[protocol.get() client] URLProtocolDidFinishLoading:protocol.get()];
+    removeCustomProtocol(protocol.get());
 }
 
-WKCustomProtocol *CustomProtocolManager::protocolForID(uint64_t customProtocolID)
+RetainPtr<WKCustomProtocol> CustomProtocolManager::protocolForID(uint64_t customProtocolID)
 {
+    MutexLocker locker(m_customProtocolMapMutex);
+
     CustomProtocolMap::const_iterator it = m_customProtocolMap.find(customProtocolID);
     if (it == m_customProtocolMap.end())
         return nil;
     
     ASSERT(it->value);
-    return it->value.get();
+    return it->value;
 }
 
 } // namespace WebKit

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to