Bug 1151309 (Part 2) - Hide errors in multipart image parts both visu…

…ally and internally. r=tn
dothq · Apr 15, 2015 · c3f939a · c3f939a
1 parent b9f71b1
commit c3f939a
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 13 deletions.
diff --git a/image/src/MultipartImage.cpp b/image/src/MultipartImage.cpp
@@ -151,6 +151,21 @@ void MultipartImage::FinishTransition()
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mNextPart, "Should have a next part here");
 
+  nsRefPtr<ProgressTracker> newCurrentPartTracker =
+    mNextPart->GetProgressTracker();
+  if (newCurrentPartTracker->GetProgress() & FLAG_HAS_ERROR) {
+    // This frame has an error; drop it.
+    mNextPart = nullptr;
+
+    // We still need to notify, though.
+    mTracker->ResetForNewRequest();
+    nsRefPtr<ProgressTracker> currentPartTracker =
+      InnerImage()->GetProgressTracker();
+    mTracker->SyncNotifyProgress(currentPartTracker->GetProgress());
+
+    return;
+  }
+
   // Stop observing the current part.
   {
     nsRefPtr<ProgressTracker> currentPartTracker =
@@ -162,8 +177,6 @@ void MultipartImage::FinishTransition()
   mTracker->ResetForNewRequest();
   SetInnerImage(mNextPart);
   mNextPart = nullptr;
-  nsRefPtr<ProgressTracker> newCurrentPartTracker =
-    InnerImage()->GetProgressTracker();
   newCurrentPartTracker->AddObserver(this);
 
   // Finally, send all the notifications for the new current part and send a
@@ -210,12 +223,14 @@ MultipartImage::OnImageDataAvailable(nsIRequest* aRequest,
   // We may trigger notifications that will free mNextPart, so keep it alive.
   nsRefPtr<Image> nextPart = mNextPart;
   if (nextPart) {
-    return nextPart->OnImageDataAvailable(aRequest, aContext, aInStr,
-                                          aSourceOffset, aCount);
+    nextPart->OnImageDataAvailable(aRequest, aContext, aInStr,
+                                   aSourceOffset, aCount);
+  } else {
+    InnerImage()->OnImageDataAvailable(aRequest, aContext, aInStr,
+                                       aSourceOffset, aCount);
   }
 
-  return InnerImage()->OnImageDataAvailable(aRequest, aContext, aInStr,
-                                            aSourceOffset, aCount);
+  return NS_OK;
 }
 
 nsresult
@@ -230,12 +245,12 @@ MultipartImage::OnImageDataComplete(nsIRequest* aRequest,
   // We may trigger notifications that will free mNextPart, so keep it alive.
   nsRefPtr<Image> nextPart = mNextPart;
   if (nextPart) {
-    return nextPart->OnImageDataComplete(aRequest, aContext, aStatus,
-                                         aLastPart);
+    nextPart->OnImageDataComplete(aRequest, aContext, aStatus, aLastPart);
+  } else {
+    InnerImage()->OnImageDataComplete(aRequest, aContext, aStatus, aLastPart);
   }
 
-  return InnerImage()->OnImageDataComplete(aRequest, aContext, aStatus,
-                                           aLastPart);
+  return NS_OK;
 }
 
 void

diff --git a/image/test/mochitest/test_bug733553.html b/image/test/mochitest/test_bug733553.html
@@ -36,11 +36,15 @@
   // An invalid image (from bug 787899) that is further delivered with a
   // "special" bad MIME type that indicates that the necko
   // multipart/x-mixed-replace parser wasn't able to parse it.
-  [0, "rillybad.jpg"],
+  // We use a width of 80 because MultipartImage will just drop rillybad.jpg
+  // and re-present damon.jpg.
+  [80, "rillybad.jpg"],
   [80, "damon.jpg"],
-  [0, "bad.jpg"],
+  // Similarly, we'll drop bad.jpg, so we use damon.jpg's width.
+  [80, "bad.jpg"],
   [1, "red.png"],
-  [0, "invalid.jpg"],
+  // We also drop invalid.jpg, so we use red.png's width.
+  [1, "invalid.jpg"],
   [40, "animated-gif2.gif"]
 ];