Source/WebCore: Revert most of the multipart changes in

http://trac.webkit.org/changeset/104756.
https://bugs.webkit.org/show_bug.cgi?id=76297

Reviewed by Alexey Proskuryakov.

http/tests/multipart/invalid-image-data.html
should stop asserting on chromium win dbg.

* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::didReceiveResponse):
(WebCore::SubresourceLoader::didReceiveData):
(WebCore::SubresourceLoader::sendDataToResource):

LayoutTests: http/tests/multipart/invalid-image-data.html should no longer
be crashing on chromium win dbg.
https://bugs.webkit.org/show_bug.cgi?id=76297

Reviewed by Alexey Proskuryakov.

* platform/chromium/test_expectations.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105000 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 87c7932..5439614 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2012-01-13  Nate Chapin  <japhet@chromium.org>
+
+        http/tests/multipart/invalid-image-data.html should no longer
+        be crashing on chromium win dbg.
+        https://bugs.webkit.org/show_bug.cgi?id=76297
+
+        Reviewed by Alexey Proskuryakov.
+
+        * platform/chromium/test_expectations.txt:
+
 2012-01-13  Raymond Toy   <rtoy@chromium.org>
         EQUALPOWER panner incorrectly computes gain
         https://bugs.webkit.org/show_bug.cgi?id=75767

diff --git a/LayoutTests/platform/chromium/test_expectations.txt b/LayoutTests/platform/chromium/test_expectations.txt
index cc8a2c6..f19fe5e 100644 (file)
--- a/LayoutTests/platform/chromium/test_expectations.txt
+++ b/LayoutTests/platform/chromium/test_expectations.txt
@@ -1213,8 +1213,7 @@ BUGCR10357 : fast/dom/cssTarget-crash.html = PASS TIMEOUT
 BUGCR10361 WIN : http/tests/misc/single-character-pi-stylesheet.xhtml = FAIL
 
 // Windows is missing the green box. Started to pass in roll to WebKit r50313
-// Started crashing on XP after http://trac.webkit.org/changeset/104756
-BUGCR8729 WIN : http/tests/multipart/invalid-image-data.html = IMAGE+TEXT CRASH
+BUGCR8729 WIN : http/tests/multipart/invalid-image-data.html = IMAGE+TEXT
 
 // -----------------------------------------------------------------
 // MAC PORT TESTS

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6671f5b..d64c7da 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2012-01-13  Nate Chapin  <japhet@chromium.org>
+
+        Revert most of the multipart changes in
+        http://trac.webkit.org/changeset/104756.
+        https://bugs.webkit.org/show_bug.cgi?id=76297
+
+        Reviewed by Alexey Proskuryakov.
+
+        http/tests/multipart/invalid-image-data.html
+        should stop asserting on chromium win dbg.
+
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::didReceiveResponse):
+        (WebCore::SubresourceLoader::didReceiveData):
+        (WebCore::SubresourceLoader::sendDataToResource):
+
 2011-01-13  Jer Noble  <jer.noble@apple.com>
 
         WebAudio: Use float instead of double values for gain operations.

diff --git a/Source/WebCore/loader/SubresourceLoader.cpp b/Source/WebCore/loader/SubresourceLoader.cpp
index 2883148..7f40261 100644 (file)
--- a/Source/WebCore/loader/SubresourceLoader.cpp
+++ b/Source/WebCore/loader/SubresourceLoader.cpp
@@ -197,12 +197,21 @@ void SubresourceLoader::didReceiveResponse(const ResourceResponse& response)
 
         // We don't count multiParts in a CachedResourceLoader's request count
         m_requestCountTracker.clear();
-        setShouldBufferData(DoNotBufferData);
         if (!m_resource->isImage()) {
             cancel();
             return;
         }
     }
+
+    RefPtr<SharedBuffer> buffer = resourceData();
+    if (m_loadingMultipartContent && buffer && buffer->size()) {
+        sendDataToResource(buffer->data(), buffer->size());
+        clearResourceData();
+        // Since a subresource loader does not load multipart sections progressively, data was delivered to the loader all at once.        
+        // After the first multipart section is complete, signal to delegates that this load is "finished" 
+        m_documentLoader->subresourceLoaderFinishedLoadingOnePart(this);
+        didFinishLoadingOnePart(0);
+    }
 }
 
 void SubresourceLoader::didReceiveData(const char* data, int length, long long encodedDataLength, bool allAtOnce)
@@ -215,17 +224,10 @@ void SubresourceLoader::didReceiveData(const char* data, int length, long long e
     RefPtr<SubresourceLoader> protect(this);
     ResourceLoader::didReceiveData(data, length, encodedDataLength, allAtOnce);
 
-    if (errorLoadingResource())
+    if (errorLoadingResource() || m_loadingMultipartContent)
         return;
 
     sendDataToResource(data, length);
-    
-    if (m_loadingMultipartContent) {
-        // Since a subresource loader does not load multipart sections progressively, data was delivered to the loader all at once.        
-        // After the first multipart section is complete, signal to delegates that this load is "finished" 
-        m_documentLoader->subresourceLoaderFinishedLoadingOnePart(this);
-        didFinishLoadingOnePart(0);
-    }
 }
 
 bool SubresourceLoader::errorLoadingResource()
@@ -241,8 +243,17 @@ bool SubresourceLoader::errorLoadingResource()
 
 void SubresourceLoader::sendDataToResource(const char* data, int length)
 {
-    RefPtr<SharedBuffer> buffer = resourceData() ? resourceData() : SharedBuffer::create(data, length);
-    m_resource->data(buffer.release(), m_loadingMultipartContent);
+    // There are two cases where we might need to create our own SharedBuffer instead of copying the one in ResourceLoader. 
+    // (1) Multipart content: The loader delivers the data in a multipart section all at once, then sends eof. 
+    //     The resource data will change as the next part is loaded, so we need to make a copy. 
+    // (2) Our client requested that the data not be buffered at the ResourceLoader level via ResourceLoaderOptions. In this case, 
+    //     ResourceLoader::resourceData() will be null. However, unlike the multipart case, we don't want to tell the CachedResource 
+    //     that all data has been received yet. 
+    if (m_loadingMultipartContent || !resourceData()) { 
+        RefPtr<SharedBuffer> copiedData = SharedBuffer::create(data, length); 
+        m_resource->data(copiedData.release(), m_loadingMultipartContent); 
+    } else 
+        m_resource->data(resourceData(), false);
 }
 
 void SubresourceLoader::didReceiveCachedMetadata(const char* data, int length)