Bug 95002 – Crash when same SVG used as a CSS background AND drawn on canvas

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Lea Verou]

After reducing this bug for a painfully long time, I narrowed it down to the linked testcase, which should be sufficiently simple: If the same SVG file is drawn on a canvas AND used as a CSS background, the browser (or tab, in Canary) crashes. 

The testcase crashes both Chrome Canary and WebKit nightlies. The bug first appeared around 2±1 updates ago.

Things that do NOT seem to be relevant to the bug:

- The element where the SVG is applied
- The way to CSS is applied (JavaScript, inline style, linked etc)
- Whether the canvas is generated or pre-existing in the page
- The element that contains the canvas
- The dimensions of the canvas or SVG
- The SVG itself (tried with multiple)
- Other CSS properties that also accept <image> do not seem to trigger this (I tried content, border-image, cursor, list-style-image).

[Philip Rogers]

Thanks for filing this detailed test report and minimized testcase!

I'm already playing in this space so let me pick this up.

[Philip Rogers]

Created attachment 160741 [details]
Minimized testcase (1 of 2)

Just attaching the testcase

[Philip Rogers]

Created attachment 160743 [details]
Minimized testcase (2 of 2)

Just attaching the testcase

[Philip Rogers]

I think I see what's going on: we're detached and don't have a renderer (aka client) so we crash when trying to get the size for the CSS background.

And the backtrace/assert from a debug build:
ASSERTION FAILED: client
/Users/progers7/Desktop/webkit/Source/WebCore/svg/graphics/SVGImageCache.cpp(79) : SVGImageCache::SizeAndScales WebCore::SVGImageCache::requestedSizeAndScales(const WebCore::CachedImageClient *) const
1   0x105c998ad WebCore::SVGImageCache::requestedSizeAndScales(WebCore::CachedImageClient const*) const
2   0x1046e42f4 WebCore::CachedImage::imageSizeForRenderer(WebCore::RenderObject const*, float)
3   0x104710f84 _ZN7WebCoreL4sizeEPNS_16HTMLImageElementE
4   0x10471101d WebCore::CanvasRenderingContext2D::drawImage(WebCore::HTMLImageElement*, float, float, float, float, int&)
5   0x10506e761 _ZN7WebCoreL53jsCanvasRenderingContext2DPrototypeFunctionDrawImage2EPN3JSC9ExecStateE
6   0x10506db62 WebCore::jsCanvasRenderingContext2DPrototypeFunctionDrawImage(JSC::ExecState*)
7   0x10a62e265
8   0x10399fe84 JSC::JITCode::execute(JSC::RegisterFile*, JSC::ExecState*, JSC::JSGlobalData*)
9   0x10399cc5c JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
10  0x10384fbc8 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
11  0x10505ca92 WebCore::JSMainThreadExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
12  0x10518dc71 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*)
13  0x104bd9f17 WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul>&)
14  0x104bd9d7b WebCore::EventTarget::fireEventListeners(WebCore::Event*)
15  0x10565cb0b WebCore::Node::handleLocalEvents(WebCore::Event*)
16  0x104baaae4 WebCore::EventContext::handleLocalEvents(WebCore::Event*) const
17  0x104bad629 WebCore::EventDispatcher::dispatchEventAtTarget(WTF::PassRefPtr<WebCore::Event>)
18  0x104bac57d WebCore::EventDispatcher::dispatchEvent(WTF::PassRefPtr<WebCore::Event>)
19  0x104bb2264 WebCore::EventDispatchMediator::dispatchEvent(WebCore::EventDispatcher*) const
20  0x104bab4cc WebCore::EventDispatcher::dispatchEvent(WebCore::Node*, WTF::PassRefPtr<WebCore::EventDispatchMediator>)
21  0x10565cbfa WebCore::Node::dispatchEvent(WTF::PassRefPtr<WebCore::Event>)
22  0x104dd6c2c WebCore::HTMLImageLoader::dispatchLoadEvent()
23  0x104ead511 WebCore::ImageLoader::dispatchPendingLoadEvent()
24  0x104ead448 WebCore::ImageLoader::dispatchPendingEvent(WebCore::EventSender<WebCore::ImageLoader>*)
25  0x104eadb1c WebCore::EventSender<WebCore::ImageLoader>::dispatchPendingEvents()
26  0x104ead621 WebCore::ImageLoader::dispatchPendingLoadEvents()
27  0x10497f6d2 WebCore::Document::implicitClose()
28  0x104c83b0b WebCore::FrameLoader::checkCallImplicitClose()
29  0x104c83803 WebCore::FrameLoader::checkCompleted()
30  0x104c83975 WebCore::FrameLoader::loadDone()
31  0x1046fb0d2 WebCore::CachedResourceLoader::loadDone()

[Philip Rogers]

Created attachment 160830 [details]
Use SVGImage instead of cached image when drawing without a render tree.

Requesting a review from Nikolas on this one... I'm just getting into this code and this patch relies on a codepath that you removed from your haromize patch:

inline Image* CachedImage::lookupOrCreateImageForRenderer(const RenderObject* renderer)
{
...
    Image* useImage = m_svgImageCache->lookupOrCreateBitmapImageForRenderer(renderer);
    if (useImage == Image::nullImage())
        return m_image.get();
...

My understanding is that we should fall back to using SVGImage (aka m_image.get()) when we aren't in the render tree (and don't have a cached image). This means, to draw we just rely on SVGImage::draw().

[Nikolas Zimmermann]

Comment on attachment 160830 [details]
Use SVGImage instead of cached image when drawing without a render tree.

This still seems fine, as-is. r=me.

[WebKit Review Bot]

Comment on attachment 160830 [details]
Use SVGImage instead of cached image when drawing without a render tree.

Clearing flags on attachment: 160830

Committed r126977: <http://trac.webkit.org/changeset/126977>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.