Bug 1174923 - Stop delaying the document load event until images are decoded. r=tn a=kwierso
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 19 Jun 2015 15:55:12 -0700
changeset 280622 a79cb8e688cd19e6bc8971f87ccad559a154b112
parent 280621 51168e14388c3cb81e014b0d6a7dac3ebfe92b89
child 280623 2afd5728f13aebe9c2daa85dd831b4e4ac2ac121
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, kwierso
bugs1174923
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1174923 - Stop delaying the document load event until images are decoded. r=tn a=kwierso
dom/base/nsImageLoadingContent.cpp
dom/base/nsImageLoadingContent.h
image/Decoder.cpp
image/RasterImage.cpp
image/test/browser/browser_bug666317.js
image/test/mochitest/mochitest.ini
image/test/mochitest/test_bug512435.html
layout/reftests/css-break/reftest.list
--- a/dom/base/nsImageLoadingContent.cpp
+++ b/dom/base/nsImageLoadingContent.cpp
@@ -82,17 +82,16 @@ nsImageLoadingContent::nsImageLoadingCon
     mImageBlockingStatus(nsIContentPolicy::ACCEPT),
     mLoadingEnabled(true),
     mIsImageStateForced(false),
     mLoading(false),
     // mBroken starts out true, since an image without a URI is broken....
     mBroken(true),
     mUserDisabled(false),
     mSuppressed(false),
-    mFireEventsOnDecode(false),
     mNewRequestsWillNeedAnimationReset(false),
     mStateChangerDepth(0),
     mCurrentRequestRegistered(false),
     mPendingRequestRegistered(false),
     mFrameCreateCalled(false),
     mVisibleCount(0)
 {
   if (!nsContentUtils::GetImgLoaderForChannel(nullptr, nullptr)) {
@@ -181,28 +180,16 @@ nsImageLoadingContent::Notify(imgIReques
       }
     }
     nsresult status =
         reqStatus & imgIRequest::STATUS_ERROR ? NS_ERROR_FAILURE : NS_OK;
     return OnLoadComplete(aRequest, status);
   }
 
   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);
   }
 
   return NS_OK;
 }
 
 nsresult
 nsImageLoadingContent::OnLoadComplete(imgIRequest* aRequest, nsresult aStatus)
@@ -272,33 +259,21 @@ nsImageLoadingContent::OnLoadComplete(im
       // visible.
       if (!mFrameCreateCalled || (f->GetStateBits() & NS_FRAME_FIRST_REFLOW) ||
           mVisibleCount > 0 || shell->AssumeAllImagesVisible()) {
         mCurrentRequest->StartDecoding();
       }
     }
   }
 
-  // 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));
   nsSVGEffects::InvalidateDirectRenderingObservers(thisNode->AsElement());
 
   return NS_OK;
 }
 
--- a/dom/base/nsImageLoadingContent.h
+++ b/dom/base/nsImageLoadingContent.h
@@ -405,17 +405,16 @@ private:
   /**
    * The state we had the last time we checked whether we needed to notify the
    * document of a state change.  These are maintained by UpdateImageState.
    */
   bool mLoading : 1;
   bool mBroken : 1;
   bool mUserDisabled : 1;
   bool mSuppressed : 1;
-  bool mFireEventsOnDecode : 1;
 
 protected:
   /**
    * A hack to get animations to reset, see bug 594771. On requests
    * that originate from setting .src, we mark them for needing their animation
    * reset when they are ready. mNewRequestsWillNeedAnimationReset is set to
    * true while preparing such requests (as a hack around needing to change an
    * interface), and the other two booleans store which of the current
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -78,19 +78,18 @@ Decoder::~Decoder()
  */
 
 void
 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
   InitInternal();
 
   mInitialized = true;
 }
 
@@ -268,17 +267,17 @@ Decoder::CompleteDecode()
 
       if (mInFrame) {
         PostFrameStop();
       }
       PostDecodeDone();
     } 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;
     }
   }
 }
 
 void
 Decoder::Finish()
