Bug 219808

Summary: Introduce RenderLayerScrollableArea
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: Layout and RenderingAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, annulen, bfulgham, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, gyuyoung.kim, kondapallykalyan, mjs, pdr, ryuan.choi, sergio, simon.fraser, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch none

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.