Bug 194160 - Don't use base layer() as the scroll layer in scrolling tree.
Summary: Don't use base layer() as the scroll layer in scrolling tree.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-01 08:58 PST by Antti Koivisto
Modified: 2019-02-01 12:11 PST (History)
4 users (show)

See Also:


Attachments
patch (48.13 KB, patch)
2019-02-01 09:04 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (48.22 KB, patch)
2019-02-01 09:48 PST, Antti Koivisto
simon.fraser: review+
Details | Formatted Diff | Diff
patch (48.22 KB, patch)
2019-02-01 11:25 PST, Antti Koivisto
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch (48.22 KB, patch)
2019-02-01 11:32 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2019-02-01 08:58:49 PST
Maintain scrollContainerLayer() and scrolledContestsLayer() separately.
Comment 1 Antti Koivisto 2019-02-01 09:04:48 PST
Created attachment 360864 [details]
patch
Comment 2 Antti Koivisto 2019-02-01 09:48:15 PST
Created attachment 360869 [details]
patch
Comment 3 Tim Horton 2019-02-01 11:01:34 PST
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?
Comment 4 Antti Koivisto 2019-02-01 11:05:35 PST
> scrolledContestsLayer.
> 
> Contests on the mind?

The patch also used to feature a "contentainer layer"
Comment 5 Simon Fraser (smfr) 2019-02-01 11:13:01 PST
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?
Comment 6 Antti Koivisto 2019-02-01 11:18:27 PST
> 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.
Comment 7 Antti Koivisto 2019-02-01 11:25:41 PST
Created attachment 360876 [details]
patch
Comment 8 WebKit Commit Bot 2019-02-01 11:29:30 PST
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
Comment 9 Antti Koivisto 2019-02-01 11:32:57 PST
Created attachment 360878 [details]
patch
Comment 10 WebKit Commit Bot 2019-02-01 12:10:29 PST
Comment on attachment 360878 [details]
patch

Clearing flags on attachment: 360878

Committed r240861: <https://trac.webkit.org/changeset/240861>
Comment 11 WebKit Commit Bot 2019-02-01 12:10:31 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2019-02-01 12:11:28 PST
<rdar://problem/47745047>