@@ -619,17 +618,17 @@ Decoder::PostFrameStop(Opacity aFrameOpa
   MOZ_ASSERT(mInFrame, "Stopping frame when we didn't start one");
   MOZ_ASSERT(mCurrentFrame, "Stopping frame when we don't have one");
 
   // Update our state
   mInFrame = false;
 
   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.
   if (!mSendPartialInvalidations && !mIsAnimated) {
     mInvalidRect.UnionRect(mInvalidRect,
                            gfx::IntRect(gfx::IntPoint(0, 0), GetSize()));
   }
 }
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1206,21 +1206,19 @@ RasterImage::OnImageDataComplete(nsIRequ
   Progress loadProgress = LoadCompleteProgress(aLastPart, mError, finalStatus);
 
   if (mDecodeOnlyOnDraw) {
     // For decode-only-on-draw images, we want to send notifications as if we've
     // already finished decoding. Otherwise some observers will never even try
     // 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.
   NotifyProgress(loadProgress);
 
   return finalStatus;
 }
 
@@ -1229,17 +1227,17 @@ RasterImage::NotifyForDecodeOnlyOnDraw()
 {
   if (!NS_IsMainThread()) {
     nsCOMPtr<nsIRunnable> runnable =
       NS_NewRunnableMethod(this, &RasterImage::NotifyForDecodeOnlyOnDraw);
     NS_DispatchToMainThread(runnable);
     return;
   }
 
-  NotifyProgress(FLAG_DECODE_STARTED | FLAG_ONLOAD_BLOCKED);
+  NotifyProgress(FLAG_DECODE_STARTED);
 }
 
 nsresult
 RasterImage::OnImageDataAvailable(nsIRequest*,
                                   nsISupports*,
                                   nsIInputStream* aInStr,
                                   uint64_t aOffset,
                                   uint32_t aCount)
--- a/image/test/browser/browser_bug666317.js
+++ b/image/test/browser/browser_bug666317.js
@@ -36,16 +36,33 @@ function isImgDecoded() {
 function forceDecodeImg() {
   let doc = gBrowser.getBrowserForTab(newTab).contentWindow.document;
   let img = doc.getElementById('testImg');
   let canvas = doc.createElement('canvas');
   let ctx = canvas.getContext('2d');
   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');
   prefBranch.setBoolPref('discardable', true);
 
   // Create and focus a new tab.
   oldTab = gBrowser.selectedTab;
   newTab = gBrowser.addTab('data:text/html,' + pageSource);
@@ -67,37 +84,38 @@ function step2() {
                            .createScriptedObserver(observer);
 
   // Clone the current imgIRequest with our new observer.
   var request = currentRequest();
   var clonedRequest = request.clone(scriptedObserver);
 
   // 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
   // cause the decoded image in the new tab to be discarded.
   gBrowser.selectedTab = oldTab;
   var os = Cc["@mozilla.org/observer-service;1"]
              .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.
   gBrowser.removeTab(newTab);
   prefBranch.setBoolPref('discardable', oldDiscardingPref);
   clonedRequest.cancelAndForgetObserver(0);
   finish();
 }
--- a/image/test/mochitest/mochitest.ini
+++ b/image/test/mochitest/mochitest.ini
@@ -65,17 +65,16 @@ support-files =
 [test_bug468160.html]
 # [test_bug478398.html]
 # disabled - See bug 579139
 [test_bug490949.html]
 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]
 [test_bug601470.html]
 skip-if = (toolkit == 'android' && processor == 'x86') #x86 only
 [test_bug614392.html]
 [test_bug657191.html]
 [test_bug671906.html]
deleted file mode 100644
--- a/image/test/mochitest/test_bug512435.html
+++ /dev/null
@@ -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>
--- a/layout/reftests/css-break/reftest.list
+++ b/layout/reftests/css-break/reftest.list
@@ -1,9 +1,9 @@
 default-preferences pref(layout.css.box-decoration-break.enabled,true)
 
 == box-decoration-break-1.html box-decoration-break-1-ref.html
 fuzzy(1,20) == box-decoration-break-with-inset-box-shadow-1.html box-decoration-break-with-inset-box-shadow-1-ref.html
 fuzzy(16,460) fuzzy-if(Android,10,3667) == box-decoration-break-with-outset-box-shadow-1.html box-decoration-break-with-outset-box-shadow-1-ref.html
 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