Support fragment URLs for all kinds of SVG images

Source/core/style/StyleFetchedImageSet.h

 /*
  * Copyright (C) 2012 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 23 matching lines @@
 namespace blink {
 
 class ImageResource;
 class CSSImageSetValue;
 
 // This class keeps one cached image and has access to a set of alternatives.
 
 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;
 
     PassRefPtrWillBeRawPtr<CSSValue> cssValue() const override;
 
     // FIXME: This is used by StyleImage for equals comparison, but this implementation
     // only looks at the image from the set that we have loaded. I'm not sure if that is
     // meaningful enough or not.
     WrappedImagePtr data() const override { return m_bestFitImage.get(); }
 
@@ skipping 13 matching lines @@
     void addClient(LayoutObject*) override;
     void removeClient(LayoutObject*) override;
     PassRefPtr<Image> image(LayoutObject*, const IntSize&) const override;
     float imageScaleFactor() const override { return m_imageScaleFactor; }
     bool knownToBeOpaque(const LayoutObject*) const override;
     ImageResource* cachedImage() const override { return m_bestFitImage.get(); }
 
     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());
 
 } // namespace blink
 
 #endif // StyleFetchedImageSet_h