[webkit-reviews] review denied: [Bug 68938] [Chromium/Mac] REGRESSION(r95573): All SVG documents crash on loading. : [Attachment 109028] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 68938: [Chromium/Mac] REGRESSION(r95573): All SVG documents crash on
loading.
https://bugs.webkit.org/show_bug.cgi?id=68938

Attachment 109028: Patch
https://bugs.webkit.org/attachment.cgi?id=109028&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
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.