Support fragment URLs for all kinds of SVG images

Source/core/style/StyleFetchedImageSet.h

Index: Source/core/style/StyleFetchedImageSet.h
diff --git a/Source/core/style/StyleFetchedImageSet.h b/Source/core/style/StyleFetchedImageSet.h
index 2ad89c3fefb7eccd410a0870c642e609f3e5ae46..657c28705ea79d3879e8ecce69db16f6d81da063 100644
--- a/Source/core/style/StyleFetchedImageSet.h
+++ b/Source/core/style/StyleFetchedImageSet.h
@@ -41,9 +41,9 @@ class CSSImageSetValue;
 class StyleFetchedImageSet final : public StyleImage, private ImageResourceClient {
     WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED(StyleFetchedImageSet);
 public:
-    static PassRefPtrWillBeRawPtr<StyleFetchedImageSet> create(ImageResource* image, float imageScaleFactor, CSSImageSetValue* value)
+    static PassRefPtrWillBeRawPtr<StyleFetchedImageSet> create(ImageResource* image, float imageScaleFactor, CSSImageSetValue* value, const KURL& urlWithFragment)
     {
-        return adoptRefWillBeNoop(new StyleFetchedImageSet(image, imageScaleFactor, value));
+        return adoptRefWillBeNoop(new StyleFetchedImageSet(image, imageScaleFactor, value, urlWithFragment));
     }
     ~StyleFetchedImageSet() override;
 
@@ -77,12 +77,13 @@ public:
     DECLARE_VIRTUAL_TRACE();
 
 private:
-    StyleFetchedImageSet(ImageResource*, float imageScaleFactor, CSSImageSetValue*);
+    StyleFetchedImageSet(ImageResource*, float imageScaleFactor, CSSImageSetValue*, const KURL&);
 
     ResourcePtr<ImageResource> m_bestFitImage;
     float m_imageScaleFactor;
 
     RawPtrWillBeMember<CSSImageSetValue> m_imageSetValue; // Not retained; it owns us.
+    KURL m_urlWithFragment;

[f(malita), 2015/09/08 18:41:19]

Plumbing the fragment through several general classes for the sole benefit of
SVG images seems unfortunate.

[davve, 2015/09/08 19:12:25]

Yes. The feature seems to have a significant cost. We might come up with a
better way of solving it or we may drop the feature if we deem it not worth the
complexity. Personally I didn't find the cost particularly high for the author
gains but I might have a skewed view begin the one who wrote the code.

If we wouldn't trash SVGImageForContainer objects all the time, we might not
have to carry the URL in parent classes like this. I can't assess at this point
the trade-offs and doability of doing so though.

[f(malita), 2015/09/08 20:45:51]

I also think the tradeoff is worth it, and I'm certainly not opposed to landing
as-is if a better approach is not obvious.

[davve, 2015/09/09 14:22:53]

I looked into keeping the fragment URL isolated to
SVGImageForContainer today but failed to find a way of not
contaminating all the layers in between style and SVG.
SVGImageForContainers are created on demand by the ImageResource
during layout or paint. They're indexed on LayoutObjects so they can't
really exist before the LayoutObject has been created.

For background images it looks like we must carry the fragment URL
from CSSImageValue (where it's stored as a string m_absoluteURL)
computed style time to layout/paint time when we create the
SVGImageForContainer.

StyleFetchedImage{,Set} seemed like one reasonable place to do that,
but yes we'll pay the price for all background images that doesn't
support fragment URLs. (For normal images, it might be easier since we
can access the src URL using imageSourceURL() during layout/paint or
so.)

 };
 
 DEFINE_STYLE_IMAGE_TYPE_CASTS(StyleFetchedImageSet, isImageResourceSet());