Bug 18673 – Crash in RenderImageGeneratedContent::imagePtr() using css content: with full page zoom

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Matt Lilek]

When a WebView has a full page zoom scale factor that isn't 1 (I'm assuming a normal page is 1), loading a page that uses css content: causes the browser to crash.  Besides the reduction, this affects the inspector if you try to switch panes with it zoomed.

Thread 0 Crashed:
0   com.apple.WebCore             	0x0232041c WebCore::RenderImageGeneratedContent::imagePtr() const + 22 (RenderImageGeneratedContent.h:56)
1   com.apple.WebCore             	0x020937e9 WebCore::RenderImage::intrinsicSizeChanged() + 39 (RenderImage.h:81)
2   com.apple.WebCore             	0x020c3a80 WebCore::RenderReplaced::setStyle(WebCore::RenderStyle*) + 152 (RenderReplaced.cpp:70)
3   com.apple.WebCore             	0x020b5ddf WebCore::RenderObject::createObject(WebCore::Node*, WebCore::RenderStyle*) + 225 (RenderObject.cpp:103)
4   com.apple.WebCore             	0x01e887ac WebCore::HTMLImageElement::createRenderer(WebCore::RenderArena*, WebCore::RenderStyle*) + 44 (HTMLImageElement.cpp:168)
5   com.apple.WebCore             	0x0202cd0d WebCore::Node::createRendererIfNeeded() + 409 (Node.cpp:1011)
6   com.apple.WebCore             	0x01df257d WebCore::Element::attach() + 17 (Element.cpp:719)
7   com.apple.WebCore             	0x01e86d37 WebCore::HTMLImageElement::attach() + 17 (HTMLImageElement.cpp:177)
8   com.apple.WebCore             	0x01eb1463 WebCore::HTMLParser::insertNode(WebCore::Node*, bool) + 857 (HTMLParser.cpp:344)
9   com.apple.WebCore             	0x01eb10f0 WebCore::HTMLParser::handleError(WebCore::Node*, bool, WebCore::AtomicString const&, int) + 7064 (HTMLParser.cpp:637)
10  com.apple.WebCore             	0x01eb1249 WebCore::HTMLParser::insertNode(WebCore::Node*, bool) + 319 (HTMLParser.cpp:318)
11  com.apple.WebCore             	0x01eb10f0 WebCore::HTMLParser::handleError(WebCore::Node*, bool, WebCore::AtomicString const&, int) + 7064 (HTMLParser.cpp:637)
12  com.apple.WebCore             	0x01eb1249 WebCore::HTMLParser::insertNode(WebCore::Node*, bool) + 319 (HTMLParser.cpp:318)
13  com.apple.WebCore             	0x01eb1d47 WebCore::HTMLParser::parseToken(WebCore::Token*) + 1445 (HTMLParser.cpp:254)
14  com.apple.WebCore             	0x01ec8d5c WebCore::HTMLTokenizer::processToken() + 598 (HTMLTokenizer.cpp:1897)
15  com.apple.WebCore             	0x01ecc026 WebCore::HTMLTokenizer::parseTag(WebCore::SegmentedString&, WebCore::HTMLTokenizer::State) + 6124 (HTMLTokenizer.cpp:1478)
16  com.apple.WebCore             	0x01eccbf9 WebCore::HTMLTokenizer::write(WebCore::SegmentedString const&, bool) + 1521 (HTMLTokenizer.cpp:1727)
17  com.apple.WebCore             	0x01e3be73 WebCore::FrameLoader::write(char const*, int, bool) + 1185 (FrameLoader.cpp:1018)
18  com.apple.WebCore             	0x01e3bfa8 WebCore::FrameLoader::addData(char const*, int) + 278 (FrameLoader.cpp:1834)
19  com.apple.WebKit              	0x001aec4d -[WebFrame(WebInternal) _addData:] + 157 (WebFrame.mm:486)
20  com.apple.WebKit              	0x001b2821 -[WebFrame(WebInternal) _receivedData:textEncodingName:] + 213 (WebFrame.mm:990)
21  com.apple.WebKit              	0x001c2024 -[WebHTMLRepresentation receivedData:withDataSource:] + 152 (WebHTMLRepresentation.mm:165)
22  com.apple.WebKit              	0x001a110a -[WebDataSource(WebInternal) _receivedData:] + 90 (WebDataSource.mm:199)
23  com.apple.WebKit              	0x001b6a46 WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 128 (WebFrameLoaderClient.mm:708)
24  com.apple.WebCore             	0x01e36b50 WebCore::FrameLoader::committedLoad(WebCore::DocumentLoader*, char const*, int) + 84 (FrameLoader.cpp:3329)
25  com.apple.WebCore             	0x01dd476f WebCore::DocumentLoader::commitLoad(char const*, int) + 87 (DocumentLoader.cpp:347)
26  com.apple.WebCore             	0x01dd497c WebCore::DocumentLoader::receivedData(char const*, int) + 76 (DocumentLoader.cpp:360)
27  com.apple.WebCore             	0x01e363f9 WebCore::FrameLoader::receivedData(char const*, int) + 41 (FrameLoader.cpp:2278)
28  com.apple.WebCore             	0x02019c7a WebCore::MainResourceLoader::addData(char const*, int, bool) + 80 (MainResourceLoader.cpp:144)
29  com.apple.WebCore             	0x0211c01d WebCore::ResourceLoader::didReceiveData(char const*, int, long long, bool) + 83 (ResourceLoader.cpp:248)
30  com.apple.WebCore             	0x02019ff8 WebCore::MainResourceLoader::didReceiveData(char const*, int, long long, bool) + 282 (MainResourceLoader.cpp:301)
31  com.apple.WebCore             	0x0211bbca WebCore::ResourceLoader::didReceiveData(WebCore::ResourceHandle*, char const*, int, int) + 62 (ResourceLoader.cpp:376)
32  com.apple.WebCore             	0x021191ed -[WebCoreResourceHandleAsDelegate connection:didReceiveData:lengthReceived:] + 201 (ResourceHandleMac.mm:502)
33  com.apple.Foundation          	0x96f673b7 -[NSURLConnection(NSURLConnectionReallyInternal) sendDidReceiveData:originalLength:] + 119
34  com.apple.Foundation          	0x96f6731e _NSURLConnectionDidReceiveData + 94
35  com.apple.CFNetwork           	0x940510af sendDidReceiveDataCallback + 518
36  com.apple.CFNetwork           	0x9404e76d _CFURLConnectionSendCallbacks + 1559
37  com.apple.CFNetwork           	0x9404e0d9 muxerSourcePerform + 283
38  com.apple.CoreFoundation      	0x9648b62e CFRunLoopRunSpecific + 3166
39  com.apple.CoreFoundation      	0x9648bd18 CFRunLoopRunInMode + 88
40  com.apple.HIToolbox           	0x958ab6a0 RunCurrentEventLoopInMode + 283
41  com.apple.HIToolbox           	0x958ab3f2 ReceiveNextEventCommon + 175
42  com.apple.HIToolbox           	0x958ab32d BlockUntilNextEventMatchingListInMode + 106
43  com.apple.AppKit              	0x91ec17d9 _DPSNextEvent + 657
44  com.apple.AppKit              	0x91ec108e -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128
45  com.apple.Safari              	0x00007f2e 0x1000 + 28462
46  com.apple.AppKit              	0x91eba0c5 -[NSApplication run] + 795
47  com.apple.AppKit              	0x91e8730a NSApplicationMain + 574
48  com.apple.Safari              	0x000b9906 0x1000 + 755974

