Fix some leaks found by the leak bot
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 08:27:03 +0000 (08:27 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 08:27:03 +0000 (08:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141149

Reviewed by Alexey Proskuryakov.

* bindings/js/JSSubtleCryptoCustom.cpp:
(WebCore::importKey): Changed argument types to std::unique_ptr for better code clarity.
(WebCore::JSSubtleCrypto::importKey): Use WTF::move instead of release.
(WebCore::JSSubtleCrypto::wrapKey): Fixed leaks by adding missing delete calls to the
case where we get a DOM exception.
(WebCore::JSSubtleCrypto::unwrapKey): Ditto.

* dom/SelectorQuery.cpp:
(WebCore::SelectorQuery::SelectorQuery): Use WTF::move here. Not clear how this could
have caused the storage leak, but it does seem obviously missing. The leak is pretty big,
implying that we leak almost all CSSSelectorList objects we parse; not sure this fixes it.

* loader/WorkerThreadableLoader.cpp:
(WebCore::WorkerThreadableLoader::MainThreadBridge::didReceiveResponse): Added code to
deleted the unguarded pointer if postTaskForModeToWorkerGlobalScope fails.
(WebCore::WorkerThreadableLoader::MainThreadBridge::didReceiveData): Ditto.
(WebCore::WorkerThreadableLoader::MainThreadBridge::didFail): Ditto.
(WebCore::WorkerThreadableLoader::MainThreadBridge::didFailAccessControlCheck): Ditto.

* platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm:
(WebCore::MediaSelectionGroupAVFObjC::updateOptions): Added missing adoptNS.

* platform/graphics/mac/GraphicsContextMac.mm:
(WebCore::GraphicsContext::updateDocumentMarkerResources): Added missing release.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179473 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp
Source/WebCore/dom/SelectorQuery.cpp
Source/WebCore/loader/WorkerThreadableLoader.cpp
Source/WebCore/platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm
Source/WebCore/platform/graphics/mac/GraphicsContextMac.mm

index 0c389bcbb50fa9e5599ffea18dfaed7af998d135..6db065bee7105d9b44e805d20bec7d81012e7bec 100644 (file)
@@ -1,3 +1,35 @@
+2015-02-02  Darin Adler  <darin@apple.com>
+
+        Fix some leaks found by the leak bot
+        https://bugs.webkit.org/show_bug.cgi?id=141149
+
+        Reviewed by Alexey Proskuryakov.
+
+        * bindings/js/JSSubtleCryptoCustom.cpp:
+        (WebCore::importKey): Changed argument types to std::unique_ptr for better code clarity.
+        (WebCore::JSSubtleCrypto::importKey): Use WTF::move instead of release.
+        (WebCore::JSSubtleCrypto::wrapKey): Fixed leaks by adding missing delete calls to the
+        case where we get a DOM exception.
+        (WebCore::JSSubtleCrypto::unwrapKey): Ditto.
+
+        * dom/SelectorQuery.cpp:
+        (WebCore::SelectorQuery::SelectorQuery): Use WTF::move here. Not clear how this could
+        have caused the storage leak, but it does seem obviously missing. The leak is pretty big,
+        implying that we leak almost all CSSSelectorList objects we parse; not sure this fixes it.
+
+        * loader/WorkerThreadableLoader.cpp:
+        (WebCore::WorkerThreadableLoader::MainThreadBridge::didReceiveResponse): Added code to
+        deleted the unguarded pointer if postTaskForModeToWorkerGlobalScope fails.
+        (WebCore::WorkerThreadableLoader::MainThreadBridge::didReceiveData): Ditto.
+        (WebCore::WorkerThreadableLoader::MainThreadBridge::didFail): Ditto.
+        (WebCore::WorkerThreadableLoader::MainThreadBridge::didFailAccessControlCheck): Ditto.
+
+        * platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm:
+        (WebCore::MediaSelectionGroupAVFObjC::updateOptions): Added missing adoptNS.
+
+        * platform/graphics/mac/GraphicsContextMac.mm:
+        (WebCore::GraphicsContext::updateDocumentMarkerResources): Added missing release.
+
 2015-02-01  Chris Dumez  <cdumez@apple.com>
 
         Use more references in HistoryItem
index 6baefcd6ebd1945169ba4ad2a3afba21cdfa6427..954d2bea4b816009a78c42682260e140bf377235 100644 (file)
@@ -435,11 +435,8 @@ JSValue JSSubtleCrypto::generateKey(ExecState* exec)
     return wrapper.promise();
 }
 
-static void importKey(ExecState* exec, CryptoKeyFormat keyFormat, CryptoOperationData data, CryptoAlgorithm* algorithmPtr, CryptoAlgorithmParameters* parametersPtr, bool extractable, CryptoKeyUsage keyUsages, CryptoAlgorithm::KeyCallback callback, CryptoAlgorithm::VoidCallback failureCallback)
+static void importKey(ExecState* exec, CryptoKeyFormat keyFormat, CryptoOperationData data, std::unique_ptr<CryptoAlgorithm> algorithm, std::unique_ptr<CryptoAlgorithmParameters> parameters, bool extractable, CryptoKeyUsage keyUsages, CryptoAlgorithm::KeyCallback callback, CryptoAlgorithm::VoidCallback failureCallback)
 {
-    std::unique_ptr<CryptoAlgorithm> algorithm(algorithmPtr);
-    std::unique_ptr<CryptoAlgorithmParameters> parameters(parametersPtr);
-
     std::unique_ptr<CryptoKeySerialization> keySerialization;
     switch (keyFormat) {
     case CryptoKeyFormat::Raw:
@@ -550,7 +547,7 @@ JSValue JSSubtleCrypto::importKey(ExecState* exec)
         wrapper.reject(nullptr);
     };
 
-    WebCore::importKey(exec, keyFormat, data, algorithm.release(), parameters.release(), extractable, keyUsages, WTF::move(successCallback), WTF::move(failureCallback));
+    WebCore::importKey(exec, keyFormat, data, WTF::move(algorithm), WTF::move(parameters), extractable, keyUsages, WTF::move(successCallback), WTF::move(failureCallback));
     if (exec->hadException())
         return jsUndefined();
 
@@ -689,6 +686,8 @@ JSValue JSSubtleCrypto::wrapKey(ExecState* exec)
     ExceptionCode ec = 0;
     WebCore::exportKey(exec, keyFormat, *key, WTF::move(exportSuccessCallback), WTF::move(exportFailureCallback));
     if (ec) {
+        delete algorithmPtr;
+        delete parametersPtr;
         setDOMException(exec, ec);
         return jsUndefined();
     }
@@ -780,7 +779,7 @@ JSValue JSSubtleCrypto::unwrapKey(ExecState* exec)
             wrapper.reject(nullptr);
         };
         ExecState* exec = domGlobalObject->globalExec();
-        WebCore::importKey(exec, keyFormat, std::make_pair(result.data(), result.size()), unwrappedKeyAlgorithmPtr, unwrappedKeyAlgorithmParametersPtr, extractable, keyUsages, WTF::move(importSuccessCallback), WTF::move(importFailureCallback));
+        WebCore::importKey(exec, keyFormat, std::make_pair(result.data(), result.size()), std::unique_ptr<CryptoAlgorithm>(unwrappedKeyAlgorithmPtr), std::unique_ptr<CryptoAlgorithmParameters>(unwrappedKeyAlgorithmParametersPtr), extractable, keyUsages, WTF::move(importSuccessCallback), WTF::move(importFailureCallback));
         if (exec->hadException()) {
             // FIXME: Report exception details to console, and possibly to calling script once there is a standardized way to pass errors to WebCrypto promise reject functions.
             exec->clearException();
@@ -797,6 +796,8 @@ JSValue JSSubtleCrypto::unwrapKey(ExecState* exec)
     ExceptionCode ec = 0;
     unwrapAlgorithm->decryptForUnwrapKey(*unwrapAlgorithmParameters, *unwrappingKey, wrappedKeyData, WTF::move(decryptSuccessCallback), WTF::move(decryptFailureCallback), ec);
     if (ec) {
+        delete unwrappedKeyAlgorithmPtr;
+        delete unwrappedKeyAlgorithmParametersPtr;
         setDOMException(exec, ec);
         return jsUndefined();
     }
index e81debcb158ab472ed6b2012a73d0f8fd57a2c97..768b0b6bca0e01c4f4454cfee301e88c2ffb8b11 100644 (file)
@@ -603,7 +603,7 @@ ALWAYS_INLINE void SelectorDataList::execute(ContainerNode& rootNode, typename S
 }
 
 SelectorQuery::SelectorQuery(CSSSelectorList&& selectorList)
-    : m_selectorList(selectorList)
+    : m_selectorList(WTF::move(selectorList))
     , m_selectors(m_selectorList)
 {
 }
index 3b75220ea35960185124a56f390d89224452cf9f..10fa99a29a26ef4dd9a75632e36e93d0595ef098 100644 (file)
@@ -162,11 +162,12 @@ void WorkerThreadableLoader::MainThreadBridge::didReceiveResponse(unsigned long
 {
     RefPtr<ThreadableLoaderClientWrapper> workerClientWrapper = m_workerClientWrapper;
     CrossThreadResourceResponseData* responseData = response.copyData().leakPtr();
-    m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, identifier, responseData] (ScriptExecutionContext& context) {
+    if (!m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, identifier, responseData] (ScriptExecutionContext& context) {
         ASSERT_UNUSED(context, context.isWorkerGlobalScope());
         OwnPtr<ResourceResponse> response(ResourceResponse::adopt(adoptPtr(responseData)));
         workerClientWrapper->didReceiveResponse(identifier, *response);
-    }, m_taskMode);
+    }, m_taskMode))
+        delete responseData;
 }
 
 void WorkerThreadableLoader::MainThreadBridge::didReceiveData(const char* data, int dataLength)
