Title: [98987] trunk/Source/WebCore
Revision
98987
Author
gav...@chromium.org
Date
2011-11-01 11:54:51 -0700 (Tue, 01 Nov 2011)

Log Message

properly end requests when a bad status code return happens
https://bugs.webkit.org/show_bug.cgi?id=71122

Calling error without ending the request set up the CachedResourceRequest so that it could
actually send out two notifyFinished() events.  This probably was the root cause of
lots of crashing instability; I know from crbug.com/75604 that this bug was causing lots
of crashes in ScriptRunner/ScriptElement for instance.

The fix is easy: just properly end the request instead of just calling error, and we won't
re-notify.

Reviewed by Nate Chapin.

No new tests, as the problem wasn't very amenable to layout tests.
There is a chromium test going through code review at http://codereview.chromium.org/8404001/

* loader/cache/CachedResourceRequest.cpp:
(WebCore::CachedResourceRequest::didReceiveData):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (98986 => 98987)


--- trunk/Source/WebCore/ChangeLog	2011-11-01 18:40:03 UTC (rev 98986)
+++ trunk/Source/WebCore/ChangeLog	2011-11-01 18:54:51 UTC (rev 98987)
@@ -1,3 +1,24 @@
+2011-11-01  Gavin Peters  <gav...@chromium.org>
+
+        properly end requests when a bad status code return happens
+        https://bugs.webkit.org/show_bug.cgi?id=71122
+
+        Calling error without ending the request set up the CachedResourceRequest so that it could
+        actually send out two notifyFinished() events.  This probably was the root cause of
+        lots of crashing instability; I know from crbug.com/75604 that this bug was causing lots
+        of crashes in ScriptRunner/ScriptElement for instance.
+
+        The fix is easy: just properly end the request instead of just calling error, and we won't
+        re-notify.
+
+        Reviewed by Nate Chapin.
+
+        No new tests, as the problem wasn't very amenable to layout tests.
+        There is a chromium test going through code review at http://codereview.chromium.org/8404001/
+
+        * loader/cache/CachedResourceRequest.cpp:
+        (WebCore::CachedResourceRequest::didReceiveData):
+
 2011-11-01  Erik Arvidsson  <a...@chromium.org>
 
         Remove LegacyDefaultOptionalArguments flag from CanvasRenderingContext2d

Modified: trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp (98986 => 98987)


--- trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp	2011-11-01 18:40:03 UTC (rev 98986)
+++ trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp	2011-11-01 18:54:51 UTC (rev 98987)
@@ -292,7 +292,12 @@
         return;
 
     if (m_resource->response().httpStatusCode() >= 400 && !m_resource->shouldIgnoreHTTPStatusCodeErrors()) {
+        if (!m_multipart)
+            m_cachedResourceLoader->decrementRequestCount(m_resource);
+        m_finishing = true;
+        m_loader->clearClient();
         m_resource->error(CachedResource::LoadError);
+        end();
         return;
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to