Move multipart resource handling to core/fetch (2/2)

third_party/WebKit/Source/core/fetch/ImageResource.cpp

Index: third_party/WebKit/Source/core/fetch/ImageResource.cpp
diff --git a/third_party/WebKit/Source/core/fetch/ImageResource.cpp b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
index 99a43439911e7876704f3f39205cd5290f707912..507ad97f8c3993810f8e48d6a9ced2d25b4545b2 100644
--- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
@@ -99,8 +99,10 @@ ImageResource::~ImageResource()
 
 DEFINE_TRACE(ImageResource)
 {
+    visitor->trace(m_multipartParser);
     Resource::trace(visitor);
     ImageObserver::trace(visitor);
+    MultipartImageResourceParser::Client::trace(visitor);
 }
 
 void ImageResource::load(ResourceFetcher* fetcher, const ResourceLoaderOptions& options)
@@ -159,9 +161,21 @@ void ImageResource::allClientsRemoved()
 {
     if (m_image && !errorOccurred())
         m_image->resetAnimation();
+    if (m_multipartParser)
+        m_multipartParser->cancel();
     Resource::allClientsRemoved();
 }
 
+void ImageResource::appendData(const char* data, size_t length)
+{
+    if (m_multipartParser) {
+        m_multipartParser->appendData(data, length);
+    } else {
+        Resource::appendData(data, length);
+        updateImage(false);
+    }
+}
+
 std::pair<blink::Image*, float> ImageResource::brokenImage(float deviceScaleFactor)
 {
     if (deviceScaleFactor >= 2) {
@@ -293,13 +307,6 @@ inline void ImageResource::clearImage()
     m_image.clear();
 }
 
-void ImageResource::appendData(const char* data, size_t length)
-{
-    Resource::appendData(data, length);
-    if (!loadingMultipartContent())
-        updateImage(false);
-}
-
 void ImageResource::updateImage(bool allDataReceived)
 {
     TRACE_EVENT0("blink", "ImageResource::updateImage");
@@ -333,18 +340,25 @@ void ImageResource::updateImage(bool allDataReceived)
     }
 }
 
-void ImageResource::finishOnePart()
+void ImageResource::finish()
 {
-    if (loadingMultipartContent())
-        clear();
-    updateImage(true);
-    if (loadingMultipartContent())
-        m_data.clear();
-    Resource::finishOnePart();
+    if (m_multipartParser) {
+        m_multipartParser->finish();
+        if (m_data) {
+            clearImage();
+            updateImage(true);
+            m_data.clear();
+        }
+    } else {
+        updateImage(true);
+    }
+    Resource::finish();
 }
 
 void ImageResource::error(Resource::Status status)
 {
+    if (m_multipartParser)
+        m_multipartParser->cancel();
     clear();
     Resource::error(status);
     notifyObservers();
@@ -352,8 +366,11 @@ void ImageResource::error(Resource::Status status)
 
 void ImageResource::responseReceived(const ResourceResponse& response, PassOwnPtr<WebDataConsumerHandle> handle)
 {
-    if (loadingMultipartContent() && m_data)
-        finishOnePart();
+    ASSERT(!handle);
+    ASSERT(!m_multipartParser);
+    // If there's no boundary, just handle the request normally.
+    if (response.isMultipart() && !response.multipartBoundary().isEmpty())
+        m_multipartParser = new MultipartImageResourceParser(response, response.multipartBoundary(), this);
     Resource::responseReceived(response, handle);
     if (RuntimeEnabledFeatures::clientHintsEnabled()) {
         m_devicePixelRatioHeaderValue = m_response.httpHeaderField(HTTPNames::Content_DPR).toFloat(&m_hasDevicePixelRatioHeaderValue);
@@ -454,6 +471,27 @@ void ImageResource::changedInRect(const blink::Image* image, const IntRect& rect
     notifyObservers(&rect);
 }
 
+void ImageResource::onePartInMultipartReceived(const ResourceResponse& response, bool isFirstPart)
+{
+    ASSERT(isMultipartImage());
+    m_response = response;
+    if (m_data) {
+        clear();
+        updateImage(true);
+        m_data.clear();
+        setLoading(false);
+        checkNotify();

[Nate Chapin, 2016/03/01 19:02:15]

I think this triggers multiple calls of ResourceClient::notifyFinished?

[yhirano, 2016/03/01 19:43:23]

Yes, and it's what the existing code is doing. Are you suggesting to fix it in
this CL?

[Nate Chapin, 2016/03/01 19:56:45]

Oops. Didn't know the existing code did that :(

That sounds like a bug, but not one that should block this refactor.

+    }
+    if (!isFirstPart && m_loader)
+        m_loader->didFinishLoadingOnePart(0, WebURLLoaderClient::kUnknownEncodedDataLength);
+}
+
+void ImageResource::multipartDataReceived(const char* bytes, size_t size)
+{
+    ASSERT(isMultipartImage());
+    Resource::appendData(bytes, size);
+}
+
 bool ImageResource::isAccessAllowed(SecurityOrigin* securityOrigin)
 {
     if (response().wasFetchedViaServiceWorker())
@@ -465,9 +503,4 @@ bool ImageResource::isAccessAllowed(SecurityOrigin* securityOrigin)
     return !securityOrigin->taintsCanvas(response().url());
 }
 
-bool ImageResource::loadingMultipartContent() const
-{
-    return m_loader && m_loader->loadingMultipartContent();
-}
-
 } // namespace blink