[webkit-reviews] review granted: [Bug 86551] Kill RenderLayer::relativePositionOffset(LayoutUnit& relX, LayoutUnit& relY) : [Attachment 142305] Patch v2: Improved after Darin's comments + added more explanation on hasLayer() in ChangeLog.

[bugzilla-daemon at webkit.org]

Abhishek Arya <inferno at chromium.org> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 86551: Kill RenderLayer::relativePositionOffset(LayoutUnit& relX,
LayoutUnit& relY)
https://bugs.webkit.org/show_bug.cgi?id=86551

Attachment 142305: Patch v2: Improved after Darin's comments + added more
explanation on hasLayer() in ChangeLog.
https://bugs.webkit.org/attachment.cgi?id=142305&action=review

------- Additional Comments from Abhishek Arya <inferno at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=142305&action=review


r=me. please do bring the null check back.

> Source/WebCore/ChangeLog:3
> +	   Kill RenderLayer::relativePositionOffset(LayoutUnit& relX,
LayoutUnit& relY)

Do you want to add "and cleanup RenderInline::clippedOverflowRectForRepaint".

> Source/WebCore/ChangeLog:8
> +	   No expected change in behavior.

Probably like 'No new tests since there is no expected change in behavior.'

> Source/WebCore/ChangeLog:14
> +	   variable and removed 2 unneeded NULL-checks:

s/variable/variables.
s/NULL/null

> Source/WebCore/ChangeLog:16
> +	       - we ensure that a RenderInlie with position: relative has a
RenderLayer

typo RenderInlie. however we won't need this. see comment below.

> Source/WebCore/rendering/RenderInline.cpp:947
> +	   if (inlineFlow->style()->position() == RelativePosition)

I think we should bring the hasLayer() check back. If some new class (like svg)
inherits from renderinline and overrides hasLayer() and does not need the
isRelPositioned() check, we will crash with a null.