REGRESSION(r95573): Crash when loading SVG documents in a flattened frame or any SVG document in Chromium/Mac.

• Component: New Bugs
• Status: RESOLVED FIXED

[Dimitri Glazkov (Google)]

[Chromium/Mac] REGRESSION(r95573): All SVG documents crash on loading.

[Dimitri Glazkov (Google)]

Created attachment 108901 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 108901 [details]
Patch

needs more testing.

[Dimitri Glazkov (Google)]

Created attachment 109028 [details]
Patch

[Dave Hyatt]

Comment on attachment 109028 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109028&action=review

Really does seem odd that Chromium is calling this on the RenderView... how do you even get the answer you want? I'm not convinced this is even doing what you think it's doing.

> Source/WebCore/rendering/RenderBlock.cpp:4897
> -    LayoutUnit cw = containingBlock()->contentLogicalWidth();
> +    LayoutUnit cw = containingBlockOrSelf(this)->contentLogicalWidth();

I wouldn't do it this way, since it doesn't really make any sense. This value really shouldn't even be used by anything since the RenderView can't have percentage heights/widths specified on it, so null checking should be better.

RenderBlock* cb = containingBlock();
LayoutUnit cw = cb ? cb->contentLogicalWidth() : 0;

I'd prefer that.

[Dimitri Glazkov (Google)]

Created attachment 109062 [details]
Patch

[Dimitri Glazkov (Google)]

(In reply to comment #4)
> (From update of attachment 109028 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109028&action=review
> 
> Really does seem odd that Chromium is calling this on the RenderView... how do you even get the answer you want? I'm not convinced this is even doing what you think it's doing.

Added test for flattened frames which also causes the same crash.

> 
> > Source/WebCore/rendering/RenderBlock.cpp:4897
> > -    LayoutUnit cw = containingBlock()->contentLogicalWidth();
> > +    LayoutUnit cw = containingBlockOrSelf(this)->contentLogicalWidth();
> 
> I wouldn't do it this way, since it doesn't really make any sense. This value really shouldn't even be used by anything since the RenderView can't have percentage heights/widths specified on it, so null checking should be better.
> 
> RenderBlock* cb = containingBlock();
> LayoutUnit cw = cb ? cb->contentLogicalWidth() : 0;
> 
> I'd prefer that.

Done!

[Dave Hyatt]

Comment on attachment 109062 [details]
Patch

r=me

[Dimitri Glazkov (Google)]

Committed r96258: <http://trac.webkit.org/changeset/96258>