Support fragment URLs for all kinds of SVG images

Source/core/fetch/ImageResource.cpp

Index: Source/core/fetch/ImageResource.cpp
diff --git a/Source/core/fetch/ImageResource.cpp b/Source/core/fetch/ImageResource.cpp
index 9151123f9e4c055c943f74353f60064f0818315c..c5882f793687e7158096669653c0f478a459dfa3 100644
--- a/Source/core/fetch/ImageResource.cpp
+++ b/Source/core/fetch/ImageResource.cpp
@@ -30,7 +30,6 @@
 #include "core/fetch/ResourceClientWalker.h"
 #include "core/fetch/ResourceFetcher.h"
 #include "core/fetch/ResourceLoader.h"
-#include "core/html/HTMLImageElement.h"
 #include "core/layout/LayoutObject.h"
 #include "core/svg/graphics/SVGImage.h"
 #include "core/svg/graphics/SVGImageForContainer.h"
@@ -212,7 +211,7 @@ blink::Image* ImageResource::imageForLayoutObject(const LayoutObject* layoutObje
     return m_image.get();
 }
 
-void ImageResource::setContainerSizeForLayoutObject(const ImageResourceClient* layoutObject, const IntSize& containerSize, float containerZoom)
+void ImageResource::setContainerParametersForLayoutObject(const ImageResourceClient* layoutObject, const IntSize& containerSize, float containerZoom, const KURL& urlWithFragment)

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

Is the fragment not visible at the Resource level? There's code to track it, at
least temporarily:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

If the super class has access to the fragment maybe we can use that instead or
relying on an explicit argument - then we could drop all the related plumbing.

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

m_fragmentIdentifierForRequest is emptied on ::load so not generally accessible
at paint time as the code looks today. Having the fragment at the resource level
seems wrong in general to me, since we want to use the same resource regardless
of how many urls with fragments that point to the same actual resource.

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

Yup, but I can't tell whether that part is needed/important (why only observe it
on the first load? do we even call load more than once?).
Makes sense.

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

Fwiw, on a simple test like
svg-resource-fragment-identifier-background.html attached in the CL, I
can't observe m_fragmentIdentifierForRequest actually being set.

Already in ResourceFetcher::requestResource there is a call to
removeFragmentIdentifierIfNeeded before hitting/checking the cache,
right before the resource is created (when not found in cache) so I'm
not sure when m_fragmentIdentifierForRequest is used.

 {
     if (containerSize.isEmpty())
         return;
@@ -227,7 +226,7 @@ void ImageResource::setContainerSizeForLayoutObject(const ImageResourceClient* l
 
     FloatSize containerSizeWithoutZoom(containerSize);
     containerSizeWithoutZoom.scale(1 / containerZoom);
-    m_imageForContainerMap->set(layoutObject, SVGImageForContainer::create(toSVGImage(m_image.get()), containerSizeWithoutZoom, containerZoom));
+    m_imageForContainerMap->set(layoutObject, SVGImageForContainer::create(toSVGImage(m_image.get()), containerSizeWithoutZoom, containerZoom, urlWithFragment));
 }
 
 bool ImageResource::usesImageContainerSize() const
@@ -525,13 +524,6 @@ Image* ImageResource::svgImageForLayoutObject(const LayoutObject* layoutObject)
     RefPtr<SVGImageForContainer> imageForContainer = it->value;
     ASSERT(!imageForContainer->size().isEmpty());
 
-    Node* node = layoutObject->node();
-    if (node && isHTMLImageElement(node)) {
-        const AtomicString& urlString = toHTMLImageElement(node)->imageSourceURL();
-        KURL url = node->document().completeURL(urlString);
-        imageForContainer->setURL(url);
-    }
-
     return imageForContainer.get();
 }