Bug 194160 – Don't use base layer() as the scroll layer in scrolling tree.

• Component: Scrolling
• Status: RESOLVED FIXED

[Antti Koivisto]

Maintain scrollContainerLayer() and scrolledContestsLayer() separately.

[Antti Koivisto]

Created attachment 360864 [details]
patch

[Antti Koivisto]

Created attachment 360869 [details]
patch

[Tim Horton]

Comment on attachment 360869 [details]
patch

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

> Source/WebCore/ChangeLog:8
> +        Maintain scrollContainerLayer() and scrolledContestsLayer() separately in ScrollingTreeScrollingNode.

scrolledContestsLayer.

Contests on the mind?

[Antti Koivisto]

> scrolledContestsLayer.
> 
> Contests on the mind?

The patch also used to feature a "contentainer layer"

[Simon Fraser (smfr)]

Comment on attachment 360869 [details]
patch

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

> Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:143
> +#if PLATFORM(COCOA)
> +    RetainPtr<CALayer> m_scrollContainerLayer;
> +    RetainPtr<CALayer> m_scrolledContentsLayer;
> +#endif

This is breaking the platform-neutrality of this class. Maybe that's OK? Or maybe we should have some kind of platform mixin for the layer management?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:556
> +            nodeLayers = { layer.backing()->graphicsLayer(), backing->scrollingLayer(), backing->scrollingContentsLayer() };

Do you want to rename backing->scrollingLayer() in this patch?

[Antti Koivisto]

> This is breaking the platform-neutrality of this class. Maybe that's OK? Or
> maybe we should have some kind of platform mixin for the layer management?

It seemed ok to me, avoids duplication and makes the structure clearer. If we want to pile in more then maybe some architectural solution would be nicer.

> Do you want to rename backing->scrollingLayer() in this patch?

Next patch, this is already getting unwieldy.

[Antti Koivisto]

Created attachment 360876 [details]
patch

[WebKit Commit Bot]

Comment on attachment 360876 [details]
patch

Rejecting attachment 360876 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 360876, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebKit/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/10996548

[Antti Koivisto]

Created attachment 360878 [details]
patch

[WebKit Commit Bot]

Comment on attachment 360878 [details]
patch

Clearing flags on attachment: 360878

Committed r240861: <https://trac.webkit.org/changeset/240861>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47745047>