Bug 250891

Summary:

Do not layout for position changes in RenderWidget::setWidgetGeometry(...)

Product:

WebKit

Reporter:

Ahmad Saleem <ahmad.saleem792>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

bfulgham, simon.fraser, zalan

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Test Case - Use 'Timeline'	none

Description Ahmad Saleem 2023-01-20 03:28:52 PST

Created attachment 464573 [details]
Test Case - Use 'Timeline'

Hi Team,

While going through Blink's commits, I came across another potential win in performance and reducing jank etc.

Blink Commit - https://src.chromium.org/viewvc/blink?view=revision&revision=158500

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderWidget.cpp#138

```Commit```

Previously, a layout could be caused by a RenderWidget changing
position. For example, an iframe in a scrollable area would get called
to layout if the area scrolled. This occurred because of the the following
callstack:
FrameView calls
RenderWidget::updateWidgetPosition(...) which calls
RenderWidget::updateWidgetGeometry(...) which calls
RenderWidget::setWidgetGeometry(...)

setWidgetGeometry would return true and force a layout if the frame rect
changed position. This should only return true if the frame rect actually
resizes, so this patch updates setWidgetGeometry to return false (not
forcing a layout) if just the frame's position changes.

______

Just wanted to raise to get input whether this is something worth merging or exploring or we have plans to do any other things or this issue is not as bad on WebKit as Blink.

Thanks!

Comment 1 Ahmad Saleem 2023-01-20 03:33:43 PST

(In reply to Ahmad Saleem from comment #0)
> Created attachment 464573 [details]
> Test Case - Use 'Timeline'
> 
> Hi Team,
> 
> While going through Blink's commits, I came across another potential win in
> performance and reducing jank etc.
> 
> Blink Commit -
> https://src.chromium.org/viewvc/blink?view=revision&revision=158500
> 
> WebKit Source -
> https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderWidget.
> cpp#138
> 
> ```Commit```
> 
> Previously, a layout could be caused by a RenderWidget changing
> position. For example, an iframe in a scrollable area would get called
> to layout if the area scrolled. This occurred because of the the following
> callstack:
> FrameView calls
> RenderWidget::updateWidgetPosition(...) which calls
> RenderWidget::updateWidgetGeometry(...) which calls
> RenderWidget::setWidgetGeometry(...)
> 
> setWidgetGeometry would return true and force a layout if the frame rect
> changed position. This should only return true if the frame rect actually
> resizes, so this patch updates setWidgetGeometry to return false (not
> forcing a layout) if just the frame's position changes.
> 
> ______
> 
> Just wanted to raise to get input whether this is something worth merging or
> exploring or we have plans to do any other things or this issue is not as
> bad on WebKit as Blink.
> 
> Thanks!

Ignore - I think based on this:

https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderWidget.cpp#158

We already have this optimization. Closing this as "RESOLVED INVALID". Thanks!