[webkit-changes] [141727] trunk/Source/WebCore

Sun, 03 Feb 2013 18:14:51 -0800

Title: [141727] trunk/Source/WebCore
Revision
141727
Author
k...@webkit.org
Date
2013-02-03 18:16:45 -0800 (Sun, 03 Feb 2013)

Log 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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (141726 => 141727)


--- trunk/Source/WebCore/ChangeLog	2013-02-04 02:15:07 UTC (rev 141726)
+++ trunk/Source/WebCore/ChangeLog	2013-02-04 02:16:45 UTC (rev 141727)
@@ -1,3 +1,26 @@
+2013-02-03  Gustavo Noronha Silva  <g...@gnome.org>
+
+        [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.
+
 2013-02-03  Kentaro Hara  <hara...@chromium.org>
 
         [V8] Pass an Isolate to HasInstance() (part 1)

Modified: trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (141726 => 141727)


--- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp	2013-02-04 02:15:07 UTC (rev 141726)
+++ trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp	2013-02-04 02:16:45 UTC (rev 141727)
@@ -524,8 +524,7 @@
     ResourceHandleInternal* d = handle->getInternal();
     gssize bytesSkipped = g_input_stream_read_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());
     if (error) {
-        ResourceHandleClient* client = handle->client();
-        client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
+        handle->client()->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
         cleanupSoupRequestOperation(handle.get());
         return;
     }
@@ -552,8 +551,7 @@
     if (handle->cancelledOrClientless())
         return;
 
-    ResourceHandleClient* client = handle->client();
-    client->didSendData(handle.get(), d->m_bodyDataSent, d->m_bodySize);
+    handle->client()->didSendData(handle.get(), d->m_bodyDataSent, d->m_bodySize);
 }
 
 static void cleanupSoupRequestOperation(ResourceHandle* handle, bool isDestroying)
@@ -622,15 +620,14 @@
     GOwnPtr<GError> error;
     d->m_inputStream = adoptGRef(soup_multipart_input_stream_next_part_finish(d->m_multipartInputStream.get(), result, &error.outPtr()));
 
-    ResourceHandleClient* client = handle->client();
     if (error) {
-        client->didFail(handle.get(), ResourceError::httpError(d->m_soupMessage.get(), error.get(), d->m_soupRequest.get()));
+        handle->client()->didFail(handle.get(), ResourceError::httpError(d->m_soupMessage.get(), error.get(), d->m_soupRequest.get()));
         cleanupSoupRequestOperation(handle.get());
         return;
     }
 
     if (!d->m_inputStream) {
-        client->didFinishLoading(handle.get(), 0);
+        handle->client()->didFinishLoading(handle.get(), 0);
         cleanupSoupRequestOperation(handle.get());
         return;
     }
@@ -639,7 +636,7 @@
     d->m_response.setURL(handle->firstRequest().url());
     d->m_response.updateFromSoupMessageHeaders(soup_multipart_input_stream_get_headers(d->m_multipartInputStream.get()));
 
-    client->didReceiveResponse(handle.get(), d->m_response);
+    handle->client()->didReceiveResponse(handle.get(), d->m_response);
 
     if (handle->cancelledOrClientless()) {
         cleanupSoupRequestOperation(handle.get());
@@ -660,7 +657,6 @@
     }
 
     ResourceHandleInternal* d = handle->getInternal();
-    ResourceHandleClient* client = handle->client();
     SoupMessage* soupMessage = d->m_soupMessage.get();
 
 
@@ -672,7 +668,7 @@
     GOwnPtr<GError> error;
     GRefPtr<GInputStream> inputStream = adoptGRef(soup_request_send_finish(d->m_soupRequest.get(), result, &error.outPtr()));
     if (error) {
-        client->didFail(handle.get(), ResourceError::httpError(soupMessage, error.get(), d->m_soupRequest.get()));
+        handle->client()->didFail(handle.get(), ResourceError::httpError(soupMessage, error.get(), d->m_soupRequest.get()));
         cleanupSoupRequestOperation(handle.get());
         return;
     }
@@ -709,7 +705,7 @@
         d->m_response.setExpectedContentLength(soup_request_get_content_length(d->m_soupRequest.get()));
     }
 
-    client->didReceiveResponse(handle.get(), d->m_response);
+    handle->client()->didReceiveResponse(handle.get(), d->m_response);
 
     if (handle->cancelledOrClientless()) {
         cleanupSoupRequestOperation(handle.get());
@@ -1318,9 +1314,8 @@
 
     g_input_stream_close_finish(d->m_inputStream.get(), res, 0);
 
-    ResourceHandleClient* client = handle->client();
-    if (client && loadingSynchronousRequest)
-        client->didFinishLoading(handle.get(), 0);
+    if (handle->client() && loadingSynchronousRequest)
+        handle->client()->didFinishLoading(handle.get(), 0);
 
     cleanupSoupRequestOperation(handle.get());
 }
@@ -1343,9 +1338,8 @@
     GOwnPtr<GError> error;
     gssize bytesRead = g_input_stream_read_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());
 
-    ResourceHandleClient* client = handle->client();
     if (error) {
-        client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
+        handle->client()->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
         cleanupSoupRequestOperation(handle.get());
         return;
     }
@@ -1363,8 +1357,8 @@
         // stream to close because the input stream is closed asynchronously. If this
         // is a synchronous request, we wait until the closeCallback, because we don't
         // want to halt the internal main loop before the input stream closes.
-        if (client && !loadingSynchronousRequest) {
-            client->didFinishLoading(handle.get(), 0);
+        if (handle->client() && !loadingSynchronousRequest) {
+            handle->client()->didFinishLoading(handle.get(), 0);
             handle->setClient(0); // Unset the client so that we do not try to access th
                                   // client in the closeCallback.
         }
@@ -1375,7 +1369,7 @@
     // It's mandatory to have sent a response before sending data
     ASSERT(!d->m_response.isNull());
 
-    client->didReceiveData(handle.get(), d->m_buffer, bytesRead, bytesRead);
+    handle->client()->didReceiveData(handle.get(), d->m_buffer, bytesRead, bytesRead);
 
     // didReceiveData may cancel the load, which may release the last reference.
     if (handle->cancelledOrClientless()) {

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to