[Adam Roben (:aroben)]

<rdar://problem/5875432>

[Julien Chaffraix]

Created attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent

[Eric Seidel (no email)]

Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent 

None of the other accessors NULL-check, why should this one?  OR why shouldn't they all?

Also, if there is no way to force DRT to use full page zoom (maybe it already does?  I'm not sure what zoom:200% does in webkit, if anything), then this should be a manual-test.

Ideally hyatt should comment on why RenderImageGeneratedContent doesn't null-check...

[Dave Hyatt]

Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent 

The URL in this bug does not crash for me.  Which OS and build  type (debug vs. release) are you experiencing this crash on?

[Julien Chaffraix]

(In reply to comment #4)
> (From update of attachment 22418 [details] [edit])
> The URL in this bug does not crash for me.  Which OS and build  type (debug vs.
> release) are you experiencing this crash on?
> 

I have tried with a nightly and ToT/debug. You have to change the zoom factor if you want the page to crash. You may also set the option to activate the full page zoom.

[Julien Chaffraix]

Taking a closer look at the code, it seems that RenderImage::intrisicSizeChanged() is called before the m_styleImage is set in RenderImageGeneratedContent which lead to the crash.

The null check I have added works because it is the only method that uses m_styleImage called when executing RenderImage::intrisicSizeChanged(), which means the current patch is not solving the core of the issue.

A solution could be to override intrisicSizeChanged() in RenderImageGeneratedContent to do the null check.

[Eric Seidel (no email)]

Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent 

Setting this explicitly for hyatt to review.

[Dave Hyatt]

Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent 

Not correct.  Right fix is not to call intrinsicStyleChanged when you don't have two non-null styles.

Patch coming.

[Dave Hyatt]

Created attachment 22468 [details]
Patch to fix problem.

[Oliver Hunt]

Comment on attachment 22468 [details]
Patch to fix problem.

is a  layout test possible?

[Dave Hyatt]

Fixed in r35327.