[webkit-reviews] review requested: [Bug 60305] Separate scrolling code out of RenderLayer : [Attachment 414792] Patch, v5 (squashed for testing)

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has asked  for review:
Bug 60305: Separate scrolling code out of RenderLayer
https://bugs.webkit.org/show_bug.cgi?id=60305

Attachment 414792: Patch, v5 (squashed for testing)

https://bugs.webkit.org/attachment.cgi?id=414792&action=review




--- Comment #32 from Nikolas Zimmermann <zimmermann at kde.org> ---
Created attachment 414792

  --> https://bugs.webkit.org/attachment.cgi?id=414792&action=review

Patch, v5 (squashed for testing)

NOTE: Set r? to trigger EWS - don't know if that's necessary, not intended for
a real review yet.

"Patch, v5" actually consists of four independent patches, that can be
reviewed/landed separated.
I squashed them together to obtain EWS results first for the final patch, since
I still expect regressions on Mac/Win etc. that need to be checked first. Once
EWS shows no regressions, I'll start uploading the individual patch chunks for
review. Feel free to check the patch nevertheless. It is based on the previous
iterations + discussions after the meeting with Simon, Said, etc.

The four independent patches are:
1) Introduce RenderLayerScrollableArea stub (build system changes, etc.)
2) Move overflow/scrolling functionality from RenderLayer ->
RenderLayerScrollableArea.
   Introduce temporary glue code: void RenderLayer::foo() { if
(m_scrollableArea) m_scrollableArea->foo(); }
   to minimize the changes to WebCore in general and ease review.
3) Remove glue code, make WebCore aware of RenderLayerScrollableArea
4) Enable dynamic creation of RenderLayerScrollableArea, only when it is
needed!
   This is the tricky part that kept me busy for a long while.

If a scrollable area is not needed for a RenderLayer, the RenderLayer will now
consume less memory than before.
I measured this to be performance and memory-neutral on two benchmarks:
MotionMark, Speedometer.

While the size reduction of RenderLayer is significant, it is negligible as
soon as the layer needs a backing store...
Thus I expect this to have little impact on memory consumption in "real-life"
use-cases. Nevertheless it simplifies the code a lot, and opens possibilities
for further refactoring & cleanup that are the main motivation for this work:
e.g. introduce RenderLayerHTML, further cleaning up RenderLayer.

The final goal is to make RenderLayer less expensive to use, to allow to be
reused for SVG.
I have a PoC branch where SVG rendering/hit-testing/etc. goes through
RenderLayer, which opens up nice possibilites, such as 3D transformations, GPU
composition etc. However on static rendering it currently regresses
performance, due to the RenderLayer cost.

Disclaimer: I just finished rebasing this on ToT -- it was ~ 2 months behind.
Only ran a limited set of layout tests, as I want to get this patch out quickly
to get some early results before EWS is shut-down due to the maintenance tasks
this week.
.