Stop using dispatch_async in ResourceHandleCFURLConnectionDelegateWithOperationQueue
Created attachment 314338 [details] Patch
This will allow me to use this code on Windows.
Comment on attachment 314338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314338&action=review > Source/WebCore/ChangeLog:9 > + Use dispatch_async_f and callOnMainThread instead. > + No change in behavior. This is missing a reason why.
Comment on attachment 314338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314338&action=review > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:110 > + struct ProtectedParameters { > + Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis; > + RefPtr<ResourceHandle> handle; > + RetainPtr<CFURLRequestRef> cfRequest; > + RetainPtr<CFURLResponseRef> originalRedirectResponse; > + }; This is fine, but I wonder if using tuples and std::tie would be better than a struct. Maybe that's not supported right now in MSVC? > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:113 > + ASSERT(context); I don't think you need to assert this. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:116 > + auto& parameters = *reinterpret_cast<ProtectedParameters*>(context); > + auto& protectedThis = parameters.protectedThis; > + auto& handle = parameters.handle; It's not clear why you chose to unpack these three parameters but not the others. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:146 > + struct ProtectedParameters { > + Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis; > + RefPtr<ResourceHandle> handle; > + RetainPtr<CFURLConnectionRef> connection; > + RetainPtr<CFURLResponseRef> cfResponse; > + }; This is fine, but I wonder if using tuples and std::tie would be better than a struct. Maybe that's not supported right now in MSVC? > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:149 > + ASSERT(context); I don't think you need to assert this. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:152 > + auto& parameters = *reinterpret_cast<ProtectedParameters*>(context); > + auto& protectedThis = parameters.protectedThis; > + auto& handle = parameters.handle; It's not clear why you chose to unpack these three parameters but not the others. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:192 > + callOnMainThread([protectedThis = makeRef(*this), handle = RefPtr<ResourceHandle>(m_handle), data = RetainPtr<CFDataRef>(data), originalLength = originalLength] () mutable { Do you need to create a second reference to m_handle if you already have protectedThis? > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:203 > + callOnMainThread([protectedThis = makeRef(*this), handle = RefPtr<ResourceHandle>(m_handle)] () mutable { Ditto. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:215 > + callOnMainThread([protectedThis = makeRef(*this), handle = RefPtr<ResourceHandle>(m_handle), error = RetainPtr<CFErrorRef>(error)] () mutable { Ditto. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:230 > + struct ProtectedParameters { > + Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis; > + RefPtr<ResourceHandle> handle; > + RetainPtr<CFCachedURLResponseRef> cachedResponse; > + }; This is fine, but I wonder if using tuples and std::tie would be better than a struct. Maybe that's not supported right now in MSVC? > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:233 > + ASSERT(context); I don't think you need to assert this. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:236 > + auto& parameters = *reinterpret_cast<ProtectedParameters*>(context); > + auto& protectedThis = parameters.protectedThis; > + auto& handle = parameters.handle; It's not clear why you chose to unpack these three parameters but not the others. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:267 > + callOnMainThread([protectedThis = makeRef(*this), handle = RefPtr<ResourceHandle>(m_handle), totalBytesWritten, totalBytesExpectedToWrite] () mutable { Do you need to create a second reference to m_handle if you already have protectedThis? > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:289 > + struct ProtectedParameters { > + Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis; > + RefPtr<ResourceHandle> handle; > + RetainPtr<CFURLProtectionSpaceRef> protectionSpace; > + }; This is fine, but I wonder if using tuples and std::tie would be better than a struct. Maybe that's not supported right now in MSVC? > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:292 > + ASSERT(context); I don't think you need to assert this. > Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:295 > + auto& parameters = *reinterpret_cast<ProtectedParameters*>(context); > + auto& protectedThis = parameters.protectedThis; > + auto& handle = parameters.handle; It's not clear why you chose to unpack these three parameters but not the others.
Created attachment 314510 [details] Patch
http://trac.webkit.org/r219089