Bug 1462272: Introduce nsImageFrame::GetCurrentRequest. r?dholbert draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 16 May 2018 20:02:47 +0200
changeset 796231 160c2379a949df2f49eda5c97192309f2feda775
parent 796230 6e173ec34f57b42218b8ad8819ea67e80f6b9ff7
child 796232 1e56644b7824e870ca23a842f2f5a8e56c9759d2
push id110185
push userbmo:emilio@crisal.io
push dateThu, 17 May 2018 09:02:34 +0000
reviewersdholbert
bugs1462272
milestone62.0a1
Bug 1462272: Introduce nsImageFrame::GetCurrentRequest. r?dholbert MozReview-Commit-ID: IXXtYClyY2z
dom/base/nsIImageLoadingContent.idl
dom/base/nsImageLoadingContent.cpp
layout/generic/nsImageFrame.cpp
layout/generic/nsImageFrame.h
--- a/dom/base/nsIImageLoadingContent.idl
+++ b/dom/base/nsIImageLoadingContent.idl
@@ -109,21 +109,16 @@ interface nsIImageLoadingContent : imgIN
    * security policies enforced.
    *
    * @param aContentDecision the decision returned from nsIContentPolicy
    *                         (any of the types REJECT_*)
    */
   [notxpcom, nostdcall] void setBlockedRequest(in int16_t aContentDecision);
 
   /**
-   * @return true if the current request's size is available.
-   */
-  [noscript, notxpcom] boolean currentRequestHasSize();
-
-  /**
    * Used to notify the image loading content node that a frame has been
    * created.
    */
   [notxpcom] void frameCreated(in nsIFrame aFrame);
 
   /**
    * Used to notify the image loading content node that a frame has been
    * destroyed.
--- a/dom/base/nsImageLoadingContent.cpp
+++ b/dom/base/nsImageLoadingContent.cpp
@@ -624,22 +624,16 @@ nsImageLoadingContent::GetRequest(int32_
   NS_ENSURE_ARG_POINTER(aRequest);
 
   ErrorResult result;
   *aRequest = GetRequest(aRequestType, result).take();
 
   return result.StealNSResult();
 }
 
-NS_IMETHODIMP_(bool)
-nsImageLoadingContent::CurrentRequestHasSize()
-{
-  return HaveSize(mCurrentRequest);
-}
-
 NS_IMETHODIMP_(void)
 nsImageLoadingContent::FrameCreated(nsIFrame* aFrame)
 {
   NS_ASSERTION(aFrame, "aFrame is null");
 
   TrackImage(mCurrentRequest, aFrame);
   TrackImage(mPendingRequest, aFrame);
 
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -268,26 +268,21 @@ nsImageFrame::Init(nsIContent*       aCo
 
   nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(aContent);
   if (!imageLoader) {
     MOZ_CRASH("Why do we have an nsImageFrame here at all?");
   }
 
   imageLoader->AddNativeObserver(mListener);
 
-  // We have a PresContext now, so we need to notify the image content node
-  // that it can register images.
+  // We have a PresContext now, so we need to notify the image content node that
+  // it can register images.
   imageLoader->FrameCreated(this);
 
-  // Give image loads associated with an image frame a small priority boost!
-  nsCOMPtr<imgIRequest> currentRequest;
-  imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                          getter_AddRefs(currentRequest));
-
-  if (currentRequest) {
+  if (nsCOMPtr<imgIRequest> currentRequest = GetCurrentRequest()) {
     uint32_t categoryToBoostPriority = imgIRequest::CATEGORY_FRAME_INIT;
 
     // Increase load priority further if intrinsic size might be important for layout.
     if (!HaveSpecifiedSize(StylePosition())) {
       categoryToBoostPriority |= imgIRequest::CATEGORY_SIZE_QUERY;
     }
 
     currentRequest->BoostPriority(categoryToBoostPriority);
@@ -828,36 +823,32 @@ nsImageFrame::EnsureIntrinsicSizeAndRati
     if (mImage) {
       UpdateIntrinsicSize(mImage);
       UpdateIntrinsicRatio(mImage);
     } else {
       // image request is null or image size not known, probably an
       // invalid image specified
       if (!(GetStateBits() & NS_FRAME_GENERATED_CONTENT)) {
         bool imageInvalid = false;
+
         // check for broken images. valid null images (eg. img src="") are
         // not considered broken because they have no image requests
-        nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
-        if (imageLoader) {
-          nsCOMPtr<imgIRequest> currentRequest;
-          imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                                  getter_AddRefs(currentRequest));
-          if (currentRequest) {
-            uint32_t imageStatus;
-            imageInvalid =
-              NS_SUCCEEDED(currentRequest->GetImageStatus(&imageStatus)) &&
-              (imageStatus & imgIRequest::STATUS_ERROR);
-          } else {
-            // check if images are user-disabled (or blocked for other
-            // reasons)
-            int16_t imageBlockingStatus;
-            imageLoader->GetImageBlockingStatus(&imageBlockingStatus);
-            imageInvalid = imageBlockingStatus != nsIContentPolicy::ACCEPT;
-          }
+        if (nsCOMPtr<imgIRequest> currentRequest = GetCurrentRequest()) {
+          uint32_t imageStatus;
+          imageInvalid =
+            NS_SUCCEEDED(currentRequest->GetImageStatus(&imageStatus)) &&
+            (imageStatus & imgIRequest::STATUS_ERROR);
+        } else if (nsCOMPtr<nsIImageLoadingContent> loader = do_QueryInterface(mContent)) {
+          // check if images are user-disabled (or blocked for other
+          // reasons)
+          int16_t imageBlockingStatus;
+          loader->GetImageBlockingStatus(&imageBlockingStatus);
+          imageInvalid = imageBlockingStatus != nsIContentPolicy::ACCEPT;
         }
+
         // invalid image specified. make the image big enough for the "broken" icon
         if (imageInvalid) {
           nscoord edgeLengthToUse =
             nsPresContext::CSSPixelsToAppUnits(
               ICON_SIZE + (2 * (ICON_PADDING + ALT_BORDER_WIDTH)));
           mIntrinsicSize.width.SetCoordValue(edgeLengthToUse);
           mIntrinsicSize.height.SetCoordValue(edgeLengthToUse);
           mIntrinsicRatio.SizeTo(1, 1);
@@ -875,32 +866,32 @@ nsImageFrame::ComputeSize(gfxContext *aR
                           nscoord aAvailableISize,
                           const LogicalSize& aMargin,
                           const LogicalSize& aBorder,
                           const LogicalSize& aPadding,
                           ComputeSizeFlags aFlags)
 {
   EnsureIntrinsicSizeAndRatio();
 
-  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
-  NS_ASSERTION(imageLoader, "No content node??");
   mozilla::IntrinsicSize intrinsicSize(mIntrinsicSize);
 
+  nsCOMPtr<imgIRequest> currentRequest = GetCurrentRequest();
+
   // XXX(seth): We may sometimes find ourselves in the situation where we have
   // mImage, but imageLoader's current request does not have a size yet.
   // This can happen when we load an image speculatively from cache, it fails
   // to validate, and the new image load hasn't fired SIZE_AVAILABLE yet. In
   // this situation we should always use mIntrinsicSize, because
   // GetNaturalWidth/Height will return 0, so we check CurrentRequestHasSize()
   // below. See bug 1019840. We will fix this in bug 1141395.
 
   // Content may override our default dimensions. This is termed as overriding
   // the intrinsic size by the spec, but all other consumers of mIntrinsic*
   // values are being used to refer to the real/true size of the image data.
-  if (imageLoader && imageLoader->CurrentRequestHasSize() && mImage &&
+  if (currentRequest && SizeIsAvailable(currentRequest) && mImage &&
       intrinsicSize.width.GetUnit() == eStyleUnit_Coord &&
       intrinsicSize.height.GetUnit() == eStyleUnit_Coord) {
     int32_t width = 0;
     int32_t height = 0;
     Unused << mImage->GetWidth(&width);
     Unused << mImage->GetHeight(&height);
     nsSize size(nsPresContext::CSSPixelsToAppUnits(width),
                 nsPresContext::CSSPixelsToAppUnits(height));
@@ -1029,26 +1020,20 @@ nsImageFrame::Reflow(nsPresContext*     
     aMetrics.Height() -= y + aReflowInput.ComputedPhysicalBorderPadding().top;
     aMetrics.Height() = std::max(0, aMetrics.Height());
   }
 
 
   // we have to split images if we are:
   //  in Paginated mode, we need to have a constrained height, and have a height larger than our available height
   uint32_t loadStatus = imgIRequest::STATUS_NONE;
-  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
-  NS_ASSERTION(imageLoader, "No content node??");
-  if (imageLoader) {
-    nsCOMPtr<imgIRequest> currentRequest;
-    imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                            getter_AddRefs(currentRequest));
-    if (currentRequest) {
-      currentRequest->GetImageStatus(&loadStatus);
-    }
+  if (nsCOMPtr<imgIRequest> currentRequest = GetCurrentRequest()) {
+    currentRequest->GetImageStatus(&loadStatus);
   }
+
   if (aPresContext->IsPaginated() &&
       ((loadStatus & imgIRequest::STATUS_SIZE_AVAILABLE) || (mState & IMAGE_SIZECONSTRAINED)) &&
       NS_UNCONSTRAINEDSIZE != aReflowInput.AvailableHeight() &&
       aMetrics.Height() > aReflowInput.AvailableHeight()) {
     // our desired height was greater than 0, so to avoid infinite
     // splitting, use 1 pixel as the min
     aMetrics.Height() = std::max(nsPresContext::CSSPixelsToAppUnits(1), aReflowInput.AvailableHeight());
     aStatus.SetIncomplete();
@@ -1817,16 +1802,30 @@ nsImageFrame::PaintImage(gfxContext& aRe
     mPrevImage = aImage;
   } else if (result == ImgDrawResult::BAD_IMAGE) {
     mPrevImage = nullptr;
   }
 
   return result;
 }
 
+already_AddRefed<imgIRequest>
+nsImageFrame::GetCurrentRequest() const
+{
+  nsCOMPtr<imgIRequest> request;
+
+  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
+  MOZ_ASSERT(imageLoader);
+
+  imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
+                          getter_AddRefs(request));
+
+  return request.forget();
+}
+
 void
 nsImageFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
                                const nsDisplayListSet& aLists)
 {
   if (!IsVisibleForPainting(aBuilder))
     return;
 
   DisplayBorderBackgroundOutline(aBuilder, aLists);
@@ -1834,28 +1833,21 @@ nsImageFrame::BuildDisplayList(nsDisplay
   uint32_t clipFlags =
     nsStyleUtil::ObjectPropsMightCauseOverflow(StylePosition()) ?
     0 : DisplayListClipState::ASSUME_DRAWING_RESTRICTED_TO_CONTENT_RECT;
 
   DisplayListClipState::AutoClipContainingBlockDescendantsToContentBox
     clip(aBuilder, this, clipFlags);
 
   if (mComputedSize.width != 0 && mComputedSize.height != 0) {
-    nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
-    NS_ASSERTION(imageLoader, "Not an image loading content?");
-
-    nsCOMPtr<imgIRequest> currentRequest;
-    if (imageLoader) {
-      imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                              getter_AddRefs(currentRequest));
-    }
-
     EventStates contentState = mContent->AsElement()->State();
     bool imageOK = IMAGE_OK(contentState, true);
 
+    nsCOMPtr<imgIRequest> currentRequest = GetCurrentRequest();
+
     // XXX(seth): The SizeIsAvailable check here should not be necessary - the
     // intention is that a non-null mImage means we have a size, but there is
     // currently some code that violates this invariant.
     if (!imageOK || !mImage || !SizeIsAvailable(currentRequest)) {
       // No image yet, or image load failed. Draw the alt-text and an icon
       // indicating the status
       aLists.Content()->AppendToTop(
         MakeDisplayItem<nsDisplayAltFeedback>(aBuilder, this));
@@ -2182,28 +2174,22 @@ nsImageFrame::GetFrameName(nsAString& aR
 
 void
 nsImageFrame::List(FILE* out, const char* aPrefix, uint32_t aFlags) const
 {
   nsCString str;
   ListGeneric(str, aPrefix, aFlags);
 
   // output the img src url
-  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
-  if (imageLoader) {
-    nsCOMPtr<imgIRequest> currentRequest;
-    imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                            getter_AddRefs(currentRequest));
-    if (currentRequest) {
-      nsCOMPtr<nsIURI> uri;
-      currentRequest->GetURI(getter_AddRefs(uri));
-      nsAutoCString uristr;
-      uri->GetAsciiSpec(uristr);
-      str += nsPrintfCString(" [src=%s]", uristr.get());
-    }
+  if (nsCOMPtr<imgIRequest> currentRequest = GetCurrentRequest()) {
+    nsCOMPtr<nsIURI> uri;
+    currentRequest->GetURI(getter_AddRefs(uri));
+    nsAutoCString uristr;
+    uri->GetAsciiSpec(uristr);
+    str += nsPrintfCString(" [src=%s]", uristr.get());
   }
   fprintf_stderr(out, "%s\n", str.get());
 }
 #endif
 
 nsIFrame::LogicalSides
 nsImageFrame::GetLogicalSkipSides(const ReflowInput* aReflowInput) const
 {
--- a/layout/generic/nsImageFrame.h
+++ b/layout/generic/nsImageFrame.h
@@ -133,16 +133,17 @@ public:
   static void ReleaseGlobals() {
     if (gIconLoad) {
       gIconLoad->Shutdown();
       gIconLoad = nullptr;
     }
     NS_IF_RELEASE(sIOService);
   }
 
+  already_AddRefed<imgIRequest> GetCurrentRequest() const;
   nsresult Notify(imgIRequest *aRequest, int32_t aType, const nsIntRect* aData);
 
   /**
    * Function to test whether aContent, which has aComputedStyle as its style,
    * should get an image frame.  Note that this method is only used by the
    * frame constructor; it's only here because it uses gIconLoad for now.
    */
   static bool ShouldCreateImageFrameFor(mozilla::dom::Element* aElement,