-_beginAnimatedResizeWithUpdates: can leave view in bad state if called during an existing animation

[Jer Noble]

-_beginAnimatedResizeWithUpdates: can leave view in bad state if called during an existing animation

[Jer Noble]

Created attachment 345194 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/42304518>

[Tim Horton]

Comment on attachment 345194 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2906
> +    [_resizeAnimationView removeFromSuperview];

resizeAnimationView has the WKContentView as its subview so you just tossed it.

Look at the existing code that does this to see how to clean up correctly

[Jer Noble]

Created attachment 345204 [details]
Patch

[Tim Horton]

Comment on attachment 345204 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2925
> -        _dynamicViewportUpdateMode = WebKit::DynamicViewportUpdateMode::NotResizing;
> +        [self _cancelAnimatedResize];

IIIIII don't know about this one. Does your code do anything even remotely acceptable if _resizeAnimationView is nil?

[Jer Noble]

Comment on attachment 345204 [details]
Patch

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

>> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2925
>> +        [self _cancelAnimatedResize];
> 
> IIIIII don't know about this one. Does your code do anything even remotely acceptable if _resizeAnimationView is nil?

Hmm, -indexOfObject: will fail, and we may move _scrollView to the end. But yeah, that should probably be protected by: "if (!_customContentView && _resizeAnimationView)".

[Tim Horton]

Other than that, it seems OK, but I am afraid. Also, this should be trivially API testable.

[Tim Horton]

There's already a file full of them.

[Jer Noble]

Created attachment 345212 [details]
Patch

[EWS Watchlist]

Comment on attachment 345212 [details]
Patch

Attachment 345212 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8572226

New failing tests:
http/tests/preload/onload_event.html

[EWS Watchlist]

Created attachment 345233 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[WebKit Commit Bot]

Comment on attachment 345212 [details]
Patch

Clearing flags on attachment: 345212

Committed r233927: <https://trac.webkit.org/changeset/233927>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.