Bug 1151309 - Part 2: Hide errors in multipart image parts both visually and internally. r=tn, a=sledru
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 14 Apr 2015 17:47:59 -0700
changeset 260296 0fcbbecc843d
parent 260295 046c97d2eb23
child 260297 cb2725c612b2
push id741
push userryanvm@gmail.com
push date2015-04-27 20:01 +0000
treeherdermozilla-release@d10817faa571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, sledru
bugs1151309
milestone38.0
Bug 1151309 - Part 2: Hide errors in multipart image parts both visually and internally. r=tn, a=sledru
image/src/MultipartImage.cpp
image/test/mochitest/test_bug733553.html
--- a/image/src/MultipartImage.cpp
+++ b/image/src/MultipartImage.cpp
@@ -146,29 +146,42 @@ MultipartImage::BeginTransitionToPart(Im
   mNextPart->IncrementAnimationConsumers();
 }
 
 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 =
       InnerImage()->GetProgressTracker();
     currentPartTracker->RemoveObserver(this);
   }
 
   // Make the next part become the current part.
   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
   // FRAME_UPDATE notification so that observers know to redraw.
   mTracker->SyncNotifyProgress(newCurrentPartTracker->GetProgress(),
                                nsIntRect::GetMaxSizedIntRect());
 }
 
@@ -205,42 +218,44 @@ MultipartImage::OnImageDataAvailable(nsI
                                      uint32_t aCount)
 {
   // Note that this method is special in that we forward it to the next part if
   // one exists, and *not* the current part.
 
   // 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
 MultipartImage::OnImageDataComplete(nsIRequest* aRequest,
                                     nsISupports* aContext,
                                     nsresult aStatus,
                                     bool aLastPart)
 {
   // Note that this method is special in that we forward it to the next part if
   // one exists, and *not* the current part.
 
   // 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
 MultipartImage::Notify(int32_t aType, const nsIntRect* aRect /* = nullptr*/)
 {
   if (aType == imgINotificationObserver::SIZE_AVAILABLE) {
     mTracker->SyncNotifyProgress(FLAG_SIZE_AVAILABLE);
   } else if (aType == imgINotificationObserver::FRAME_UPDATE) {
--- a/image/test/mochitest/test_bug733553.html
+++ b/image/test/mochitest/test_bug733553.html
@@ -31,21 +31,25 @@ var testParts = [
   [177, "shaver.png"],
   [1, "red.png"],
   [80, "damon.jpg"],
   [80, "damon.jpg"],
   [80, "damon.jpg"],
   // 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"]
 ];
 
 // We'll append the part number to this, and tell the informant
 const BASE_URL = "bug733553-informant.sjs?";
 
 function initializeOnload() {
   var firstimg = document.createElement('img');