Bug 88475 – Optimize FrameView::scrollXForFixedPosition() / scrollYForFixedPosition()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Optimize FrameView::scrollXForFixedPosition() / scrollYForFixedPosition()

[Simon Fraser (smfr)]

Created attachment 146160 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/11612205>

[Sam Weinig]

Comment on attachment 146160 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.h:114
> +    void move(LayoutSize size) { m_rect.move(size); }

This should probably be passed as a const& ref.

[Pratik Solanki]

Comment on attachment 146160 [details]
Patch

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

> Source/WebCore/page/FrameView.h:204
> -    int scrollXForFixedPosition() const;
> -    int scrollYForFixedPosition() const;
> +    int scrollXForFixedPosition() const { return scrollOffsetForFixedPosition().width(); }
> +    int scrollYForFixedPosition() const { return scrollOffsetForFixedPosition().height(); }

Best to just remove scrollXForFixedPosition() and scrollYForFixedPosition() so no one does something like IntSize(scrollXForFixedPosition(), scrollYForFixedPosition()) in the future.

[Pratik Solanki]

Comment on attachment 146160 [details]
Patch

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

>> Source/WebCore/page/FrameView.h:204
>> +    int scrollYForFixedPosition() const { return scrollOffsetForFixedPosition().height(); }
> 
> Best to just remove scrollXForFixedPosition() and scrollYForFixedPosition() so no one does something like IntSize(scrollXForFixedPosition(), scrollYForFixedPosition()) in the future.

And if you do remove them, delete the functions from WebCore.order file as well.

(In reply to comment #5)

> And if you do remove them, delete the functions from WebCore.order file as well.

We usually leave the order file alone when making code changes.

[Simon Fraser (smfr)]

I removed them.
http://trac.webkit.org/changeset/119736

[Antti Koivisto]

We should probably cache the platform scroll position so that it is queried only once during a single layout/style recalc.