Changeset 219089 in webkit


Ignore:
Timestamp:
Jul 3, 2017 1:17:40 PM (7 years ago)
Author:
achristensen@apple.com
Message:

Stop using dispatch_async in ResourceHandleCFURLConnectionDelegateWithOperationQueue
​https://bugs.webkit.org/show_bug.cgi?id=174059

Reviewed by Andy Estes.

Use dispatch_async_f and callOnMainThread instead.
No change in behavior.
This will allow me to use this code on Windows.

  • platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:

(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::willSendRequest):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveData):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFinishLoading):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFail):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::willCacheResponse):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveChallenge):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didSendBodyData):
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::canRespondToProtectionSpace):

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r219082 r219089  
     12017-06-30  Alex Christensen  <achristensen@webkit.org>
     2
     3        Stop using dispatch_async in ResourceHandleCFURLConnectionDelegateWithOperationQueue
     4        https://bugs.webkit.org/show_bug.cgi?id=174059
     5
     6        Reviewed by Andy Estes.
     7
     8        Use dispatch_async_f and callOnMainThread instead.
     9        No change in behavior.
     10        This will allow me to use this code on Windows.
     11
     12        * platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:
     13        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::willSendRequest):
     14        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse):
     15        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveData):
     16        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFinishLoading):
     17        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFail):
     18        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::willCacheResponse):
     19        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveChallenge):
     20        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didSendBodyData):
     21        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::canRespondToProtectionSpace):
     22
    1232017-07-03  Andy Estes  <aestes@apple.com>
    224

  • trunk/Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp

    r215686 r219089  
    102102
    103103    ASSERT(!isMainThread());
    104 
    105     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    106     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    107     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    108 
    109     dispatch_async(dispatch_get_main_queue(), ^{
     104   
     105    struct ProtectedParameters {
     106        Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis;
     107        RetainPtr<CFURLRequestRef> cfRequest;
     108        RetainPtr<CFURLResponseRef> originalRedirectResponse;
     109    };
     110   
     111    auto work = [] (void* context) {
     112        auto& parameters = *reinterpret_cast<ProtectedParameters*>(context);
     113        auto& protectedThis = parameters.protectedThis;
     114        auto& handle = protectedThis->m_handle;
     115        auto& cfRequest = parameters.cfRequest;
     116       
    110117        if (!protectedThis->hasHandle()) {
    111             continueWillSendRequest(nullptr);
    112             return;
    113         }
    114 
    115         LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::willSendRequest(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    116 
    117         RetainPtr<CFURLResponseRef> redirectResponse = synthesizeRedirectResponseIfNecessary(cfRequest, originalRedirectResponse);
     118            protectedThis->continueWillSendRequest(nullptr);
     119            return;
     120        }
     121
     122        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::willSendRequest(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     123
     124        RetainPtr<CFURLResponseRef> redirectResponse = protectedThis->synthesizeRedirectResponseIfNecessary(cfRequest.get(), parameters.originalRedirectResponse.get());
    118125        ASSERT(redirectResponse);
    119126
    120         ResourceRequest request = createResourceRequest(cfRequest, redirectResponse.get());
    121         m_handle->willSendRequest(WTFMove(request), redirectResponse.get());
    122     });
    123 
     127        ResourceRequest request = protectedThis->createResourceRequest(cfRequest.get(), redirectResponse.get());
     128        handle->willSendRequest(WTFMove(request), redirectResponse.get());
     129    };
     130   
     131    ProtectedParameters parameters { makeRef(*this), cfRequest, originalRedirectResponse };
     132    dispatch_async_f(dispatch_get_main_queue(), &parameters, work);
    124133    dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
    125134
    … …  
    129138void ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse(CFURLConnectionRef connection, CFURLResponseRef cfResponse)
    130139{
    131     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    132     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    133     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    134 
    135     dispatch_async(dispatch_get_main_queue(), ^{
    136         if (!protectedThis->hasHandle() || !m_handle->client()) {
    137             continueDidReceiveResponse();
    138             return;
    139         }
    140 
    141         LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
     140    struct ProtectedParameters {
     141        Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis;
     142        RetainPtr<CFURLConnectionRef> connection;
     143        RetainPtr<CFURLResponseRef> cfResponse;
     144    };
     145   
     146    auto work = [] (void* context) {
     147        auto& parameters = *reinterpret_cast<ProtectedParameters*>(context);
     148        auto& protectedThis = parameters.protectedThis;
     149        auto& handle = protectedThis->m_handle;
     150        auto& cfResponse = parameters.cfResponse;
     151       
     152        if (!protectedThis->hasHandle() || !handle->client()) {
     153            protectedThis->continueDidReceiveResponse();
     154            return;
     155        }
     156
     157        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
    142158
    143159        // Avoid MIME type sniffing if the response comes back as 304 Not Modified.
    144         auto msg = CFURLResponseGetHTTPResponse(cfResponse);
     160        auto msg = CFURLResponseGetHTTPResponse(cfResponse.get());
    145161        int statusCode = msg ? CFHTTPMessageGetResponseStatusCode(msg) : 0;
    146162
    147163        if (statusCode != 304) {
    148             bool isMainResourceLoad = m_handle->firstRequest().requester() == ResourceRequest::Requester::Main;
    149             adjustMIMETypeIfNecessary(cfResponse, isMainResourceLoad);
     164            bool isMainResourceLoad = handle->firstRequest().requester() == ResourceRequest::Requester::Main;
     165            adjustMIMETypeIfNecessary(cfResponse.get(), isMainResourceLoad);
    150166        }
    151167
    152168#if !PLATFORM(IOS)
    153         if (_CFURLRequestCopyProtocolPropertyForKey(m_handle->firstRequest().cfURLRequest(DoNotUpdateHTTPBody), CFSTR("ForceHTMLMIMEType")))
    154             CFURLResponseSetMIMEType(cfResponse, CFSTR("text/html"));
     169        if (_CFURLRequestCopyProtocolPropertyForKey(handle->firstRequest().cfURLRequest(DoNotUpdateHTTPBody), CFSTR("ForceHTMLMIMEType")))
     170            CFURLResponseSetMIMEType(cfResponse.get(), CFSTR("text/html"));
    155171#endif // !PLATFORM(IOS)
    156172       
    157         ResourceResponse resourceResponse(cfResponse);
     173        ResourceResponse resourceResponse(cfResponse.get());
    158174#if ENABLE(WEB_TIMING)
    159         ResourceHandle::getConnectionTimingData(connection, resourceResponse.deprecatedNetworkLoadMetrics());
    160 #else
    161         UNUSED_PARAM(connection);
     175        ResourceHandle::getConnectionTimingData(parameters.connection.get(), resourceResponse.deprecatedNetworkLoadMetrics());
    162176#endif
    163177       
    164         m_handle->didReceiveResponse(WTFMove(resourceResponse));
    165     });
     178        handle->didReceiveResponse(WTFMove(resourceResponse));
     179    };
     180   
     181    ProtectedParameters parameters { makeRef(*this), connection, cfResponse };
     182    dispatch_async_f(dispatch_get_main_queue(), &parameters, work);
    166183    dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
    167184}
    … …  
    169186void ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveData(CFDataRef data, CFIndex originalLength)
    170187{
    171     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    172     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    173     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    174     CFRetain(data);
    175 
    176     dispatch_async(dispatch_get_main_queue(), ^{
    177         if (protectedThis->hasHandle() && m_handle->client()) {
    178             LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveData(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    179 
    180             m_handle->client()->didReceiveBuffer(m_handle, SharedBuffer::create(data), originalLength);
    181         }
    182 
    183         CFRelease(data);
     188    callOnMainThread([protectedThis = makeRef(*this), data = RetainPtr<CFDataRef>(data), originalLength = originalLength] () mutable {
     189        auto& handle = protectedThis->m_handle;
     190        if (!protectedThis->hasHandle() || !handle->client())
     191            return;
     192       
     193        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveData(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     194
     195        handle->client()->didReceiveBuffer(handle, SharedBuffer::create(data.get()), originalLength);
    184196    });
    185197}
    … …  
    187199void ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFinishLoading()
    188200{
    189     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    190     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    191     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    192     dispatch_async(dispatch_get_main_queue(), ^{
    193         if (!protectedThis->hasHandle() || !m_handle->client())
    194             return;
    195 
    196         LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFinishLoading(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    197 
    198         m_handle->client()->didFinishLoading(m_handle);
     201    callOnMainThread([protectedThis = makeRef(*this)] () mutable {
     202        auto& handle = protectedThis->m_handle;
     203        if (!protectedThis->hasHandle() || !handle->client())
     204            return;
     205
     206        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFinishLoading(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     207
     208        handle->client()->didFinishLoading(handle);
    199209    });
    200210}
    … …  
    202212void ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFail(CFErrorRef error)
    203213{
    204     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    205     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    206     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    207     CFRetain(error);
    208     dispatch_async(dispatch_get_main_queue(), ^{
    209         if (protectedThis->hasHandle() && m_handle->client()) {
    210             LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFail(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    211 
    212             m_handle->client()->didFail(m_handle, ResourceError(error));
    213         }
    214 
    215         CFRelease(error);
     214    callOnMainThread([protectedThis = makeRef(*this), error = RetainPtr<CFErrorRef>(error)] () mutable {
     215        auto& handle = protectedThis->m_handle;
     216        if (!protectedThis->hasHandle() || !handle->client())
     217            return;
     218       
     219        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didFail(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     220
     221        handle->client()->didFail(handle, ResourceError(error.get()));
    216222    });
    217223}
    … …  
    219225CFCachedURLResponseRef ResourceHandleCFURLConnectionDelegateWithOperationQueue::willCacheResponse(CFCachedURLResponseRef cachedResponse)
    220226{
    221     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    222     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    223     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    224 
    225     dispatch_async(dispatch_get_main_queue(), ^{
    226         if (!protectedThis->hasHandle() || !m_handle->client()) {
    227             continueWillCacheResponse(nullptr);
    228             return;
    229         }
    230 
    231         LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::willCacheResponse(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    232 
    233         m_handle->client()->willCacheResponseAsync(m_handle, cachedResponse);
    234     });
     227    struct ProtectedParameters {
     228        Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis;
     229        RetainPtr<CFCachedURLResponseRef> cachedResponse;
     230    };
     231   
     232    auto work = [] (void* context) {
     233        auto& parameters = *reinterpret_cast<ProtectedParameters*>(context);
     234        auto& protectedThis = parameters.protectedThis;
     235        auto& handle = protectedThis->m_handle;
     236       
     237        if (!protectedThis->hasHandle() || !handle->client()) {
     238            protectedThis->continueWillCacheResponse(nullptr);
     239            return;
     240        }
     241
     242        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::willCacheResponse(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     243
     244        handle->client()->willCacheResponseAsync(handle, parameters.cachedResponse.get());
     245    };
     246   
     247    ProtectedParameters parameters { makeRef(*this), cachedResponse };
     248    dispatch_async_f(dispatch_get_main_queue(), &parameters, work);
    235249    dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
    236250    return m_cachedResponseResult.leakRef();
    … …  
    239253void ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveChallenge(CFURLAuthChallengeRef challenge)
    240254{
    241     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    242     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    243     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    244     CFRetain(challenge);
    245     dispatch_async(dispatch_get_main_queue(), ^{
    246         if (protectedThis->hasHandle()) {
    247             LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveChallenge(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    248 
    249             m_handle->didReceiveAuthenticationChallenge(AuthenticationChallenge(challenge, m_handle));
    250         }
    251 
    252         CFRelease(challenge);
     255    callOnMainThread([protectedThis = makeRef(*this), challenge = RetainPtr<CFURLAuthChallengeRef>(challenge)] () mutable {
     256        auto& handle = protectedThis->m_handle;
     257        if (!protectedThis->hasHandle())
     258            return;
     259       
     260        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveChallenge(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     261
     262        handle->didReceiveAuthenticationChallenge(AuthenticationChallenge(challenge.get(), handle));
    253263    });
    254264}
    … …  
    256266void ResourceHandleCFURLConnectionDelegateWithOperationQueue::didSendBodyData(CFIndex totalBytesWritten, CFIndex totalBytesExpectedToWrite)
    257267{
    258     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    259     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    260     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    261     dispatch_async(dispatch_get_main_queue(), ^{
    262         if (!protectedThis->hasHandle() || !m_handle->client())
    263             return;
    264 
    265         LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didSendBodyData(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    266 
    267         m_handle->client()->didSendData(m_handle, totalBytesWritten, totalBytesExpectedToWrite);
     268    callOnMainThread([protectedThis = makeRef(*this), totalBytesWritten, totalBytesExpectedToWrite] () mutable {
     269        auto& handle = protectedThis->m_handle;
     270        if (!protectedThis->hasHandle() || !handle->client())
     271            return;
     272
     273        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::didSendBodyData(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     274
     275        handle->client()->didSendData(handle, totalBytesWritten, totalBytesExpectedToWrite);
    268276    });
    269277}
    … …  
    277285Boolean ResourceHandleCFURLConnectionDelegateWithOperationQueue::canRespondToProtectionSpace(CFURLProtectionSpaceRef protectionSpace)
    278286{
    279     // FIXME: The block implicitly copies protector object, which is wasteful. We should just call ref(),
    280     // capture "this" by pointer value, and use a C++ lambda to prevent other unintentional capturing.
    281     RefPtr<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis(this);
    282 
    283     dispatch_async(dispatch_get_main_queue(), ^{
     287    struct ProtectedParameters {
     288        Ref<ResourceHandleCFURLConnectionDelegateWithOperationQueue> protectedThis;
     289        RetainPtr<CFURLProtectionSpaceRef> protectionSpace;
     290    };
     291   
     292    auto work = [] (void* context) {
     293        auto& parameters = *reinterpret_cast<ProtectedParameters*>(context);
     294        auto& protectedThis = parameters.protectedThis;
     295        auto& handle = protectedThis->m_handle;
     296       
    284297        if (!protectedThis->hasHandle()) {
    285             continueCanAuthenticateAgainstProtectionSpace(false);
    286             return;
    287         }
    288 
    289         LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::canRespondToProtectionSpace(handle=%p) (%s)", m_handle, m_handle->firstRequest().url().string().utf8().data());
    290 
    291         ProtectionSpace coreProtectionSpace = ProtectionSpace(protectionSpace);
     298            protectedThis->continueCanAuthenticateAgainstProtectionSpace(false);
     299            return;
     300        }
     301
     302        LOG(Network, "CFNet - ResourceHandleCFURLConnectionDelegateWithOperationQueue::canRespondToProtectionSpace(handle=%p) (%s)", handle, handle->firstRequest().url().string().utf8().data());
     303
     304        ProtectionSpace coreProtectionSpace = ProtectionSpace(parameters.protectionSpace.get());
    292305#if PLATFORM(IOS)
    293306        if (coreProtectionSpace.authenticationScheme() == ProtectionSpaceAuthenticationSchemeUnknown) {
    … …  
    297310        }
    298311#endif // PLATFORM(IOS)
    299         m_handle->canAuthenticateAgainstProtectionSpace(coreProtectionSpace);
    300     });
     312        handle->canAuthenticateAgainstProtectionSpace(coreProtectionSpace);
     313    };
     314   
     315    ProtectedParameters parameters { makeRef(*this), protectionSpace };
     316    dispatch_async_f(dispatch_get_main_queue(), &parameters, work);
    301317    dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
    302318    return m_boolResult;
Note: See TracChangeset for help on using the changeset viewer.