Bug 1174923 - Stop delaying the document load event until images are decoded. r=tn a=kwierso

dom/base/nsImageLoadingContent.cpp

@@ -87,7 +87,6 @@ nsImageLoadingContent::nsImageLoadingContent()
     mBroken(true),
     mUserDisabled(false),
     mSuppressed(false),
-    mFireEventsOnDecode(false),
     mNewRequestsWillNeedAnimationReset(false),
     mStateChangerDepth(0),
     mCurrentRequestRegistered(false),
@@ -186,18 +185,6 @@ nsImageLoadingContent::Notify(imgIRequest* aRequest,
   }
 
   if (aType == imgINotificationObserver::DECODE_COMPLETE) {
-    if (mFireEventsOnDecode) {
-      mFireEventsOnDecode = false;
-
-      uint32_t reqStatus;
-      aRequest->GetImageStatus(&reqStatus);
-      if (reqStatus & imgIRequest::STATUS_ERROR) {
-        FireEvent(NS_LITERAL_STRING("error"));
-      } else {
-        FireEvent(NS_LITERAL_STRING("load"));
-      }
-    }
-
     UpdateImageState(true);
   }
 
@@ -277,23 +264,11 @@ nsImageLoadingContent::OnLoadComplete(imgIRequest* aRequest, nsresult aStatus)
     }
   }
 
-  // We want to give the decoder a chance to find errors. If we haven't found
-  // an error yet and we've started decoding, either from the above
-  // StartDecoding or from some other place, we must only fire these events
-  // after we finish decoding.
-  uint32_t reqStatus;
-  aRequest->GetImageStatus(&reqStatus);
-  if (NS_SUCCEEDED(aStatus) && !(reqStatus & imgIRequest::STATUS_ERROR) &&
-      (reqStatus & imgIRequest::STATUS_DECODE_STARTED) &&
-      !(reqStatus & imgIRequest::STATUS_DECODE_COMPLETE)) {
-    mFireEventsOnDecode = true;
+  // Fire the appropriate DOM event.
+  if (NS_SUCCEEDED(aStatus)) {
+    FireEvent(NS_LITERAL_STRING("load"));
   } else {
-    // Fire the appropriate DOM event.
-    if (NS_SUCCEEDED(aStatus)) {
-      FireEvent(NS_LITERAL_STRING("load"));
-    } else {
-      FireEvent(NS_LITERAL_STRING("error"));
-    }
+    FireEvent(NS_LITERAL_STRING("error"));
   }
 
   nsCOMPtr<nsINode> thisNode = do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));

dom/base/nsImageLoadingContent.h

@@ -410,7 +410,6 @@ private:
   bool mBroken : 1;
   bool mUserDisabled : 1;
   bool mSuppressed : 1;
-  bool mFireEventsOnDecode : 1;
 
 protected:
   /**

image/Decoder.cpp

@@ -83,9 +83,8 @@ Decoder::Init()
   // No re-initializing
   MOZ_ASSERT(!mInitialized, "Can't re-initialize a decoder!");
 
-  // Fire OnStartDecode at init time to support bug 512435.
   if (!IsSizeDecode()) {
-      mProgress |= FLAG_DECODE_STARTED | FLAG_ONLOAD_BLOCKED;
+      mProgress |= FLAG_DECODE_STARTED;
   }
 
   // Implementation-specific initialization
@@ -273,7 +272,7 @@ Decoder::CompleteDecode()
     } else {
       // We're not usable. Record some final progress indicating the error.
       if (!IsSizeDecode()) {
-        mProgress |= FLAG_DECODE_COMPLETE | FLAG_ONLOAD_UNBLOCKED;
+        mProgress |= FLAG_DECODE_COMPLETE;
       }
       mProgress |= FLAG_HAS_ERROR;
     }
@@ -624,7 +623,7 @@ Decoder::PostFrameStop(Opacity aFrameOpacity    /* = Opacity::TRANSPARENT */,
 
   mCurrentFrame->Finish(aFrameOpacity, aDisposalMethod, aTimeout, aBlendMethod);
 
-  mProgress |= FLAG_FRAME_COMPLETE | FLAG_ONLOAD_UNBLOCKED;
+  mProgress |= FLAG_FRAME_COMPLETE;
 
   // If we're not sending partial invalidations, then we send an invalidation
   // here when the first frame is complete.

image/RasterImage.cpp

@@ -1211,11 +1211,9 @@ RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*, nsresult aStatus,
     // to draw. (We may have already sent some of these notifications from
     // NotifyForDecodeOnlyOnDraw(), but ProgressTracker will ensure no duplicate
     // notifications get sent.)
-    loadProgress |= FLAG_ONLOAD_BLOCKED |
-                    FLAG_DECODE_STARTED |
+    loadProgress |= FLAG_DECODE_STARTED |
                     FLAG_FRAME_COMPLETE |
-                    FLAG_DECODE_COMPLETE |
-                    FLAG_ONLOAD_UNBLOCKED;
+                    FLAG_DECODE_COMPLETE;
   }
 
   // Notify our listeners, which will fire this image's load event.
@@ -1234,7 +1232,7 @@ RasterImage::NotifyForDecodeOnlyOnDraw()
     return;
   }
 
