Bug 88475 - Optimize FrameView::scrollXForFixedPosition() / scrollYForFixedPosition()
Summary: Optimize FrameView::scrollXForFixedPosition() / scrollYForFixedPosition()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-06-06 17:36 PDT by Simon Fraser (smfr)
Modified: 2012-06-07 14:10 PDT (History)
7 users (show)

See Also:


Attachments
Patch (11.79 KB, patch)
2012-06-06 17:44 PDT, Simon Fraser (smfr)
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-06-06 17:36:20 PDT
Optimize FrameView::scrollXForFixedPosition() / scrollYForFixedPosition()
Comment 1 Simon Fraser (smfr) 2012-06-06 17:44:07 PDT
Created attachment 146160 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2012-06-06 17:48:16 PDT
<rdar://problem/11612205>
Comment 3 Sam Weinig 2012-06-06 18:14:05 PDT
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.
Comment 4 Pratik Solanki 2012-06-06 18:25:03 PDT
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.
Comment 5 Pratik Solanki 2012-06-06 18:27:26 PDT
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.
Comment 6 mitz 2012-06-06 18:36:13 PDT
(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.
Comment 7 Simon Fraser (smfr) 2012-06-07 10:57:22 PDT
I removed them.
http://trac.webkit.org/changeset/119736
Comment 8 Antti Koivisto 2012-06-07 14:10:59 PDT
We should probably cache the platform scroll position so that it is queried only once during a single layout/style recalc.