Bug 1625797 - Replace RasterImage nsIProperties impl with accessors for hotspot coordinate. r=tnikkel

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

--HG--
extra : moz-landing-system : lando

dom/events/EventStateManager.cpp

@@ -3827,37 +3827,23 @@ static bool ShouldBlockCustomCursor(nsPresContext* aPresContext,
 static gfx::IntPoint ComputeHotspot(imgIContainer* aContainer,
                                     const Maybe<gfx::Point>& aHotspot) {
   MOZ_ASSERT(aContainer);
+
   // css3-ui says to use the CSS-specified hotspot if present,
   // otherwise use the intrinsic hotspot, otherwise use the top left
   // corner.
-  uint32_t hotspotX, hotspotY;
   if (aHotspot) {
     int32_t imgWidth, imgHeight;
     aContainer->GetWidth(&imgWidth);
     aContainer->GetHeight(&imgHeight);
-
-    // XXX std::max(NS_lround(x), 0)?
-    hotspotX = aHotspot->x > 0.0f ? uint32_t(aHotspot->x + 0.5f) : uint32_t(0);
-    if (hotspotX >= uint32_t(imgWidth)) hotspotX = imgWidth - 1;
-    hotspotY = aHotspot->y > 0.0f ? uint32_t(aHotspot->y + 0.5f) : uint32_t(0);
-    if (hotspotY >= uint32_t(imgHeight)) hotspotY = imgHeight - 1;
-  } else {
-    hotspotX = 0;
-    hotspotY = 0;
-    nsCOMPtr<nsIProperties> props(do_QueryInterface(aContainer));
-    if (props) {
-      nsCOMPtr<nsISupportsPRUint32> hotspotXWrap, hotspotYWrap;
-
-      props->Get("hotspotX", NS_GET_IID(nsISupportsPRUint32),
-                 getter_AddRefs(hotspotXWrap));
-      props->Get("hotspotY", NS_GET_IID(nsISupportsPRUint32),
-                 getter_AddRefs(hotspotYWrap));
-
-      if (hotspotXWrap) hotspotXWrap->GetData(&hotspotX);
-      if (hotspotYWrap) hotspotYWrap->GetData(&hotspotY);
-    }
+    auto hotspot = gfx::IntPoint::Round(*aHotspot);
+    return {std::max(std::min(hotspot.x, imgWidth - 1), 0),
+            std::max(std::min(hotspot.y, imgHeight - 1), 0)};
   }
-  return {hotspotX, hotspotY};
+
+  gfx::IntPoint hotspot;
+  aContainer->GetHotspotX(&hotspot.x);
+  aContainer->GetHotspotY(&hotspot.y);
+  return hotspot;
 }
 
 static CursorImage ComputeCustomCursor(nsPresContext* aPresContext,

image/DynamicImage.cpp

@@ -294,5 +294,13 @@ void DynamicImage::PropagateUseCounters(dom::Document*) {
   // No use counters.
 }
 
+nsresult DynamicImage::GetHotspotX(int32_t* aX) {
+  return Image::GetHotspotX(aX);
+}
+
+nsresult DynamicImage::GetHotspotY(int32_t* aY) {
+  return Image::GetHotspotY(aY);
+}
+
 }  // namespace image
 }  // namespace mozilla

image/Image.h