@@ -174,11 +175,12 @@ void WorkerThreadableLoader::MainThreadBridge::didReceiveData(const char* data,
     RefPtr<ThreadableLoaderClientWrapper> workerClientWrapper = m_workerClientWrapper;
     Vector<char>* vectorPtr = new Vector<char>(dataLength);
     memcpy(vectorPtr->data(), data, dataLength);
-    m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, vectorPtr] (ScriptExecutionContext& context) {
+    if (!m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, vectorPtr] (ScriptExecutionContext& context) {
         ASSERT_UNUSED(context, context.isWorkerGlobalScope());
         OwnPtr<Vector<char>> vector = adoptPtr(vectorPtr);
         workerClientWrapper->didReceiveData(vector->data(), vector->size());
-    }, m_taskMode);
+    }, m_taskMode))
+        delete vectorPtr;
 }
 
 void WorkerThreadableLoader::MainThreadBridge::didFinishLoading(unsigned long identifier, double finishTime)
@@ -194,22 +196,24 @@ void WorkerThreadableLoader::MainThreadBridge::didFail(const ResourceError& erro
 {
     RefPtr<ThreadableLoaderClientWrapper> workerClientWrapper = m_workerClientWrapper;
     ResourceError* capturedError = new ResourceError(error.copy());
-    m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, capturedError] (ScriptExecutionContext& context) {
+    if (!m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, capturedError] (ScriptExecutionContext& context) {
         ASSERT_UNUSED(context, context.isWorkerGlobalScope());
         workerClientWrapper->didFail(*capturedError);
         delete capturedError;
-    }, m_taskMode);
+    }, m_taskMode))
+        delete capturedError;
 }
 
 void WorkerThreadableLoader::MainThreadBridge::didFailAccessControlCheck(const ResourceError& error)
 {
     RefPtr<ThreadableLoaderClientWrapper> workerClientWrapper = m_workerClientWrapper;
     ResourceError* capturedError = new ResourceError(error.copy());
-    m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, capturedError] (ScriptExecutionContext& context) {
+    if (!m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper, capturedError] (ScriptExecutionContext& context) {
         ASSERT_UNUSED(context, context.isWorkerGlobalScope());
         workerClientWrapper->didFailAccessControlCheck(*capturedError);
         delete capturedError;
-    }, m_taskMode);
+    }, m_taskMode))
+        delete capturedError;
 }
 
 void WorkerThreadableLoader::MainThreadBridge::didFailRedirectCheck()
