Bug 1552636 - Remove eStyleImageType_URL. r=#style,TYLin draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 18 May 2019 08:21:26 +0200
changeset 2008482 17463e3e8292e360ddaa79b697f7a185128512ac
parent 2008481 e104cbadc98d9ac33cd44567205627dcc074a100
child 2008483 46e3df2f90da5cd95562b3ac71fec170369473b5
child 2008842 353432583f425a8749d6705e02d1acd3a1734c72
push id363917
push useremilio@crisal.io
push dateSat, 18 May 2019 06:31:42 +0000
treeherdertry@46e3df2f90da [default view] [failures only]
bugs1552636, 1352096
milestone68.0a1
Bug 1552636 - Remove eStyleImageType_URL. r=#style,TYLin It was introduced in bug 1352096 to reduce complexity with Stylo (apparently). Right now it doesn't look like it reduces any complexity, and it's a bit annoying with some of the patches that I'm writing at the moment. So unless there's any objection I think it should go away. Differential Revision: https://phabricator.services.mozilla.com/D31708
layout/generic/nsFloatManager.cpp
layout/style/GeckoBindings.cpp
layout/style/GeckoBindings.h
layout/style/nsComputedDOMStyle.cpp
layout/style/nsStyleConsts.h
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
layout/svg/SVGObserverUtils.cpp
layout/svg/nsCSSClipPathInstance.cpp
layout/svg/nsSVGUtils.cpp
servo/components/style/gecko/conversions.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/values/animated/mod.rs
servo/components/style/values/computed/basic_shape.rs
servo/components/style/values/specified/basic_shape.rs
--- a/layout/generic/nsFloatManager.cpp
+++ b/layout/generic/nsFloatManager.cpp
@@ -2276,20 +2276,16 @@ nsFloatManager::FloatInfo::FloatInfo(nsI
                                   styleDisplay->mShapeMargin,
                                   LogicalSize(aWM, aContainerSize).ISize(aWM));
 
   switch (shapeOutside.GetType()) {
     case StyleShapeSourceType::None:
       // No need to create shape info.
       return;
 
-    case StyleShapeSourceType::URL:
-      MOZ_ASSERT_UNREACHABLE("shape-outside doesn't have URL source type!");
-      return;
-
     case StyleShapeSourceType::Path:
       MOZ_ASSERT_UNREACHABLE("shape-outside doesn't have Path source type!");
       return;
 
     case StyleShapeSourceType::Image: {
       float shapeImageThreshold = styleDisplay->mShapeImageThreshold;
       mShapeInfo = ShapeInfo::CreateImageShape(
           shapeOutside.ShapeImage(), shapeImageThreshold, shapeMargin, mFrame,
--- a/layout/style/GeckoBindings.cpp
+++ b/layout/style/GeckoBindings.cpp
@@ -1535,21 +1535,16 @@ void Gecko_CopyShapeSourceFrom(StyleShap
 
   *aDst = *aSrc;
 }
 
 void Gecko_DestroyShapeSource(StyleShapeSource* aShape) {
   aShape->~StyleShapeSource();
 }
 
-void Gecko_StyleShapeSource_SetURLValue(StyleShapeSource* aShape,
-                                        URLValue* aURL) {
-  aShape->SetURL(*aURL);
-}
-
 void Gecko_NewShapeImage(StyleShapeSource* aShape) {
   aShape->SetShapeImage(MakeUnique<nsStyleImage>());
 }
 
 void Gecko_SetToSVGPath(StyleShapeSource* aShape,
                         StyleForgottenArcSlicePtr<StylePathCommand> aCommands,
                         StyleFillRule aFill) {
   MOZ_ASSERT(aShape);
--- a/layout/style/GeckoBindings.h
+++ b/layout/style/GeckoBindings.h
@@ -520,19 +520,16 @@ void Gecko_SetStyleCoordCalcValue(nsStyl
 
 void Gecko_CopyShapeSourceFrom(mozilla::StyleShapeSource* dst,
                                const mozilla::StyleShapeSource* src);
 
 void Gecko_DestroyShapeSource(mozilla::StyleShapeSource* shape);
 
 void Gecko_NewShapeImage(mozilla::StyleShapeSource* shape);
 
-void Gecko_StyleShapeSource_SetURLValue(mozilla::StyleShapeSource* shape,
-                                        mozilla::css::URLValue* uri);
-
 void Gecko_SetToSVGPath(
     mozilla::StyleShapeSource* shape,
     mozilla::StyleForgottenArcSlicePtr<mozilla::StylePathCommand>,
     mozilla::StyleFillRule);
 
 void Gecko_SetStyleMotion(mozilla::UniquePtr<mozilla::StyleMotion>* aMotion,
                           mozilla::StyleMotion* aValue);
 
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -640,19 +640,16 @@ static void AddImageURL(const nsStyleIma
   if (auto* urlValue = aImage.GetURLValue()) {
     AddImageURL(*urlValue, aURLs);
   }
 }
 
 static void AddImageURL(const StyleShapeSource& aShapeSource,
                         nsTArray<nsString>& aURLs) {
   switch (aShapeSource.GetType()) {
-    case StyleShapeSourceType::URL:
-      AddImageURL(aShapeSource.URL(), aURLs);
-      break;
     case StyleShapeSourceType::Image:
       AddImageURL(aShapeSource.ShapeImage(), aURLs);
       break;
     default:
       break;
   }
 }
 
@@ -2556,18 +2553,17 @@ already_AddRefed<CSSValue> nsComputedDOM
       firstLayer.mOrigin != StyleGeometryBox::BorderBox ||
       firstLayer.mComposite != NS_STYLE_MASK_COMPOSITE_ADD ||
       firstLayer.mMaskMode != StyleMaskMode::MatchSource ||
       !nsStyleImageLayers::IsInitialPositionForLayerType(
           firstLayer.mPosition, nsStyleImageLayers::LayerType::Mask) ||
       !firstLayer.mRepeat.IsInitialValue() ||
       !firstLayer.mSize.IsInitialValue() ||
       !(firstLayer.mImage.GetType() == eStyleImageType_Null ||
-        firstLayer.mImage.GetType() == eStyleImageType_Image ||
-        firstLayer.mImage.GetType() == eStyleImageType_URL)) {
+        firstLayer.mImage.GetType() == eStyleImageType_Image)) {
     return nullptr;
   }
 
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
 
   SetValueToURLValue(firstLayer.mImage.GetURLValue(), val);
 
   return val.forget();
--- a/layout/style/nsStyleConsts.h
+++ b/layout/style/nsStyleConsts.h
@@ -166,18 +166,17 @@ enum class StyleScrollbarWidth : uint8_t
   Auto,
   Thin,
   None,
 };
 
 // Shape source type
 enum class StyleShapeSourceType : uint8_t {
   None,
-  URL,    // clip-path only
-  Image,  // shape-outside only
+  Image,  // shape-outside / clip-path only
   Shape,
   Box,
   Path,  // SVG path function
 };
 
 // -moz-stack-sizing
 enum class StyleStackSizing : uint8_t {
   Ignore,
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -828,17 +828,16 @@ bool StyleShapeSource::operator==(const 
   if (mType != aOther.mType) {
     return false;
   }
 
   switch (mType) {
     case StyleShapeSourceType::None:
       return true;
 
-    case StyleShapeSourceType::URL:
     case StyleShapeSourceType::Image:
       return *mShapeImage == *aOther.mShapeImage;
 
     case StyleShapeSourceType::Shape:
       return *mBasicShape == *aOther.mBasicShape &&
              mReferenceBox == aOther.mReferenceBox;
 
     case StyleShapeSourceType::Box:
@@ -847,26 +846,16 @@ bool StyleShapeSource::operator==(const 
     case StyleShapeSourceType::Path:
       return *mSVGPath == *aOther.mSVGPath;
   }
 
   MOZ_ASSERT_UNREACHABLE("Unexpected shape source type!");
   return true;
 }
 
-void StyleShapeSource::SetURL(const css::URLValue& aValue) {
-  if (mType != StyleShapeSourceType::Image &&
-      mType != StyleShapeSourceType::URL) {
-    DoDestroy();
-    new (&mShapeImage) UniquePtr<nsStyleImage>(new nsStyleImage());
-  }
-  mShapeImage->SetURLValue(do_AddRef(&aValue));
-  mType = StyleShapeSourceType::URL;
-}
-
 void StyleShapeSource::SetShapeImage(UniquePtr<nsStyleImage> aShapeImage) {
   MOZ_ASSERT(aShapeImage);
   DoDestroy();
   new (&mShapeImage) UniquePtr<nsStyleImage>(std::move(aShapeImage));
   mType = StyleShapeSourceType::Image;
 }
 
 imgIRequest* StyleShapeSource::GetShapeImageData() const {
@@ -916,20 +905,16 @@ void StyleShapeSource::SetReferenceBox(S
 
 void StyleShapeSource::DoCopy(const StyleShapeSource& aOther) {
   switch (aOther.mType) {
     case StyleShapeSourceType::None:
       mReferenceBox = StyleGeometryBox::NoBox;
       mType = StyleShapeSourceType::None;
       break;
 
-    case StyleShapeSourceType::URL:
-      SetURL(aOther.URL());
-      break;
-
     case StyleShapeSourceType::Image:
       SetShapeImage(MakeUnique<nsStyleImage>(aOther.ShapeImage()));
       break;
 
     case StyleShapeSourceType::Shape: {
       UniquePtr<StyleBasicShape> shape(Servo_CloneBasicShape(&aOther.BasicShape()));
       // TODO(emilio): This could be a copy-ctor call like above if we teach
       // cbindgen to generate copy-constructors for tagged unions.
@@ -948,17 +933,16 @@ void StyleShapeSource::DoCopy(const Styl
 }
 
 void StyleShapeSource::DoDestroy() {
   switch (mType) {
     case StyleShapeSourceType::Shape:
       mBasicShape.~UniquePtr<StyleBasicShape>();
       break;
     case StyleShapeSourceType::Image:
-    case StyleShapeSourceType::URL:
       mShapeImage.~UniquePtr<nsStyleImage>();
       break;
     case StyleShapeSourceType::Path:
       mSVGPath.~UniquePtr<StyleSVGPath>();
       break;
     case StyleShapeSourceType::None:
     case StyleShapeSourceType::Box:
       // Not a union type, so do nothing.
@@ -1994,38 +1978,32 @@ void nsStyleImage::DoCopy(const nsStyleI
   SetNull();
 
   if (aOther.mType == eStyleImageType_Image) {
     SetImageRequest(do_AddRef(aOther.mImage));
   } else if (aOther.mType == eStyleImageType_Gradient) {
     SetGradientData(aOther.mGradient);
   } else if (aOther.mType == eStyleImageType_Element) {
     SetElementId(do_AddRef(aOther.mElementId));
-  } else if (aOther.mType == eStyleImageType_URL) {
-    SetURLValue(do_AddRef(aOther.mURLValue));
   }
 
   UniquePtr<nsStyleSides> cropRectCopy;
   if (aOther.mCropRect) {
     cropRectCopy = MakeUnique<nsStyleSides>(*aOther.mCropRect.get());
   }
   SetCropRect(std::move(cropRectCopy));
 }
 
 void nsStyleImage::SetNull() {
   if (mType == eStyleImageType_Gradient) {
     mGradient->Release();
   } else if (mType == eStyleImageType_Image) {
     NS_RELEASE(mImage);
   } else if (mType == eStyleImageType_Element) {
     NS_RELEASE(mElementId);
-  } else if (mType == eStyleImageType_URL) {
-    // FIXME: NS_RELEASE doesn't handle const gracefully (unlike RefPtr).
-    const_cast<css::URLValue*>(mURLValue)->Release();
-    mURLValue = nullptr;
   }
 
   mType = eStyleImageType_Null;
   mCropRect = nullptr;
 }
 
 void nsStyleImage::SetImageRequest(
     already_AddRefed<nsStyleImageRequest> aImage) {
@@ -2069,29 +2047,16 @@ void nsStyleImage::SetElementId(already_
     mType = eStyleImageType_Element;
   }
 }
 
 void nsStyleImage::SetCropRect(UniquePtr<nsStyleSides> aCropRect) {
   mCropRect = std::move(aCropRect);
 }
 
-void nsStyleImage::SetURLValue(already_AddRefed<const URLValue> aValue) {
-  RefPtr<const URLValue> value = aValue;
-
-  if (mType != eStyleImageType_Null) {
-    SetNull();
-  }
-
-  if (value) {
-    mURLValue = value.forget().take();
-    mType = eStyleImageType_URL;
-  }
-}
-
 static int32_t ConvertToPixelCoord(const nsStyleCoord& aCoord,
                                    int32_t aPercentScale) {
   double pixelValue;
   switch (aCoord.GetUnit()) {
     case eStyleUnit_Percent:
       pixelValue = aCoord.GetPercentValue() * aPercentScale;
       break;
     case eStyleUnit_Factor:
@@ -2184,17 +2149,17 @@ bool nsStyleImage::IsOpaque() const {
   if (!IsComplete()) {
     return false;
   }
 
   if (mType == eStyleImageType_Gradient) {
     return mGradient->IsOpaque();
   }
 
-  if (mType == eStyleImageType_Element || mType == eStyleImageType_URL) {
+  if (mType == eStyleImageType_Element) {
     return false;
   }
 
   MOZ_ASSERT(mType == eStyleImageType_Image, "unexpected image type");
   MOZ_ASSERT(GetImageData(), "should've returned earlier above");
 
   nsCOMPtr<imgIContainer> imageContainer;
   GetImageData()->GetImage(getter_AddRefs(imageContainer));
@@ -2216,17 +2181,16 @@ bool nsStyleImage::IsOpaque() const {
 }
 
 bool nsStyleImage::IsComplete() const {
   switch (mType) {
     case eStyleImageType_Null:
       return false;
     case eStyleImageType_Gradient:
     case eStyleImageType_Element:
-    case eStyleImageType_URL:
       return true;
     case eStyleImageType_Image: {
       if (!IsResolved()) {
         return false;
       }
       imgRequestProxy* req = GetImageData();
       if (!req) {
         return false;
@@ -2243,17 +2207,16 @@ bool nsStyleImage::IsComplete() const {
 }
 
 bool nsStyleImage::IsLoaded() const {
   switch (mType) {
     case eStyleImageType_Null:
       return false;
     case eStyleImageType_Gradient:
     case eStyleImageType_Element:
-    case eStyleImageType_URL:
       return true;
     case eStyleImageType_Image: {
       imgRequestProxy* req = GetImageData();
       if (!req) {
         return false;
       }
       uint32_t status = imgIRequest::STATUS_ERROR;
       return NS_SUCCEEDED(req->GetImageStatus(&status)) &&
@@ -2288,20 +2251,16 @@ bool nsStyleImage::operator==(const nsSt
   if (mType == eStyleImageType_Gradient) {
     return *mGradient == *aOther.mGradient;
   }
 
   if (mType == eStyleImageType_Element) {
     return mElementId == aOther.mElementId;
   }
 
-  if (mType == eStyleImageType_URL) {
-    return DefinitelyEqualURIs(mURLValue, aOther.mURLValue);
-  }
-
   return true;
 }
 
 void nsStyleImage::PurgeCacheForViewportChange(
     const mozilla::Maybe<nsSize>& aSVGViewportSize,
     const bool aHasIntrinsicRatio) const {
   EnsureCachedBIData();
 
@@ -2323,24 +2282,17 @@ already_AddRefed<nsIURI> nsStyleImage::G
     return nullptr;
   }
 
   nsCOMPtr<nsIURI> uri = mImage->GetImageURI();
   return uri.forget();
 }
 
 const css::URLValue* nsStyleImage::GetURLValue() const {
-  if (mType == eStyleImageType_Image) {
-    return mImage->GetImageValue();
-  }
-  if (mType == eStyleImageType_URL) {
-    return mURLValue;
-  }
-
-  return nullptr;
+  return mType == eStyleImageType_Image ? mImage->GetImageValue() : nullptr;
 }
 
 // --------------------
 // nsStyleImageLayers
 //
 
 const nsCSSPropertyID nsStyleImageLayers::kBackgroundLayerTable[] = {
     eCSSProperty_background,             // shorthand
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -258,17 +258,16 @@ class nsStyleImageRequest {
 
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(nsStyleImageRequest::Mode)
 
 enum nsStyleImageType {
   eStyleImageType_Null,
   eStyleImageType_Image,
   eStyleImageType_Gradient,
   eStyleImageType_Element,
-  eStyleImageType_URL
 };
 
 struct CachedBorderImageData {
   ~CachedBorderImageData() { PurgeCachedImages(); }
 
   // Caller are expected to ensure that the value of aSVGViewportSize is
   // different from the cached one since the method won't do the check.
   void SetCachedSVGViewportSize(const mozilla::Maybe<nsSize>& aSVGViewportSize);
@@ -303,17 +302,16 @@ struct nsStyleImage {
   nsStyleImage(const nsStyleImage& aOther);
   nsStyleImage& operator=(const nsStyleImage& aOther);
 
   void SetNull();
   void SetImageRequest(already_AddRefed<nsStyleImageRequest> aImage);
   void SetGradientData(nsStyleGradient* aGradient);
   void SetElementId(already_AddRefed<nsAtom> aElementId);
   void SetCropRect(mozilla::UniquePtr<nsStyleSides> aCropRect);
-  void SetURLValue(already_AddRefed<const URLValue> aURLValue);
 
   void ResolveImage(mozilla::dom::Document& aDocument,
                     const nsStyleImage* aOldImage) {
     MOZ_ASSERT(mType != eStyleImageType_Image || mImage);
     if (mType == eStyleImageType_Image && !mImage->IsResolved()) {
       const nsStyleImageRequest* oldRequest =
           (aOldImage && aOldImage->GetType() == eStyleImageType_Image)
               ? aOldImage->ImageRequest()
@@ -427,19 +425,16 @@ struct nsStyleImage {
   // This variable keeps some cache data for border image and is lazily
   // allocated since it is only used in border image case.
   mozilla::UniquePtr<CachedBorderImageData> mCachedBIData;
 
   nsStyleImageType mType;
   union {
     nsStyleImageRequest* mImage;
     nsStyleGradient* mGradient;
-    const URLValue* mURLValue;  // See the comment in SetStyleImage's 'case
-                                // eCSSUnit_URL' section to know why we need to
-                                // store URLValues separately from mImage.
     nsAtom* mElementId;
   };
 
   // This is _currently_ used only in conjunction with eStyleImageType_Image.
   mozilla::UniquePtr<nsStyleSides> mCropRect;
 };
 
 struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleColor {
@@ -1592,24 +1587,16 @@ struct StyleShapeSource final {
   bool operator==(const StyleShapeSource& aOther) const;
 
   bool operator!=(const StyleShapeSource& aOther) const {
     return !(*this == aOther);
   }
 
   StyleShapeSourceType GetType() const { return mType; }
 
-  const css::URLValue& URL() const {
-    MOZ_ASSERT(mType == StyleShapeSourceType::URL, "Wrong shape source type!");
-    MOZ_ASSERT(mShapeImage && mShapeImage->GetURLValue());
-    return *mShapeImage->GetURLValue();
-  }
-
-  void SetURL(const css::URLValue& aURLValue);
-
   const nsStyleImage& ShapeImage() const {
     MOZ_ASSERT(mType == StyleShapeSourceType::Image,
                "Wrong shape source type!");
     MOZ_ASSERT(mShapeImage);
     return *mShapeImage;
   }
 
   // Iff we have "shape-outside:<image>" with an image URI (not a gradient),
--- a/layout/svg/SVGObserverUtils.cpp
+++ b/layout/svg/SVGObserverUtils.cpp
@@ -1166,22 +1166,23 @@ void SVGObserverUtils::DetachFromCanvasC
 }
 
 static nsSVGPaintingProperty* GetOrCreateClipPathObserver(
     nsIFrame* aClippedFrame) {
   MOZ_ASSERT(!aClippedFrame->GetPrevContinuation(),
              "Require first continuation");
 
   const nsStyleSVGReset* svgStyleReset = aClippedFrame->StyleSVGReset();
-  if (svgStyleReset->mClipPath.GetType() != StyleShapeSourceType::URL) {
+  if (svgStyleReset->mClipPath.GetType() != StyleShapeSourceType::Image) {
     return nullptr;
   }
-  const css::URLValue& url = svgStyleReset->mClipPath.URL();
+  const css::URLValue* url = svgStyleReset->mClipPath.ShapeImage().GetURLValue();
+  MOZ_ASSERT(url);
   RefPtr<URLAndReferrerInfo> pathURI =
-      ResolveURLUsingLocalRef(aClippedFrame, &url);
+      ResolveURLUsingLocalRef(aClippedFrame, url);
   return GetPaintingProperty(pathURI, aClippedFrame, ClipPathProperty());
 }
 
 SVGObserverUtils::ReferenceState SVGObserverUtils::GetAndObserveClipPath(
     nsIFrame* aClippedFrame, nsSVGClipPathFrame** aClipPathFrame) {
   if (aClipPathFrame) {
     *aClipPathFrame = nullptr;
   }
--- a/layout/svg/nsCSSClipPathInstance.cpp
+++ b/layout/svg/nsCSSClipPathInstance.cpp
@@ -49,17 +49,17 @@ void nsCSSClipPathInstance::ApplyBasicSh
 /* static*/
 bool nsCSSClipPathInstance::HitTestBasicShapeOrPathClip(
     nsIFrame* aFrame, const gfxPoint& aPoint) {
   auto& clipPathStyle = aFrame->StyleSVGReset()->mClipPath;
   StyleShapeSourceType type = clipPathStyle.GetType();
   MOZ_ASSERT(type != StyleShapeSourceType::None, "unexpected none value");
   // In the future nsCSSClipPathInstance may handle <clipPath> references as
   // well. For the time being return early.
-  if (type == StyleShapeSourceType::URL) {
+  if (type == StyleShapeSourceType::Image) {
     return false;
   }
 
   nsCSSClipPathInstance instance(aFrame, clipPathStyle);
 
   RefPtr<DrawTarget> drawTarget =
       gfxPlatform::GetPlatform()->ScreenReferenceDrawTarget();
   RefPtr<Path> path = instance.CreateClipPath(
--- a/layout/svg/nsSVGUtils.cpp
+++ b/layout/svg/nsSVGUtils.cpp
@@ -452,20 +452,20 @@ void nsSVGUtils::DetermineMaskUsage(nsIF
   // XXX check return value?
   SVGObserverUtils::GetAndObserveMasks(firstFrame, &maskFrames);
   aUsage.shouldGenerateMaskLayer = (maskFrames.Length() > 0);
 
   nsSVGClipPathFrame* clipPathFrame;
   // XXX check return value?
   SVGObserverUtils::GetAndObserveClipPath(firstFrame, &clipPathFrame);
   MOZ_ASSERT(!clipPathFrame ||
-             svgReset->mClipPath.GetType() == StyleShapeSourceType::URL);
+             svgReset->mClipPath.GetType() == StyleShapeSourceType::Image);
 
   switch (svgReset->mClipPath.GetType()) {
-    case StyleShapeSourceType::URL:
+    case StyleShapeSourceType::Image:
       if (clipPathFrame) {
         if (clipPathFrame->IsTrivial()) {
           aUsage.shouldApplyClipPath = true;
         } else {
           aUsage.shouldGenerateClipMaskLayer = true;
         }
       }
       break;
--- a/servo/components/style/gecko/conversions.rs
+++ b/servo/components/style/gecko/conversions.rs
@@ -378,17 +378,16 @@ impl nsStyleImage {
             nsStyleImageType::eStyleImageType_Gradient => {
                 Some(GenericImage::Gradient(self.get_gradient()))
             },
             nsStyleImageType::eStyleImageType_Element => {
                 use crate::gecko_string_cache::Atom;
                 let atom = bindings::Gecko_GetImageElement(self);
                 Some(GenericImage::Element(Atom::from_raw(atom)))
             },
-            _ => panic!("Unexpected image type"),
         }
     }
 
     unsafe fn get_image_url(&self) -> ComputedImageUrl {
         let image_request = bindings::Gecko_GetImageRequest(self)
             .as_ref()
             .expect("Null image request?");
         ComputedImageUrl::from_image_request(image_request)
@@ -530,20 +529,18 @@ impl nsStyleImage {
     }
 }
 
 pub mod basic_shape {
     //! Conversions from and to CSS shape representations.
     use crate::gecko_bindings::structs::{
         StyleGeometryBox, StyleShapeSource, StyleShapeSourceType,
     };
-    use crate::gecko_bindings::sugar::refptr::RefPtr;
     use crate::values::computed::basic_shape::{BasicShape, ClippingShape, FloatAreaShape};
     use crate::values::computed::motion::OffsetPath;
-    use crate::values::computed::url::ComputedUrl;
     use crate::values::generics::basic_shape::{GeometryBox, Path, ShapeBox, ShapeSource};
     use crate::values::specified::SVGPathData;
 
     impl StyleShapeSource {
         /// Convert StyleShapeSource to ShapeSource except URL and Image
         /// types.
         fn into_shape_source<ReferenceBox, ImageOrUrl>(
             &self,
@@ -559,17 +556,17 @@ pub mod basic_shape {
                     let shape = Box::new(other_shape.clone());
                     let reference_box = if self.mReferenceBox == StyleGeometryBox::NoBox {
                         None
                     } else {
                         Some(self.mReferenceBox.into())
                     };
                     Some(ShapeSource::Shape(shape, reference_box))
                 },
-                StyleShapeSourceType::URL | StyleShapeSourceType::Image => None,
+                StyleShapeSourceType::Image => None,
                 StyleShapeSourceType::Path => {
                     let path = self.to_svg_path().expect("expect an SVGPathData");
                     let fill = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }.mFillRule;
                     Some(ShapeSource::Path(Path { fill, path }))
                 },
             }
         }
 
@@ -582,40 +579,36 @@ pub mod basic_shape {
                 },
                 _ => None,
             }
         }
     }
 
     impl<'a> From<&'a StyleShapeSource> for ClippingShape {
         fn from(other: &'a StyleShapeSource) -> Self {
+            use crate::values::generics::image::Image as GenericImage;
             match other.mType {
-                StyleShapeSourceType::URL => unsafe {
+                StyleShapeSourceType::Image => unsafe {
                     let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr;
-                    let other_url =
-                        RefPtr::new(*shape_image.__bindgen_anon_1.mURLValue.as_ref() as *mut _);
-                    let url = ComputedUrl::from_url_value(other_url);
-                    ShapeSource::ImageOrUrl(url)
-                },
-                StyleShapeSourceType::Image => {
-                    unreachable!("ClippingShape doesn't support Image!");
+                    let image = shape_image.into_image().expect("Cannot convert to Image");
+                    match image {
+                        GenericImage::Url(url) => ShapeSource::ImageOrUrl(url),
+                        _ => panic!("ClippingShape doesn't support non-url images"),
+                    }
                 },
                 _ => other
                     .into_shape_source()
                     .expect("Couldn't convert to StyleSource!"),
             }
         }
     }
 
     impl<'a> From<&'a StyleShapeSource> for FloatAreaShape {
         fn from(other: &'a StyleShapeSource) -> Self {
             match other.mType {
-                StyleShapeSourceType::URL => {
-                    unreachable!("FloatAreaShape doesn't support URL!");
-                },
                 StyleShapeSourceType::Image => unsafe {
                     let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr;
                     let image = shape_image.into_image().expect("Cannot convert to Image");
                     ShapeSource::ImageOrUrl(image)
                 },
                 _ => other
                     .into_shape_source()
                     .expect("Couldn't convert to StyleSource!"),
@@ -627,17 +620,16 @@ pub mod basic_shape {
         fn from(other: &'a StyleShapeSource) -> Self {
             match other.mType {
                 StyleShapeSourceType::Path => {
                     OffsetPath::Path(other.to_svg_path().expect("Cannot convert to SVGPathData"))
                 },
                 StyleShapeSourceType::None => OffsetPath::none(),
                 StyleShapeSourceType::Shape |
                 StyleShapeSourceType::Box |
-                StyleShapeSourceType::URL |
                 StyleShapeSourceType::Image => unreachable!("Unsupported offset-path type"),
             }
         }
     }
 
     impl From<ShapeBox> for StyleGeometryBox {
         fn from(reference: ShapeBox) -> Self {
             use crate::gecko_bindings::structs::StyleGeometryBox::*;
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3618,35 +3618,29 @@ fn set_style_svg_path(
         let ref mut ${ident} = self.gecko.${gecko_ffi_name};
 
         // clean up existing struct.
         unsafe { bindings::Gecko_DestroyShapeSource(${ident}) };
 
         ${ident}.mType = StyleShapeSourceType::None;
 
         match v {
-            % if ident == "clip_path":
-            ShapeSource::ImageOrUrl(ref url) => {
-                unsafe {
-                    bindings::Gecko_StyleShapeSource_SetURLValue(${ident}, url.url_value_ptr())
-                }
-            }
-            % elif ident == "shape_outside":
+            ShapeSource::None => {} // don't change the type
             ShapeSource::ImageOrUrl(image) => {
+                % if ident == "clip_path":
+                use crate::values::generics::image::Image;
+
+                let image = Image::Url(image);
+                % endif
                 unsafe {
                     bindings::Gecko_NewShapeImage(${ident});
                     let style_image = &mut *${ident}.__bindgen_anon_1.mShapeImage.as_mut().mPtr;
                     style_image.set(image);
                 }
             }
-            % else:
-               <% raise Exception("Unknown property: %s" % ident) %>
-            }
-            % endif
-            ShapeSource::None => {} // don't change the type
             ShapeSource::Box(reference) => {
                 ${ident}.mReferenceBox = reference.into();
                 ${ident}.mType = StyleShapeSourceType::Box;
             }
             ShapeSource::Path(p) => set_style_svg_path(${ident}, p.path, p.fill),
             ShapeSource::Shape(servo_shape, maybe_box) => {
                 unsafe {
                     ${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr =
--- a/servo/components/style/values/animated/mod.rs
+++ b/servo/components/style/values/animated/mod.rs
@@ -5,17 +5,17 @@
 //! Animated values.
 //!
 //! Some values, notably colors, cannot be interpolated directly with their
 //! computed values and need yet another intermediate representation. This
 //! module's raison d'ĂȘtre is to ultimately contain all these types.
 
 use crate::properties::PropertyId;
 use crate::values::computed::length::LengthPercentage;
-use crate::values::computed::url::ComputedUrl;
+use crate::values::computed::url::{ComputedUrl, ComputedImageUrl};
 use crate::values::computed::Angle as ComputedAngle;
 use crate::values::computed::Image;
 use crate::values::specified::SVGPathData;
 use crate::values::CSSFloat;
 use app_units::Au;
 use smallvec::SmallVec;
 use std::cmp;
 
@@ -375,16 +375,17 @@ macro_rules! trivial_to_animated_value {
         }
     };
 }
 
 trivial_to_animated_value!(Au);
 trivial_to_animated_value!(LengthPercentage);
 trivial_to_animated_value!(ComputedAngle);
 trivial_to_animated_value!(ComputedUrl);
+trivial_to_animated_value!(ComputedImageUrl);
 trivial_to_animated_value!(bool);
 trivial_to_animated_value!(f32);
 // Note: This implementation is for ToAnimatedValue of ShapeSource.
 //
 // SVGPathData uses Box<[T]>. If we want to derive ToAnimatedValue for all the
 // types, we have to do "impl ToAnimatedValue for Box<[T]>" first.
 // However, the general version of "impl ToAnimatedValue for Box<[T]>" needs to
 // clone |T| and convert it into |T::AnimatedValue|. However, for SVGPathData
--- a/servo/components/style/values/computed/basic_shape.rs
+++ b/servo/components/style/values/computed/basic_shape.rs
@@ -2,27 +2,27 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 //! CSS handling for the computed value of
 //! [`basic-shape`][basic-shape]s
 //!
 //! [basic-shape]: https://drafts.csswg.org/css-shapes/#typedef-basic-shape
 
-use crate::values::computed::url::ComputedUrl;
+use crate::values::computed::url::ComputedImageUrl;
 use crate::values::computed::{Image, LengthPercentage, NonNegativeLengthPercentage};
 use crate::values::generics::basic_shape as generic;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
 
 /// A computed alias for FillRule.
 pub use crate::values::generics::basic_shape::FillRule;
 
 /// A computed clipping shape.
-pub type ClippingShape = generic::ClippingShape<BasicShape, ComputedUrl>;
+pub type ClippingShape = generic::ClippingShape<BasicShape, ComputedImageUrl>;
 
 /// A computed float area shape.
 pub type FloatAreaShape = generic::FloatAreaShape<BasicShape, Image>;
 
 /// A computed basic shape.
 pub type BasicShape = generic::GenericBasicShape<
     LengthPercentage,
     LengthPercentage,
--- a/servo/components/style/values/specified/basic_shape.rs
+++ b/servo/components/style/values/specified/basic_shape.rs
@@ -10,29 +10,29 @@
 use crate::parser::{Parse, ParserContext};
 use crate::values::generics::basic_shape as generic;
 use crate::values::generics::basic_shape::{GeometryBox, Path, PolygonCoord};
 use crate::values::generics::basic_shape::{ShapeBox, ShapeSource};
 use crate::values::generics::rect::Rect;
 use crate::values::specified::border::BorderRadius;
 use crate::values::specified::image::Image;
 use crate::values::specified::position::{HorizontalPosition, Position, VerticalPosition};
-use crate::values::specified::url::SpecifiedUrl;
+use crate::values::specified::url::SpecifiedImageUrl;
 use crate::values::specified::SVGPathData;
 use crate::values::specified::{LengthPercentage, NonNegativeLengthPercentage};
 use crate::Zero;
 use cssparser::Parser;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 
 /// A specified alias for FillRule.
 pub use crate::values::generics::basic_shape::FillRule;
 
 /// A specified clipping shape.
-pub type ClippingShape = generic::ClippingShape<BasicShape, SpecifiedUrl>;
+pub type ClippingShape = generic::ClippingShape<BasicShape, SpecifiedImageUrl>;
 
 /// A specified float area shape.
 pub type FloatAreaShape = generic::FloatAreaShape<BasicShape, Image>;
 
 /// A specified basic shape.
 pub type BasicShape = generic::BasicShape<
     HorizontalPosition,
     VerticalPosition,
@@ -75,17 +75,17 @@ impl Parse for ClippingShape {
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         if is_clip_path_path_enabled(context) {
             if let Ok(p) = input.try(|i| Path::parse(context, i)) {
                 return Ok(ShapeSource::Path(p));
             }
         }
 
-        if let Ok(url) = input.try(|i| SpecifiedUrl::parse(context, i)) {
+        if let Ok(url) = input.try(|i| SpecifiedImageUrl::parse(context, i)) {
             return Ok(ShapeSource::ImageOrUrl(url));
         }
 
         Self::parse_common(context, input)
     }
 }
 
 impl Parse for FloatAreaShape {