Bug 223446 - Handle empty bound size during animated resize
Summary: Handle empty bound size during animated resize
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-18 09:26 PDT by Sihui Liu
Modified: 2021-03-19 15:34 PDT (History)
6 users (show)

See Also:


Attachments
Patch (7.05 KB, patch)
2021-03-18 10:01 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (7.67 KB, patch)
2021-03-18 14:31 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (7.77 KB, patch)
2021-03-18 16:56 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.