Bug 1527085 - Ensure we invalidate when the image request changes. r=jrmuizel

When the underlying image request (imgIRequest) changes for an image, we
need to ensure that we invalidate the cached WebRenderImageData such that
the image container stored therein is updated to be for the correct
image. This gets a little tricky because some display items store both
the current and previous images, and choose to display the latter if the
former is not yet ready. We also don't know what image the image
container belongs to. As such, we now compare the producer ID of the
current frame in the image container, to the expected producer ID of the
current image request. If they don't match, we must regenerate the
display list.

Differential Revision: https://phabricator.services.mozilla.com/D19699

gfx/layers/ImageContainer.h

@@ -387,8 +387,8 @@ class ImageContainer final : public SupportsWeakPtr<ImageContainer> {
    */
   explicit ImageContainer(const CompositableHandle& aHandle);
 
-  typedef uint32_t FrameID;
-  typedef uint32_t ProducerID;
+  typedef ContainerFrameID FrameID;
+  typedef ContainerProducerID ProducerID;
 
   RefPtr<PlanarYCbCrImage> CreatePlanarYCbCrImage();
 

gfx/layers/ImageTypes.h

@@ -7,6 +7,8 @@
 #ifndef GFX_IMAGETYPES_H
 #define GFX_IMAGETYPES_H
 
+#include <stdint.h>     // for uint32_t
+
 namespace mozilla {
 
 enum class ImageFormat {
@@ -106,6 +108,16 @@ enum class YUVColorSpace {
   UNKNOWN,
 };
 
+namespace layers {
+
+typedef uint32_t ContainerFrameID;
+constexpr ContainerFrameID kContainerFrameID_Invalid = 0;
+
+typedef uint32_t ContainerProducerID;
+constexpr ContainerProducerID kContainerProducerID_Invalid = 0;
+
+}  // namespace layers
+
 }  // namespace mozilla
 
 #endif

gfx/layers/ipc/SharedSurfacesChild.cpp

@@ -325,7 +325,8 @@ SharedSurfacesChild::AsSourceSurfaceSharedData(SourceSurface* aSurface) {
 
 /* static */ nsresult SharedSurfacesChild::Share(
     ImageContainer* aContainer, RenderRootStateManager* aManager,
-    wr::IpcResourceUpdateQueue& aResources, wr::ImageKey& aKey) {
+    wr::IpcResourceUpdateQueue& aResources, wr::ImageKey& aKey,
+    ContainerProducerID aProducerId) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aContainer);
   MOZ_ASSERT(aManager);
@@ -340,6 +341,15 @@ SharedSurfacesChild::AsSourceSurfaceSharedData(SourceSurface* aSurface) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
+  if (aProducerId != kContainerProducerID_Invalid &&
+      images[0].mProducerID != aProducerId) {
+    // If the producer ID of the surface in the container does not match the
+    // expected producer ID, then we do not want to proceed with sharing. This
+    // is useful for when callers are unsure if given container is for the same
+    // producer / underlying image request.
+    return NS_ERROR_FAILURE;
+  }
+
   RefPtr<gfx::SourceSurface> surface = images[0].mImage->GetAsSourceSurface();
   if (!surface) {
     return NS_ERROR_NOT_IMPLEMENTED;

gfx/layers/ipc/SharedSurfacesChild.h

@@ -15,6 +15,7 @@
 #include "mozilla/gfx/UserData.h"              // for UserDataKey
 #include "mozilla/webrender/WebRenderTypes.h"  // for wr::ImageKey
 #include "nsTArray.h"                          // for AutoTArray
+#include "ImageTypes.h"                        // for ContainerProducerID
 
 namespace mozilla {
 namespace layers {
@@ -91,7 +92,7 @@ class SharedSurfacesChild final {
   static nsresult Share(ImageContainer* aContainer,
                         RenderRootStateManager* aManager,
                         wr::IpcResourceUpdateQueue& aResources,
-                        wr::ImageKey& aKey);
+                        wr::ImageKey& aKey, ContainerProducerID aProducerId);
 
   /**
    * Get the external ID, if any, bound to the shared surface. Used for memory

gfx/layers/wr/WebRenderUserData.cpp

@@ -39,7 +39,7 @@ void WebRenderBackgroundData::AddWebRenderCommands(
 }
 
 /* static */ bool WebRenderUserData::ProcessInvalidateForImage(
-    nsIFrame* aFrame, DisplayItemType aType) {
+    nsIFrame* aFrame, DisplayItemType aType, ContainerProducerID aProducerId) {
   MOZ_ASSERT(aFrame);
 
   if (!aFrame->HasProperty(WebRenderUserDataProperty::Key())) {
@@ -57,7 +57,7 @@ void WebRenderBackgroundData::AddWebRenderCommands(
 
   RefPtr<WebRenderImageData> image =
       GetWebRenderUserData<WebRenderImageData>(aFrame, type);
-  if (image && image->UsingSharedSurface()) {
+  if (image && image->UsingSharedSurface(aProducerId)) {
     return true;
   }
 
@@ -107,7 +107,8 @@ WebRenderImageData::~WebRenderImageData() {
   }
 }
 
-bool WebRenderImageData::UsingSharedSurface() const {
+bool WebRenderImageData::UsingSharedSurface(
+    ContainerProducerID aProducerId) const {
   if (!mContainer || !mKey || mOwnsKey) {
     return false;
   }
@@ -117,7 +118,7 @@ bool WebRenderImageData::UsingSharedSurface() const {
   // rebuild the scene.
   wr::ImageKey key;
   nsresult rv = SharedSurfacesChild::Share(
-      mContainer, mManager, mManager->AsyncResourceUpdates(), key);
+      mContainer, mManager, mManager->AsyncResourceUpdates(), key, aProducerId);
   return NS_SUCCEEDED(rv) && mKey.ref() == key;
 }
 
@@ -149,8 +150,8 @@ Maybe<wr::ImageKey> WebRenderImageData::UpdateImageKey(
 
   wr::WrImageKey key;
   if (!aFallback) {
-    nsresult rv =
-        SharedSurfacesChild::Share(aContainer, mManager, aResources, key);
+    nsresult rv = SharedSurfacesChild::Share(aContainer, mManager, aResources,
+                                             key, kContainerProducerID_Invalid);
     if (NS_SUCCEEDED(rv)) {
       // Ensure that any previously owned keys are released before replacing. We
       // don't own this key, the surface itself owns it, so that it can be

gfx/layers/wr/WebRenderUserData.h

@@ -13,6 +13,7 @@
 #include "mozilla/webrender/WebRenderAPI.h"
 #include "mozilla/layers/AnimationInfo.h"
 #include "nsIFrame.h"
+#include "ImageTypes.h"
 
 class nsDisplayItemGeometry;
 
@@ -57,8 +58,8 @@ class WebRenderUserData {
 
   static bool SupportsAsyncUpdate(nsIFrame* aFrame);
 
-  static bool ProcessInvalidateForImage(nsIFrame* aFrame,
-                                        DisplayItemType aType);
+  static bool ProcessInvalidateForImage(nsIFrame* aFrame, DisplayItemType aType,
+                                        ContainerProducerID aProducerId);
 
   NS_INLINE_DECL_REFCOUNTING(WebRenderUserData)
 
@@ -151,7 +152,7 @@ class WebRenderImageData : public WebRenderUserData {
 
   bool IsAsync() { return mPipelineId.isSome(); }
 
-  bool UsingSharedSurface() const;
+  bool UsingSharedSurface(ContainerProducerID aProducerId) const;
 
   void ClearImageKey();
 

image/DynamicImage.cpp

@@ -120,6 +120,12 @@ DynamicImage::GetType(uint16_t* aType) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+DynamicImage::GetProducerId(uint32_t* aId) {
+  *aId = 0;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 DynamicImage::GetAnimated(bool* aAnimated) {
   *aAnimated = false;

image/Image.h

@@ -271,6 +271,10 @@ class ImageResource : public Image {
   explicit ImageResource(nsIURI* aURI);
   ~ImageResource();
 
+  layers::ContainerProducerID GetImageProducerId() const {
+    return mImageProducerID;
+  }
+
   bool GetSpecTruncatedTo1k(nsCString& aSpec) const;
 
   // Shared functionality for implementors of imgIContainer. Every

image/ImageWrapper.cpp

@@ -131,6 +131,11 @@ ImageWrapper::GetOrientation() { return mInnerImage->GetOrientation(); }
 NS_IMETHODIMP
 ImageWrapper::GetType(uint16_t* aType) { return mInnerImage->GetType(aType); }
 
+NS_IMETHODIMP
+ImageWrapper::GetProducerId(uint32_t* aId) {
+  return mInnerImage->GetProducerId(aId);
+}
+
 NS_IMETHODIMP
 ImageWrapper::GetAnimated(bool* aAnimated) {
   return mInnerImage->GetAnimated(aAnimated);

image/RasterImage.cpp

@@ -276,6 +276,14 @@ RasterImage::GetType(uint16_t* aType) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+RasterImage::GetProducerId(uint32_t* aId) {
+  NS_ENSURE_ARG_POINTER(aId);
+
+  *aId = ImageResource::GetImageProducerId();
+  return NS_OK;
+}
+
 LookupResult RasterImage::LookupFrameInternal(const IntSize& aSize,
                                               uint32_t aFlags,
                                               PlaybackType aPlaybackType,

image/VectorImage.cpp

@@ -661,6 +661,15 @@ VectorImage::GetType(uint16_t* aType) {
   return NS_OK;
 }
 
+//******************************************************************************
+NS_IMETHODIMP
+VectorImage::GetProducerId(uint32_t* aId) {
+  NS_ENSURE_ARG_POINTER(aId);
+
+  *aId = ImageResource::GetImageProducerId();
+  return NS_OK;
+}
+
 //******************************************************************************
 NS_IMETHODIMP
 VectorImage::GetAnimated(bool* aAnimated) {

image/imgIContainer.idl

@@ -142,6 +142,11 @@ interface imgIContainer : nsISupports
    */
   readonly attribute boolean animated;
 
+  /**
+   * Producer ID for image containers created by this image.
+   */
+  [infallible] readonly attribute unsigned long producerId;
+
   /**
    * Flags for imgIContainer operations.
    *

image/imgIRequest.idl

@@ -29,6 +29,11 @@ interface imgIRequest : nsIRequest
    */
   readonly attribute imgIContainer image;
 
+  /**
+   * Producer ID for image containers created by this image.
+   */
+  [infallible] readonly attribute unsigned long producerId;
+
   /**
    * Bits set in the return value from imageStatus
    * @name statusflags

image/imgRequestProxy.cpp

@@ -10,6 +10,7 @@
 #include "imgLoader.h"
 #include "Image.h"
 #include "ImageOps.h"
+#include "ImageTypes.h"
 #include "nsError.h"
 #include "nsCRTGlue.h"
 #include "imgINotificationObserver.h"
@@ -662,6 +663,21 @@ imgRequestProxy::GetImage(imgIContainer** aImage) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+imgRequestProxy::GetProducerId(uint32_t* aId) {
+  NS_ENSURE_TRUE(aId, NS_ERROR_NULL_POINTER);
+
+  nsCOMPtr<imgIContainer> image;
+  nsresult rv = GetImage(getter_AddRefs(image));
+  if (NS_SUCCEEDED(rv)) {
+    *aId = image->GetProducerId();
+  } else {
+    *aId = layers::kContainerProducerID_Invalid;
+  }
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 imgRequestProxy::GetImageStatus(uint32_t* aStatus) {
   if (IsValidating()) {

layout/generic/nsImageFrame.cpp

@@ -740,7 +740,9 @@ void nsImageFrame::InvalidateSelf(const nsIntRect* aLayerInvalidRect,
   // Check if WebRender has interacted with this frame. If it has
   // we need to let it know that things have changed.
   const auto type = DisplayItemType::TYPE_IMAGE;
-  if (WebRenderUserData::ProcessInvalidateForImage(this, type)) {
+  const auto producerId =
+      mImage ? mImage->GetProducerId() : kContainerProducerID_Invalid;
+  if (WebRenderUserData::ProcessInvalidateForImage(this, type, producerId)) {
     return;
   }
 

layout/style/ImageLoader.cpp

@@ -481,7 +481,7 @@ static bool IsRenderNoImages(uint32_t aDisplayItemKey) {
   return flags & TYPE_RENDERS_NO_IMAGES;
 }
 
-static void InvalidateImages(nsIFrame* aFrame) {
+static void InvalidateImages(nsIFrame* aFrame, imgIRequest* aRequest) {
   bool invalidateFrame = false;
   const SmallPointerArray<DisplayItemData>& array = aFrame->DisplayItemData();
   for (uint32_t i = 0; i < array.Length(); i++) {
@@ -516,7 +516,7 @@ static void InvalidateImages(nsIFrame* aFrame) {
           break;
         case layers::WebRenderUserData::UserDataType::eImage:
           if (static_cast<layers::WebRenderImageData*>(data.get())
-                  ->UsingSharedSurface()) {
+                  ->UsingSharedSurface(aRequest->GetProducerId())) {
             break;
           }
           MOZ_FALLTHROUGH;
@@ -532,7 +532,9 @@ static void InvalidateImages(nsIFrame* aFrame) {
   }
 }
 
-void ImageLoader::RequestPaintIfNeeded(FrameSet* aFrameSet, bool aForcePaint) {
+void ImageLoader::RequestPaintIfNeeded(FrameSet* aFrameSet,
+                                       imgIRequest* aRequest,
+                                       bool aForcePaint) {
   NS_ASSERTION(aFrameSet, "Must have a frame set");
   NS_ASSERTION(mDocument, "Should have returned earlier!");
 
@@ -545,7 +547,7 @@ void ImageLoader::RequestPaintIfNeeded(FrameSet* aFrameSet, bool aForcePaint) {
         // might not find the right display item.
         frame->InvalidateFrame();
       } else {
-        InvalidateImages(frame);
+        InvalidateImages(frame, aRequest);
 
         // Update ancestor rendering observers (-moz-element etc)
         nsIFrame* f = frame;
@@ -730,7 +732,7 @@ nsresult ImageLoader::OnFrameComplete(imgIRequest* aRequest) {
   // Since we just finished decoding a frame, we always want to paint, in case
   // we're now able to paint an image that we couldn't paint before (and hence
   // that we don't have retained data for).
-  RequestPaintIfNeeded(frameSet, /* aForcePaint = */ true);
+  RequestPaintIfNeeded(frameSet, aRequest, /* aForcePaint = */ true);
 
   return NS_OK;
 }
@@ -745,7 +747,7 @@ nsresult ImageLoader::OnFrameUpdate(imgIRequest* aRequest) {
     return NS_OK;
   }
 
-  RequestPaintIfNeeded(frameSet, /* aForcePaint = */ false);
+  RequestPaintIfNeeded(frameSet, aRequest, /* aForcePaint = */ false);
 
   return NS_OK;
 }

layout/style/ImageLoader.h

@@ -143,7 +143,8 @@ class ImageLoader final : public imgINotificationObserver {
 
   nsPresContext* GetPresContext();
 
-  void RequestPaintIfNeeded(FrameSet* aFrameSet, bool aForcePaint);
+  void RequestPaintIfNeeded(FrameSet* aFrameSet, imgIRequest* aRequest,
+                            bool aForcePaint);
   void UnblockOnloadIfNeeded(nsIFrame* aFrame, imgIRequest* aRequest);
   void RequestReflowIfNeeded(FrameSet* aFrameSet, imgIRequest* aRequest);
   void RequestReflowOnFrame(FrameWithFlags* aFwf, imgIRequest* aRequest);

layout/xul/nsImageBoxFrame.cpp

@@ -824,7 +824,8 @@ nsresult nsImageBoxFrame::OnFrameUpdate(imgIRequest* aRequest) {
   // Check if WebRender has interacted with this frame. If it has
   // we need to let it know that things have changed.
   const auto type = DisplayItemType::TYPE_XUL_IMAGE;
-  if (WebRenderUserData::ProcessInvalidateForImage(this, type)) {
+  const auto producerId = aRequest->GetProducerId();
+  if (WebRenderUserData::ProcessInvalidateForImage(this, type, producerId)) {
     return NS_OK;
   }