-  NotifyProgress(FLAG_DECODE_STARTED | FLAG_ONLOAD_BLOCKED);
+  NotifyProgress(FLAG_DECODE_STARTED);
 }
 
 nsresult

image/test/browser/browser_bug666317.js

@@ -41,6 +41,23 @@ function forceDecodeImg() {
   ctx.drawImage(img, 0, 0);
 }
 
+function runAfterAsyncEvents(aCallback) {
+  function handlePostMessage(aEvent) {
+    if (aEvent.data == 'next') {
+      window.removeEventListener('message', handlePostMessage, false);
+      aCallback();
+    }
+  }
+
+  window.addEventListener('message', handlePostMessage, false);
+
+  // We'll receive the 'message' event after everything else that's currently in
+  // the event queue (which is a stronger guarantee than setTimeout, because
+  // setTimeout events may be coalesced). This lets us ensure that we run
+  // aCallback *after* any asynchronous events are delivered.
+  window.postMessage('next', '*');
+}
+
 function test() {
   // Enable the discarding pref.
   oldDiscardingPref = prefBranch.getBoolPref('discardable');
@@ -72,6 +89,13 @@ function step2() {
 
   // Check that the image is decoded.
   forceDecodeImg();
+
+  // The FRAME_COMPLETE notification is delivered asynchronously, so continue
+  // after we're sure it has been delivered.
+  runAfterAsyncEvents(() => step3(result, scriptedObserver, clonedRequest));
+}
+
+function step3(result, scriptedObserver, clonedRequest) {
   ok(isImgDecoded(), 'Image should initially be decoded.');
 
   // Focus the old tab, then fire a memory-pressure notification.  This should
@@ -81,18 +105,12 @@ function step2() {
              .getService(Ci.nsIObserverService);
   os.notifyObservers(null, 'memory-pressure', 'heap-minimize');
 
-  // The discard notification is delivered asynchronously, so pump the event
-  // loop before checking.
-  window.addEventListener('message', function (event) {
-    if (event.data == 'step3') {
-      step3(result, scriptedObserver, clonedRequest);
-    }
-  }, false);
-
-  window.postMessage('step3', '*');
+  // The DISCARD notification is delivered asynchronously, so continue after
+  // we're sure it has been delivered.
+  runAfterAsyncEvents(() => step4(result, scriptedObserver, clonedRequest));
 }
 
-function step3(result, scriptedObserver, clonedRequest) {
+function step4(result, scriptedObserver, clonedRequest) {
   ok(result.wasDiscarded, 'Image should be discarded.');
 
   // And we're done.

image/test/mochitest/mochitest.ini

@@ -70,7 +70,6 @@ skip-if = (toolkit == 'android' && processor == 'x86') #x86 only
 [test_bug496292.html]
 [test_bug497665.html]
 skip-if = (toolkit == 'android' && processor == 'x86') #x86 only
-[test_bug512435.html]
 [test_bug552605-1.html]
 [test_bug552605-2.html]
 [test_bug553982.html]

image/test/mochitest/test_bug512435.html

@@ -1,49 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=512435
--->
-<head>
-  <title>Test for Bug 512435</title>
-  <script type="application/javascript" src="/MochiKit/MochiKit.js"></script>
-  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <script type="text/javascript" src="/tests/image/test/mochitest/imgutils.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
-</head>
-<body>
-<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=512435">Mozilla Bug 512435</a>
-<img id="img_a">
-<img id="img_b">
-</div>
-<pre id="test">
-<script type="application/javascript">
-
-// Boilerplate
-const Ci = SpecialPowers.Ci;
-const Cc = SpecialPowers.Cc;
-SimpleTest.waitForExplicitFinish();
-
-// We're relying on very particular behavior for certain images - clear the
-// image cache, _then_ set src
-clearImageCache();
-document.getElementById("img_a").src = "damon.jpg";
-document.getElementById("img_b").src = "shaver.png";
-
-// Our handler
-function loadHandler() {
-
-  // The two images should be decoded
-  ok(isFrameDecoded("img_a"), "img_a should be decoded before onload fires");
-  ok(isFrameDecoded("img_b"), "img_b should be decoded before onload fires");
-
-  // All done
-  SimpleTest.finish();
-}
-
-// Set our onload handler
-window.onload = loadHandler;
-
-</script>
-</pre>
-</body>
-</html>

layout/reftests/css-break/reftest.list

@@ -6,4 +6,4 @@ fuzzy(16,460) fuzzy-if(Android,10,3667) == box-decoration-break-with-outset-box-
 random-if(!gtkWidget) HTTP(..) == box-decoration-break-border-image.html box-decoration-break-border-image-ref.html
 == box-decoration-break-block-border-padding.html box-decoration-break-block-border-padding-ref.html
 == box-decoration-break-block-margin.html box-decoration-break-block-margin-ref.html
-fuzzy-if(Android,8,6627) == box-decoration-break-first-letter.html box-decoration-break-first-letter-ref.html
+fuzzy-if(!Android,1,5) fuzzy-if(Android,8,6627) == box-decoration-break-first-letter.html box-decoration-break-first-letter-ref.html