Bug 55643

Summary: Chromium unnecessarily creates SVGImage when an SVG document contains images
Product: WebKit Reporter: Xianzhu Wang <wangxianzhu>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: abarth, brettw, dglazkov, eric, fishd, schenney, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Description Flags
Archive of layout-test-results from ec2-cr-linux-02 none

Description Xianzhu Wang 2011-03-02 18:50:00 PST
When debugging bug 55017, I found that on chromium-linux (maybe also on other platforms) unnecessarily creates an SVGImage object when an SVG document contains images. When the first image is loaded, an SVGImage object will be created and the SVG document itself will be loaded as the content of the image.
Comment 1 Xianzhu Wang 2011-03-02 20:08:15 PST
The problem is because of the different implementations of KURL(base, relative).string() when base is an empty string. The WebKit version of KURL returns relative, while Google version of KURL returns an empty string. The former looks more correct to me.

The steps of how the problem occurs are as follows:
1) When the parser has parsed the src attribute of an image element in an SVG document, it calls ImageLoader::updateFromElement() of an SVGImageLoader.
2) The URL is got from the attribute value with SVGImageLoader::sourceURI() which in turn calls KURL(element()->base(), stripLeadingAndTrailingHTMLSpaces(attr)).string)
3) Google version of KURL returns an empty URL. The document.completeURL returns the URL of the SVG document itself. The CachedResourceLoader then will request the SVG document as the image instead of the correct image file.

As the WebKit version of KURL returns the original relative URL if base is empty, the above next steps run normally without errors.
Comment 2 Xianzhu Wang 2011-03-02 20:26:10 PST
Just accidentally pressed Enter key before completed the last comment. Continued.

As the SVG document itself is loaded as the image, an SVGImage object is created and put into the cache and returned as the loaded image. Though this is incorrect, the ImageLoader will load the image again when the element has a non-empty base URL and this time the correct image will be loaded and returned.

Seems we should fix KURLGoogle to let it behave the same as the original KURL. Is this the correct solution?

HTMLImageLoader::sourceURI() just returns the trimmed input attribute. Why does SVGImageLoader::sourceURI() require the base url?
Comment 3 Xianzhu Wang 2011-05-04 02:30:17 PDT
Created attachment 92204 [details]
Comment 4 Xianzhu Wang 2011-05-04 17:55:05 PDT
Now requesting for review. Thanks!
Comment 5 Xianzhu Wang 2011-05-04 18:03:37 PDT
Comment on attachment 92204 [details]

According to discussions in this thread:
We should make the fix in google-url rather than in KURLGoogle.cpp.

So I withdraw review request of this patch.
Comment 6 Xianzhu Wang 2011-05-04 18:54:32 PDT
googleurl bug filed: http://code.google.com/p/google-url/issues/detail?id=23
Comment 7 WebKit Review Bot 2011-05-04 19:43:57 PDT
Attachment 92204 [details] did not pass chromium-ews:
Output: http://queues.webkit.org/results/8552777
New failing tests:
Comment 8 WebKit Review Bot 2011-05-04 19:44:07 PDT
Created attachment 92363 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-22-virtual-x86_64-with-Ubuntu-10.10-maverick