Bug 199348 – Use separate variables for moving and stationary scrolling relationships in RemoteLayerTreeNode

• Status: RESOLVED
• Resolution: FIXED
• Component: Scrolling

[Antti Koivisto]

A layer can have only one acting scroll parent. Not using a vector for that case makes the code clearer.

[Antti Koivisto]

Created attachment 373190 [details]
patch

[Antti Koivisto]

Created attachment 373195 [details]
patch

[Darin Adler]

Comment on attachment 373195 [details]
patch

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

While I’m not a real expert on this code, this looks good to me.

> Source/WebCore/page/scrolling/ScrollingTree.h:187
> +    HashSet<RefPtr<ScrollingTreeOverflowScrollProxyNode>> m_activeOverflowScrollProxyNodes;
> +    HashSet<RefPtr<ScrollingTreePositionedNode>> m_activePositionedNodes;

A number of places elsewhere use HashSet<Ref<>> rather than HashSet<RefPtr<>>. Is there a reason to prefer RefPtr here?

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:134
> +                return (WKChildScrollView *)actingParent->uiView();

Not new, but: Would be nice to have a type checking cast like downcast<> here for clarity/assertion. Also unclear to me how solid the guarantee behind this typecast is.

[Antti Koivisto]

Created attachment 373210 [details]
patch

[Antti Koivisto]

Created attachment 373211 [details]
patch

[Antti Koivisto]

> A number of places elsewhere use HashSet<Ref<>> rather than
> HashSet<RefPtr<>>. Is there a reason to prefer RefPtr here?

No reason, I just didn't remember it works.

[WebKit Commit Bot]

Comment on attachment 373211 [details]
patch

Clearing flags on attachment: 373211

Committed r246962: <https://trac.webkit.org/changeset/246962>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52441382>