Bug 1457532 Part 1: Narrow ImageLoader::OnLoadComplete to only unblock onload in response to an error status on the image request. draft
authorBrad Werth <bwerth@mozilla.com>
Fri, 27 Apr 2018 12:00:35 -0700
changeset 795359 0c373e28eeb08ed993bbf40106c3328bfb0c8a13
parent 795256 cf3ee14023483cbbb57129479537c713e22c1980
child 795360 af903de736aaed31e911258bae4f05359ad770ae
child 795481 8862da462be03265f55c4ee81a6a0e9ac17efbff
push id109951
push userbwerth@mozilla.com
push dateTue, 15 May 2018 18:40:57 +0000
bugs1457532
milestone62.0a1
Bug 1457532 Part 1: Narrow ImageLoader::OnLoadComplete to only unblock onload in response to an error status on the image request. MozReview-Commit-ID: 929PTrT9iP1
layout/style/ImageLoader.cpp
layout/style/ImageLoader.h
--- a/layout/style/ImageLoader.cpp
+++ b/layout/style/ImageLoader.cpp
@@ -92,30 +92,35 @@ ImageLoader::AssociateRequestToFrame(img
   }
 
   // Check if the frame requires special processing.
   if (aFlags & REQUEST_REQUIRES_REFLOW) {
     fwfToModify->mFlags |= REQUEST_REQUIRES_REFLOW;
 
     // If we weren't already blocking onload, do that now.
     if ((fwfToModify->mFlags & REQUEST_HAS_BLOCKED_ONLOAD) == 0) {
-      fwfToModify->mFlags |= REQUEST_HAS_BLOCKED_ONLOAD;
-
-      // Block document onload until we either remove the frame in
-      // RemoveRequestToFrameMapping or onLoadComplete, or complete a reflow.
-      mDocument->BlockOnload();
+      // Get request status to see if we should block onload, and if we can
+      // request reflow immediately.
+      uint32_t status = 0;
+      if (NS_SUCCEEDED(aRequest->GetImageStatus(&status)) &&
+          !(status & imgIRequest::STATUS_ERROR)) {
+        // No error, so we can block onload.
+        fwfToModify->mFlags |= REQUEST_HAS_BLOCKED_ONLOAD;
 
-      // We need to stay blocked until we get a reflow. If the first frame
-      // is not yet decoded, we'll trigger that reflow from onFrameComplete.
-      // But if the first frame is already decoded, we need to trigger that
-      // reflow now, because we'll never get a call to onFrameComplete.
-      uint32_t status = 0;
-      if(NS_SUCCEEDED(aRequest->GetImageStatus(&status)) &&
-         status & imgIRequest::STATUS_FRAME_COMPLETE) {
-        RequestReflowOnFrame(fwfToModify, aRequest);
+        // Block document onload until we either remove the frame in
+        // RemoveRequestToFrameMapping or onLoadComplete, or complete a reflow.
+        mDocument->BlockOnload();
+
+        // We need to stay blocked until we get a reflow. If the first frame
+        // is not yet decoded, we'll trigger that reflow from onFrameComplete.
+        // But if the first frame is already decoded, we need to trigger that
+        // reflow now, because we'll never get a call to onFrameComplete.
+        if(status & imgIRequest::STATUS_FRAME_COMPLETE) {
+          RequestReflowOnFrame(fwfToModify, aRequest);
+        }
       }
     }
   }
 
   // Do some sanity checking to ensure that we only add to one mapping
   // iff we also add to the other mapping.
   DebugOnly<bool> didAddToFrameSet(false);
   DebugOnly<bool> didAddToRequestSet(false);
@@ -486,20 +491,16 @@ ImageLoader::RequestReflowIfNeeded(Frame
       RequestReflowOnFrame(&fwf, aRequest);
     }
   }
 }
 
 void
 ImageLoader::RequestReflowOnFrame(FrameWithFlags* aFwf, imgIRequest* aRequest)
 {
-  // Set the flag indicating that we've requested reflow. This flag will never
-  // be unset.
-  aFwf->mFlags |= REQUEST_HAS_REQUESTED_REFLOW;
-
   nsIFrame* frame = aFwf->mFrame;
 
   // Actually request the reflow.
   nsIFrame* parent = frame->GetInFlowParent();
   parent->PresShell()->FrameNeedsReflow(parent, nsIPresShell::eStyleChange,
                                         NS_FRAME_IS_DIRTY);
 
   // We'll respond to the reflow events by unblocking onload, regardless
@@ -662,29 +663,29 @@ ImageLoader::OnLoadComplete(imgIRequest*
     return NS_OK;
   }
 
   FrameSet* frameSet = mRequestToFrameMap.Get(aRequest);
   if (!frameSet) {
     return NS_OK;
   }
 
-  // This may be called for a request that never sent a complete frame.
-  // This is what happens in a CORS mode violation, and may happen during
-  // other network events. We check for any frames that have blocked
-  // onload but haven't requested reflow. In such a case, we unblock
-  // onload here, since onFrameComplete will not be called for this request.
-  FrameFlags flagsToCheck(REQUEST_HAS_BLOCKED_ONLOAD |
-                          REQUEST_HAS_REQUESTED_REFLOW);
-  for (FrameWithFlags& fwf : *frameSet) {
-    if ((fwf.mFlags & flagsToCheck) == REQUEST_HAS_BLOCKED_ONLOAD) {
-      // We've blocked onload but haven't requested reflow. Unblock onload
-      // and clear the flag.
-      mDocument->UnblockOnload(false);
-      fwf.mFlags &= ~REQUEST_HAS_BLOCKED_ONLOAD;
+  // Check if aRequest has an error state. If it does, we need to unblock
+  // Document onload for all the frames associated with this request that
+  // have blocked onload. This is what happens in a CORS mode violation, and
+  // may happen during other network events.
+  uint32_t status = 0;
+  if(NS_SUCCEEDED(aRequest->GetImageStatus(&status)) &&
+     status & imgIRequest::STATUS_ERROR) {
+    for (FrameWithFlags& fwf : *frameSet) {
+      if (fwf.mFlags & REQUEST_HAS_BLOCKED_ONLOAD) {
+        // We've blocked onload. Unblock onload and clear the flag.
+        mDocument->UnblockOnload(false);
+        fwf.mFlags &= ~REQUEST_HAS_BLOCKED_ONLOAD;
+      }
     }
   }
 
   return NS_OK;
 }
 
 void
 ImageLoader::FlushUseCounters()
--- a/layout/style/ImageLoader.h
+++ b/layout/style/ImageLoader.h
@@ -35,18 +35,17 @@ struct ImageValue;
 class ImageLoader final : public imgINotificationObserver
 {
 public:
   // We also associate flags alongside frames in the request-to-frames hashmap.
   // These are used for special handling of events for requests.
   typedef uint32_t FrameFlags;
   enum {
     REQUEST_REQUIRES_REFLOW      = 1u << 0,
-    REQUEST_HAS_REQUESTED_REFLOW = 1u << 1,
-    REQUEST_HAS_BLOCKED_ONLOAD   = 1u << 2
+    REQUEST_HAS_BLOCKED_ONLOAD   = 1u << 1,
   };
 
   typedef mozilla::css::ImageValue Image;
 
   explicit ImageLoader(nsIDocument* aDocument)
   : mDocument(aDocument),
     mInClone(false)
   {