Changeset 141727 in webkit


Ignore:
Timestamp:
Feb 3, 2013 6:16:45 PM (11 years ago)
Author:
kov@webkit.org
Message:

[Soup] Do not use local variables for the client
​https://bugs.webkit.org/show_bug.cgi?id=108714

Reviewed by Martin Robinson.

Covered by existing tests, refactoring code only.

We have had problems in the past with the client being destroyed or
changed inside a method or function, and we ended up with a stale
pointer, leading to crashes. This refactoring is an effort to minimize
the possibility of hitting that same issue in the future.

  • platform/network/soup/ResourceHandleSoup.cpp:

(WebCore::redirectSkipCallback): no longer use a local variable to hold
the client.
(WebCore::wroteBodyDataCallback): ditto.
(WebCore::nextMultipartResponsePartCallback): ditto.
(WebCore::sendRequestCallback): ditto.
(WebCore::closeCallback): ditto.
(WebCore::readCallback): ditto.

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r141721 r141727  
     12013-02-03  Gustavo Noronha Silva  <gns@gnome.org>
     2
     3        [Soup] Do not use local variables for the client
     4        https://bugs.webkit.org/show_bug.cgi?id=108714
     5
     6        Reviewed by Martin Robinson.
     7
     8        Covered by existing tests, refactoring code only.
     9
     10        We have had problems in the past with the client being destroyed or
     11        changed inside a method or function, and we ended up with a stale
     12        pointer, leading to crashes. This refactoring is an effort to minimize
     13        the possibility of hitting that same issue in the future.
     14
     15        * platform/network/soup/ResourceHandleSoup.cpp:
     16        (WebCore::redirectSkipCallback): no longer use a local variable to hold
     17        the client.
     18        (WebCore::wroteBodyDataCallback): ditto.
     19        (WebCore::nextMultipartResponsePartCallback): ditto.
     20        (WebCore::sendRequestCallback): ditto.
     21        (WebCore::closeCallback): ditto.
     22        (WebCore::readCallback): ditto.
     23
    1242013-02-03  Kentaro Hara  <haraken@chromium.org>
    225

  • trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp

    r140836 r141727  
    525525    gssize bytesSkipped = g_input_stream_read_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());
    526526    if (error) {
    527         ResourceHandleClient* client = handle->client();
    528         client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
     527        handle->client()->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
    529528        cleanupSoupRequestOperation(handle.get());
    530529        return;
    … …  
    553552        return;
    554553
    555     ResourceHandleClient* client = handle->client();
    556     client->didSendData(handle.get(), d->m_bodyDataSent, d->m_bodySize);
     554    handle->client()->didSendData(handle.get(), d->m_bodyDataSent, d->m_bodySize);
    557555}
    558556
    … …  
    623621    d->m_inputStream = adoptGRef(soup_multipart_input_stream_next_part_finish(d->m_multipartInputStream.get(), result, &error.outPtr()));
    624622
    625     ResourceHandleClient* client = handle->client();
    626623    if (error) {
    627         client->didFail(handle.get(), ResourceError::httpError(d->m_soupMessage.get(), error.get(), d->m_soupRequest.get()));
     624        handle->client()->didFail(handle.get(), ResourceError::httpError(d->m_soupMessage.get(), error.get(), d->m_soupRequest.get()));
    628625        cleanupSoupRequestOperation(handle.get());
    629626        return;
    … …  
    631628
    632629    if (!d->m_inputStream) {
    633         client->didFinishLoading(handle.get(), 0);
     630        handle->client()->didFinishLoading(handle.get(), 0);
    634631        cleanupSoupRequestOperation(handle.get());
    635632        return;
    … …  
    640637    d->m_response.updateFromSoupMessageHeaders(soup_multipart_input_stream_get_headers(d->m_multipartInputStream.get()));
    641638
    642     client->didReceiveResponse(handle.get(), d->m_response);
     639    handle->client()->didReceiveResponse(handle.get(), d->m_response);
    643640
    644641    if (handle->cancelledOrClientless()) {
    … …  
    661658
    662659    ResourceHandleInternal* d = handle->getInternal();
    663     ResourceHandleClient* client = handle->client();
    664660    SoupMessage* soupMessage = d->m_soupMessage.get();
    665661
    … …  
    673669    GRefPtr<GInputStream> inputStream = adoptGRef(soup_request_send_finish(d->m_soupRequest.get(), result, &error.outPtr()));
    674670    if (error) {
    675         client->didFail(handle.get(), ResourceError::httpError(soupMessage, error.get(), d->m_soupRequest.get()));
     671        handle->client()->didFail(handle.get(), ResourceError::httpError(soupMessage, error.get(), d->m_soupRequest.get()));
    676672        cleanupSoupRequestOperation(handle.get());
    677673        return;
    … …  
    710706    }
    711707
    712     client->didReceiveResponse(handle.get(), d->m_response);
     708    handle->client()->didReceiveResponse(handle.get(), d->m_response);
    713709
    714710    if (handle->cancelledOrClientless()) {
    … …  
    13191315    g_input_stream_close_finish(d->m_inputStream.get(), res, 0);
    13201316
    1321     ResourceHandleClient* client = handle->client();
    1322     if (client && loadingSynchronousRequest)
    1323         client->didFinishLoading(handle.get(), 0);
     1317    if (handle->client() && loadingSynchronousRequest)
     1318        handle->client()->didFinishLoading(handle.get(), 0);
    13241319
    13251320    cleanupSoupRequestOperation(handle.get());
    … …  
    13441339    gssize bytesRead = g_input_stream_read_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());
    13451340
    1346     ResourceHandleClient* client = handle->client();
    13471341    if (error) {
    1348         client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
     1342        handle->client()->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
    13491343        cleanupSoupRequestOperation(handle.get());
    13501344        return;
    … …  
    13641358        // is a synchronous request, we wait until the closeCallback, because we don't
    13651359        // want to halt the internal main loop before the input stream closes.
    1366         if (client && !loadingSynchronousRequest) {
    1367             client->didFinishLoading(handle.get(), 0);
     1360        if (handle->client() && !loadingSynchronousRequest) {
     1361            handle->client()->didFinishLoading(handle.get(), 0);
    13681362            handle->setClient(0); // Unset the client so that we do not try to access th
    13691363                                  // client in the closeCallback.
    … …  
    13761370    ASSERT(!d->m_response.isNull());
    13771371
    1378     client->didReceiveData(handle.get(), d->m_buffer, bytesRead, bytesRead);
     1372    handle->client()->didReceiveData(handle.get(), d->m_buffer, bytesRead, bytesRead);
    13791373
    13801374    // didReceiveData may cancel the load, which may release the last reference.
Note: See TracChangeset for help on using the changeset viewer.