Bug 223446

Summary: Handle empty bound size during animated resize
Product: WebKit Reporter: Sihui Liu <sihui_liu>
Component: WebKit2Assignee: Sihui Liu <sihui_liu>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, kkinnunen, sihui_liu, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Sihui Liu 2021-03-18 09:26:25 PDT
...
Comment 1 Sihui Liu 2021-03-18 10:01:03 PDT
Created attachment 423612 [details]
Patch
Comment 2 Tim Horton 2021-03-18 13:44:15 PDT
Comment on attachment 423612 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h:254
> +    CGRect _validOldBounds;

Probably worth giving these names that make it clear they're just about (rare, SPI) animated resize; these names seem... more generic.

Maybe stick them up with _animatedResizeOriginalContentWidth?

> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:-2910
> -    ASSERT_WITH_MESSAGE(!(_viewLayoutSizeOverride && newViewLayoutSize.isEmpty()), "Clients controlling the layout size should maintain a valid layout size to minimize layouts.");

Is there a reason we're not just fixing the clients? (and maybe making this louder?). Since it's SPI, it should be achievable, and it is wasted work otherwise.
Comment 3 Sihui Liu 2021-03-18 14:31:49 PDT
Created attachment 423658 [details]
Patch
Comment 4 Sihui Liu 2021-03-18 16:56:08 PDT
Created attachment 423671 [details]
Patch for landing
Comment 5 EWS 2021-03-18 17:25:21 PDT
Committed r274692: <https://commits.webkit.org/r274692>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423671 [details].
Comment 6 Radar WebKit Bug Importer 2021-03-18 17:26:17 PDT
<rdar://problem/75598829>
Comment 7 Geoffrey Garen 2021-03-19 10:24:58 PDT
> Is there a reason we're not just fixing the clients? (and maybe making this
> louder?). Since it's SPI, it should be achievable, and it is wasted work
> otherwise.

I think the answer -- based on ChangeLog and conversation with Sihui -- is that auto layout just sometimes does this (very temporarily), outside of direct client control.

Maybe this means we should file a Radar for auto layout.
Comment 8 Tim Horton 2021-03-19 15:34:49 PDT
(In reply to Geoffrey Garen from comment #7)
> > Is there a reason we're not just fixing the clients? (and maybe making this
> > louder?). Since it's SPI, it should be achievable, and it is wasted work
> > otherwise.
> 
> I think the answer -- based on ChangeLog and conversation with Sihui -- is
> that auto layout just sometimes does this (very temporarily), outside of
> direct client control.
> 
> Maybe this means we should file a Radar for auto layout.

Yeah, she explained offline (and updated the changelog)! I do think we should make sure there is a radar, since it seems wasteful.