[Webkit-unassigned] [Bug 135769] Implement snapping behavior for iOS

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=135769





--- Comment #2 from Simon Fraser (smfr) <simon.fraser at apple.com>  2014-08-13 16:52:37 PST ---
(From update of attachment 236563)
View in context: https://bugs.webkit.org/attachment.cgi?id=236563&action=review

Looks good other than the encoding/decoding.

> Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:62
> +    Vector<LayoutUnit> horizontalSnapOffsets() const { return m_horizontalSnapOffsets; }
> +    Vector<LayoutUnit> verticalSnapOffsets() const { return m_verticalSnapOffsets; }

Please return a const reference.

> Source/WebCore/rendering/RenderLayer.cpp:3041
> +    

Whitespace.

> Source/WebCore/rendering/RenderLayer.cpp:3316
> +#if ENABLE(CSS_SCROLL_SNAP)
> +    // FIXME: Ensure that offsets are also updated in case of programmatic style changes.
> +    updateSnapOffsets();
> +#endif

Shouldn't this be in an earlier patch?

> Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:135
> +        encoder << static_cast<uint64_t>(node.horizontalSnapOffsets().size());
> +        for (LayoutUnit position : node.horizontalSnapOffsets())
> +            encoder << position.toDouble();

Can't you just encode the Vector directly?

> Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:222
> +    if (node.hasChangedProperty(ScrollingStateScrollingNode::HorizontalSnapOffsets)) {
> +        if (!decoder.decode(size))
> +            return false;
> +
> +        Vector<LayoutUnit> horizontalSnapOffsets;
> +        for (size_t i = 0; i < size; i++) {
> +            double position = 0;
> +            if (!decoder.decode(position))
> +                return false;
> +
> +            horizontalSnapOffsets.append(position);
> +        }
> +        node.setHorizontalSnapOffsets(horizontalSnapOffsets);

Similarly, I think the vector should just be decodable directly.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.