Bug 219808 - Introduce RenderLayerScrollableArea
Summary: Introduce RenderLayerScrollableArea
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-11 16:02 PST by Nikolas Zimmermann
Modified: 2020-12-30 15:02 PST (History)
18 users (show)

See Also:


Attachments
Patch (8.96 KB, patch)
2020-12-11 16:08 PST, Nikolas Zimmermann
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (9.36 KB, patch)
2020-12-11 16:28 PST, Nikolas Zimmermann
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (10.58 KB, patch)
2020-12-12 14:01 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2020-12-11 16:02:56 PST
All overflow/scroll related functionality should be moved out of RenderLayer, to cleanup and simplify the layer code.
This allows to optimize for the case where a layer is not scrollable: we no longer have to keep track of many scrolling
related conditions/variables and save a good amount of memory per layer.

Plan:
1) Introduce RenderLayerScrollableArea inheriting from ScrollableArea, with a back-reference to RenderLayer -- that mimics the design of RenderLayerFilters.
2) RenderLayer no longer needs to inherit from ScrollableArea, instead it stores a RenderLayerScrollableArea object.
3) Move all overflow/scroll/marquee/resize/... handling from RenderLayer to RenderLayerScrollableArea
4) Only create RenderLayerScrollableArea objects if necessary!
   This requires heavy testing of the regular and composited code paths, which I had to perform in the past weeks.

--

This ticket adds a RenderLayerScrollableArea stub implementation and adds it to all build systems.
Comment 1 Nikolas Zimmermann 2020-12-11 16:08:19 PST
Created attachment 416061 [details]
Patch
Comment 2 Nikolas Zimmermann 2020-12-11 16:28:45 PST
Created attachment 416067 [details]
Patch
Comment 3 Tim Horton 2020-12-11 16:41:28 PST
Comment on attachment 416067 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerScrollableArea.cpp:19
> + * of either the Mozilla Public License Version 1.1, found at

This is a pretty odd license for WebKit, is it intentional? We tend to prefer BSD-2; https://webkit.org/licensing-webkit/ mentions the LGPL, and there is some LGPL code, but not sure about this MPL bit. I would check with someone to make sure this is right!
Comment 4 Nikolas Zimmermann 2020-12-11 16:52:51 PST
(In reply to Tim Horton from comment #3)
> Comment on attachment 416067 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=416067&action=review
> 
> > Source/WebCore/rendering/RenderLayerScrollableArea.cpp:19
> > + * of either the Mozilla Public License Version 1.1, found at
> 
> This is a pretty odd license for WebKit, is it intentional? We tend to
> prefer BSD-2; https://webkit.org/licensing-webkit/ mentions the LGPL, and
> there is some LGPL code, but not sure about this MPL bit. I would check with
> someone to make sure this is right!

I copy & pasted the license from RenderLayer, since RenderLayerScrollableArea will contain moved code from RenderLayer mostly, with slight adaptions (s/this/m_layer/).

I assumed that I would need to preserve the original license, no?
I'm not an expert on this topic, please enlighten me if that's incorrect.
Comment 5 Tim Horton 2020-12-11 16:55:56 PST
(In reply to Nikolas Zimmermann from comment #4)
> (In reply to Tim Horton from comment #3)
> > Comment on attachment 416067 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=416067&action=review
> > 
> > > Source/WebCore/rendering/RenderLayerScrollableArea.cpp:19
> > > + * of either the Mozilla Public License Version 1.1, found at
> > 
> > This is a pretty odd license for WebKit, is it intentional? We tend to
> > prefer BSD-2; https://webkit.org/licensing-webkit/ mentions the LGPL, and
> > there is some LGPL code, but not sure about this MPL bit. I would check with
> > someone to make sure this is right!
> 
> I copy & pasted the license from RenderLayer, since
> RenderLayerScrollableArea will contain moved code from RenderLayer mostly,
> with slight adaptions (s/this/m_layer/).
> 
> I assumed that I would need to preserve the original license, no?
> I'm not an expert on this topic, please enlighten me if that's incorrect.

Ah! In that case I think you probably want to pull the full copyright header along too.
Comment 6 Nikolas Zimmermann 2020-12-11 17:00:01 PST
(In reply to Tim Horton from comment #5)
> Ah! In that case I think you probably want to pull the full copyright header
> along too.

Fair enough, will include that in the next variant -- this doesn't build with clang atm anyhow :-)
Comment 7 Nikolas Zimmermann 2020-12-12 14:01:30 PST
Created attachment 416108 [details]
Patch
Comment 8 Radar WebKit Bug Importer 2020-12-18 16:04:47 PST
<rdar://problem/72484180>
Comment 9 Nikolas Zimmermann 2020-12-30 15:02:15 PST
Comment on attachment 416108 [details]
Patch

Clearing flags on attachment: 416108

Committed r271111: <https://trac.webkit.org/changeset/271111>
Comment 10 Nikolas Zimmermann 2020-12-30 15:02:23 PST
All reviewed patches have been landed.  Closing bug.