@@ -266,6 +266,15 @@ class Image : public imgIContainer {
   virtual void SetHasError() = 0;
 
   virtual nsIURI* GetURI() const = 0;
+
+  nsresult GetHotspotX(int32_t* aX) override {
+    *aX = 0;
+    return NS_OK;
+  }
+  nsresult GetHotspotY(int32_t* aY) override {
+    *aY = 0;
+    return NS_OK;
+  }
 };
 
 class ImageResource : public Image {

image/ImageWrapper.cpp

@@ -124,6 +124,14 @@ Maybe<AspectRatio> ImageWrapper::GetIntrinsicRatio() {
   return mInnerImage->GetIntrinsicRatio();
 }
 
+nsresult ImageWrapper::GetHotspotX(int32_t* aX) {
+  return Image::GetHotspotX(aX);
+}
+
+nsresult ImageWrapper::GetHotspotY(int32_t* aY) {
+  return Image::GetHotspotY(aY);
+}
+
 NS_IMETHODIMP_(Orientation)
 ImageWrapper::GetOrientation() { return mInnerImage->GetOrientation(); }
 

image/RasterImage.cpp

@@ -61,9 +61,9 @@ using std::ceil;
 using std::min;
 
 #ifndef DEBUG
-NS_IMPL_ISUPPORTS(RasterImage, imgIContainer, nsIProperties)
+NS_IMPL_ISUPPORTS(RasterImage, imgIContainer)
 #else
-NS_IMPL_ISUPPORTS(RasterImage, imgIContainer, nsIProperties, imgIContainerDebug)
+NS_IMPL_ISUPPORTS(RasterImage, imgIContainer, imgIContainerDebug)
 #endif
 
 //******************************************************************************
@@ -743,17 +743,9 @@ bool RasterImage::SetMetadata(const ImageMetadata& aMetadata,
   }
 
   if (aMetadata.HasHotspot()) {
-    IntPoint hotspot = aMetadata.GetHotspot();
-
-    nsCOMPtr<nsISupportsPRUint32> intwrapx =
-        do_CreateInstance(NS_SUPPORTS_PRUINT32_CONTRACTID);
-    nsCOMPtr<nsISupportsPRUint32> intwrapy =
-        do_CreateInstance(NS_SUPPORTS_PRUINT32_CONTRACTID);
-    intwrapx->SetData(hotspot.x);
-    intwrapy->SetData(hotspot.y);
-
-    Set("hotspotX", intwrapx);
-    Set("hotspotY", intwrapy);
+    auto hotspot = aMetadata.GetHotspot();
+    mHotspot.x = std::max(std::min(hotspot.x, mSize.width - 1), 0);
+    mHotspot.y = std::max(std::min(hotspot.y, mSize.height - 1), 0);
   }
 
   return true;
@@ -971,48 +963,14 @@ nsresult RasterImage::SetSourceSizeHint(uint32_t aSizeHint) {
   return rv;
 }
 
-/********* Methods to implement lazy allocation of nsIProperties object *******/
-NS_IMETHODIMP
-RasterImage::Get(const char* prop, const nsIID& iid, void** result) {
-  if (!mProperties) {
-    return NS_ERROR_FAILURE;
-  }
-  return mProperties->Get(prop, iid, result);
+nsresult RasterImage::GetHotspotX(int32_t* aX) {
+  *aX = mHotspot.x;
+  return NS_OK;
 }
 
-NS_IMETHODIMP
-RasterImage::Set(const char* prop, nsISupports* value) {
-  if (!mProperties) {
-    mProperties = new nsProperties();
-  }
-  return mProperties->Set(prop, value);
-}
-
-NS_IMETHODIMP
-RasterImage::Has(const char* prop, bool* _retval) {
-  NS_ENSURE_ARG_POINTER(_retval);
-  if (!mProperties) {
-    *_retval = false;
-    return NS_OK;
-  }
-  return mProperties->Has(prop, _retval);
-}
-
-NS_IMETHODIMP
-RasterImage::Undefine(const char* prop) {
-  if (!mProperties) {
-    return NS_ERROR_FAILURE;
-  }
-  return mProperties->Undefine(prop);
-}
-
-NS_IMETHODIMP
-RasterImage::GetKeys(nsTArray<nsCString>& keys) {
-  if (!mProperties) {
-    keys.Clear();
-    return NS_OK;
-  }
-  return mProperties->GetKeys(keys);
+nsresult RasterImage::GetHotspotY(int32_t* aY) {
+  *aY = mHotspot.y;
+  return NS_OK;
 }
 
 void RasterImage::Discard() {

image/RasterImage.h

@@ -20,7 +20,6 @@
 #include "Image.h"
 #include "nsCOMPtr.h"
 #include "imgIContainer.h"
-#include "nsIProperties.h"
 #include "nsTArray.h"
 #include "LookupResult.h"
 #include "nsThreadUtils.h"
@@ -139,7 +138,6 @@ class ImageMetadata;
 class SourceBuffer;
 
 class RasterImage final : public ImageResource,
-                          public nsIProperties,
                           public SupportsWeakPtr<RasterImage>
 #ifdef DEBUG
     ,
@@ -152,7 +150,6 @@ class RasterImage final : public ImageResource,
  public:
   MOZ_DECLARE_WEAKREFERENCE_TYPENAME(RasterImage)
   NS_DECL_THREADSAFE_ISUPPORTS
-  NS_DECL_NSIPROPERTIES
   NS_DECL_IMGICONTAINER
 #ifdef DEBUG
   NS_DECL_IMGICONTAINERDEBUG
@@ -367,7 +364,8 @@ class RasterImage final : public ImageResource,
   /// If this has a value, we're waiting for SetSize() to send the load event.
   Maybe<Progress> mLoadProgress;
 
-  nsCOMPtr<nsIProperties> mProperties;
+  // Hotspot of this image, or (0, 0) if there is no hotspot data.
+  gfx::IntPoint mHotspot;
 
   /// If this image is animated, a FrameAnimator which manages its animation.
   UniquePtr<FrameAnimator> mFrameAnimator;

image/VectorImage.cpp

@@ -1560,5 +1560,13 @@ void VectorImage::MediaFeatureValuesChangedAllDocuments(
   }
 }
 
+nsresult VectorImage::GetHotspotX(int32_t* aX) {
+  return Image::GetHotspotX(aX);
+}
+
+nsresult VectorImage::GetHotspotY(int32_t* aY) {
+  return Image::GetHotspotY(aY);
+}
+
 }  // namespace image
 }  // namespace mozilla

image/imgIContainer.idl

@@ -106,6 +106,16 @@ interface imgIContainer : nsISupports
    */
   [notxpcom, nostdcall] readonly attribute MaybeAspectRatio intrinsicRatio;
 
+  /**
+   * The x coordinate of the image's hotspot, or 0 if there is no hotspot.
+   */
+  readonly attribute int32_t hotspotX;
+
+  /**
+   * The y coordinate of the image's hotspot, or 0 if there is no hotspot.
+   */
+  readonly attribute int32_t hotspotY;
+
   /**
    * Given a size at which this image will be displayed, and the drawing
    * parameters affecting how it will be drawn, returns the image size which

image/test/unit/test_imgtools.js

@@ -784,10 +784,8 @@ function run_test() {
       inMimeType
     );
 
-    var props = container.QueryInterface(Ci.nsIProperties);
-
-    Assert.equal(props.get("hotspotX", Ci.nsISupportsPRUint32).data, 10);
-    Assert.equal(props.get("hotspotY", Ci.nsISupportsPRUint32).data, 9);
+    Assert.equal(container.hotspotX, 10);
+    Assert.equal(container.hotspotY, 9);
 
     /* ========== end ========== */
   } catch (e) {