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+

Nikolas Zimmermann
Reported 2021-01-22 02:25:01 PST
Finish introduction of RenderLayerScrollableArea: remove remaining glue code from RenderLayer
Attachments
Patch, v1 (43.99 KB, patch)
2021-01-22 02:34 PST, Nikolas Zimmermann
no flags
Patch, v2 (44.03 KB, patch)
2021-01-22 13:14 PST, Nikolas Zimmermann
no flags
Patch, v3 (70.23 KB, patch)
2021-01-22 16:20 PST, Nikolas Zimmermann
simon.fraser: review+
Nikolas Zimmermann
Comment 1 2021-01-22 02:34:01 PST
Created attachment 418124 [details] Patch, v1
Nikolas Zimmermann
Comment 2 2021-01-22 13:14:25 PST
Created attachment 418174 [details] Patch, v2
Simon Fraser (smfr)
Comment 3 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
Nikolas Zimmermann
Comment 4 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.
Nikolas Zimmermann
Comment 5 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).
Nikolas Zimmermann
Comment 6 2021-01-22 16:20:05 PST
Created attachment 418195 [details] Patch, v3
Nikolas Zimmermann
Comment 7 2021-01-25 14:31:19 PST
Radar WebKit Bug Importer
Comment 8 2021-01-25 14:32:19 PST
Note You need to log in before you can comment on or make changes to this bug.