index e7a16bf8ee9aa1ee33da0f22bb7661e2a589ac21..010b14edb38215d2c14190e55c53a485daa8a314 100644 (file)
@@ -107,10 +107,10 @@ void MediaSelectionGroupAVFObjC::updateOptions()
     for (auto& avOption : m_options.keys())
         [oldAVOptions addObject:avOption];
 
-    RetainPtr<NSMutableSet> addedAVOptions = [newAVOptions mutableCopy];
+    RetainPtr<NSMutableSet> addedAVOptions = adoptNS([newAVOptions mutableCopy]);
     [addedAVOptions minusSet:oldAVOptions.get()];
 
-    RetainPtr<NSMutableSet> removedAVOptions = [oldAVOptions mutableCopy];
+    RetainPtr<NSMutableSet> removedAVOptions = adoptNS([oldAVOptions mutableCopy]);
     [removedAVOptions minusSet:newAVOptions.get()];
 
     for (AVMediaSelectionOption* removedAVOption in removedAVOptions.get()) {
index c5e1441002e6bfe92b28a0314fee330312dc8757..b26c2c3572797ad27cbcdac11b896e5653e77f6c 100644 (file)
@@ -152,8 +152,11 @@ static NSColor *correctionPatternColor = nullptr;
 
 void GraphicsContext::updateDocumentMarkerResources()
 {
+    [spellingPatternColor release];
     spellingPatternColor = nullptr;
+    [grammarPatternColor release];
     grammarPatternColor = nullptr;
+    [correctionPatternColor release];
     correctionPatternColor = nullptr;
 }