Bug 220851 - Finish introduction of RenderLayerScrollableArea: remove remaining glue code from RenderLayer
Summary: Finish introduction of RenderLayerScrollableArea: remove remaining glue code ...
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: 60305
  Show dependency tree / graph
 
Reported: 2021-01-22 02:25 PST by Nikolas Zimmermann
Modified: 2021-01-25 14:32 PST (History)
22 users (show)

See Also:

Attachments
Patch, v1 (43.99 KB, patch)
2021-01-22 02:34 PST, Nikolas Zimmermann 		no flags Details | Formatted Diff | Diff
Patch, v2 (44.03 KB, patch)
2021-01-22 13:14 PST, Nikolas Zimmermann 		no flags Details | Formatted Diff | Diff
Patch, v3 (70.23 KB, patch)
2021-01-22 16:20 PST, Nikolas Zimmermann 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2021-01-22 02:25:01 PST 
Finish introduction of RenderLayerScrollableArea: remove remaining glue code from RenderLayer
Comment 1 Nikolas Zimmermann 2021-01-22 02:34:01 PST 
Created attachment 418124 [details]
Patch, v1
Comment 2 Nikolas Zimmermann 2021-01-22 13:14:25 PST 
Created attachment 418174 [details]
Patch, v2
Comment 3 Simon Fraser (smfr) 2021-01-22 14:50:05 PST 
Comment on attachment 418174 [details]
Patch, v2

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

> Source/WebCore/page/EventHandler.cpp:1499
> +                    return layer->renderer().shouldPlaceBlockDirectionScrollbarOnLeft() ? southWestResizeCursor() : southEastResizeCursor();

Isn't layer->renderer() just renderer?

> Source/WebCore/page/FrameView.cpp:2601
> +            if (auto* scrollableLayer = renderView->layer()->scrollableArea())
> +                scrollableLayer->updateLayerPositionsAfterDocumentScroll();

Is it a scrollableLayer or scrollableArea? Pick one! I think all of these would read better as layerScrollableArea or just scrollableArea
Comment 4 Nikolas Zimmermann 2021-01-22 15:50:09 PST 
(In reply to Simon Fraser (smfr) from comment #3)
> Isn't layer->renderer() just renderer?
Fixed.

> > Source/WebCore/page/FrameView.cpp:2601
> > +            if (auto* scrollableLayer = renderView->layer()->scrollableArea())
> > +                scrollableLayer->updateLayerPositionsAfterDocumentScroll();
> 
> Is it a scrollableLayer or scrollableArea? Pick one! I think all of these
> would read better as layerScrollableArea or just scrollableArea
Okay, I propose to prepare a follow-up patch that consistently uses the 'scrollableArea' naming scheme instead of 'scrollableLayer' or variations.
Comment 5 Nikolas Zimmermann 2021-01-22 16:18:40 PST 
As discused on Slack, I'll prepare another polished version of the patch including the variable renames (s/scrollableLayer/scrollableArea/ in many places).
Comment 6 Nikolas Zimmermann 2021-01-22 16:20:05 PST 
Created attachment 418195 [details]
Patch, v3
Comment 7 Nikolas Zimmermann 2021-01-25 14:31:19 PST 
Committed r271814: <https://trac.webkit.org/changeset/r271814>
Comment 8 Radar WebKit Bug Importer 2021-01-25 14:32:19 PST 
<rdar://problem/73588759>