Bug 220851

Summary: Finish introduction of RenderLayerScrollableArea: remove remaining glue code from RenderLayer
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: Layout and RenderingAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, bfulgham, cdumez, cfleizach, changseok, dmazzoni, esprehn+autocc, ews-watchlist, fred.wang, glenn, gyuyoung.kim, jcraig, jdiggs, kondapallykalyan, mifenton, mmaxfield, pdr, samuel_white, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 60305    
Attachments:
Description Flags
Patch, v1
none
Patch, v2
none
Patch, v3 simon.fraser: review+

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>