Bug 1019840 - Use cached intrinsic size in nsImageFrame::ComputeSize unless the image loader has a size. r=tn, a=sledru
authorSeth Fowler <seth@mozilla.com>
Wed, 18 Mar 2015 18:29:32 -0700
changeset 258017 30fe8b0a6e06b3567ab63a1d0c163446b6b96161
parent 258016 813378228975c4d04793108a8cd20221eb924692
child 258018 3b636a032b240597871b2b09be0c3d89c8ae42cf
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, sledru
bugs1019840
milestone38.0a2
Bug 1019840 - Use cached intrinsic size in nsImageFrame::ComputeSize unless the image loader has a size. r=tn, a=sledru
dom/base/nsIImageLoadingContent.idl
dom/base/nsImageLoadingContent.cpp
layout/generic/nsImageFrame.cpp
--- a/dom/base/nsIImageLoadingContent.idl
+++ b/dom/base/nsIImageLoadingContent.idl
@@ -23,26 +23,21 @@ interface nsIFrame;
  * the load, firing any DOM events as needed.
  *
  * An implementation of this interface may support the concepts of a
  * "current" image and a "pending" image.  If it does, a request to change
  * the currently loaded image will start a "pending" request which will
  * become current only when the image is loaded.  It is the responsibility of
  * observers to check which request they are getting notifications for.
  *
- * Observers added in mid-load will not get any notifications they
- * missed.  We should NOT freeze this interface without considering
- * this issue.  (It could be that the image status on imgIRequest is
- * sufficient, when combined with the imageBlockingStatus information.)
- *
  * Please make sure to update the MozImageLoadingContent WebIDL
  * interface to mirror this interface when changing it.
  */
 
-[scriptable, builtinclass, uuid(5794d12b-3195-4526-a814-a2181f6c71fe)]
+[scriptable, builtinclass, uuid(770f7d84-c917-42d7-bf8d-d1b70649e733)]
 interface nsIImageLoadingContent : imgINotificationObserver
 {
   /**
    * Request types.  Image loading content nodes attempt to do atomic
    * image changes when the image url is changed.  This means that
    * when the url changes the new image load will start, but the old
    * image will remain the "current" request until the new image is
    * fully loaded.  At that point, the old "current" request will be
@@ -96,16 +91,21 @@ interface nsIImageLoadingContent : imgIN
    * is thrown)
    *
    * @throws NS_ERROR_UNEXPECTED if the request type requested is not
    * known
    */
   imgIRequest getRequest(in long aRequestType);
 
   /**
+   * @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
@@ -488,16 +488,22 @@ nsImageLoadingContent::GetRequest(int32_
   NS_ENSURE_ARG_POINTER(aRequest);
 
   ErrorResult result;
   *aRequest = GetRequest(aRequestType, result).take();
 
   return result.ErrorCode();
 }
 
+NS_IMETHODIMP_(bool)
+nsImageLoadingContent::CurrentRequestHasSize()
+{
+  return HaveSize(mCurrentRequest);
+}
+
 NS_IMETHODIMP_(void)
 nsImageLoadingContent::FrameCreated(nsIFrame* aFrame)
 {
   NS_ASSERTION(aFrame, "aFrame is null");
 
   mFrameCreateCalled = true;
 
   if (aFrame->HasAnyStateBits(NS_FRAME_IN_POPUP)) {
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -738,20 +738,28 @@ nsImageFrame::ComputeSize(nsRenderingCon
                           ComputeSizeFlags aFlags)
 {
   EnsureIntrinsicSizeAndRatio();
 
   nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
   NS_ASSERTION(imageLoader, "No content node??");
   mozilla::IntrinsicSize intrinsicSize(mIntrinsicSize);
 
+  // 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 && mImage &&
+  if (imageLoader && imageLoader->CurrentRequestHasSize() && mImage &&
       intrinsicSize.width.GetUnit() == eStyleUnit_Coord &&
       intrinsicSize.height.GetUnit() == eStyleUnit_Coord) {
     uint32_t width;
     uint32_t height;
     if (NS_SUCCEEDED(imageLoader->GetNaturalWidth(&width)) &&
         NS_SUCCEEDED(imageLoader->GetNaturalHeight(&height))) {
       nscoord appWidth = nsPresContext::CSSPixelsToAppUnits((int32_t)width);
       nscoord appHeight = nsPresContext::CSSPixelsToAppUnits((int32_t)height);