Bug 213867 – Crash in +[UIViewController _viewControllerForFullScreenPresentationFromView:] when WKContentView is deallocated

• Component: New Bugs
• Status: RESOLVED FIXED

[Austin]

Crash in +[UIViewController _viewControllerForFullScreenPresentationFromView:] when WKContentView is deallocated

[Austin]

Created attachment 403326 [details]
Patch

[Darin Adler]

Comment on attachment 403326 [details]
Patch

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

> Source/WebKit/ChangeLog:8
> +        No new tests (OOPS!).

Can’t land a patch with this line. Need to delete it.

> Source/WebKit/ChangeLog:11
> +        (-[WKFileUploadPanel dismiss]):

Should have a comment here. Even a simple one like "Add a null check." would do.

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:288
> -    [[UIViewController _viewControllerForFullScreenPresentationFromView:_view.getAutoreleased()] dismissViewControllerAnimated:NO completion:nil];
> +    UIView *autoreleasedView = _view.getAutoreleased();
> +    if (autoreleasedView)
> +        [[UIViewController _viewControllerForFullScreenPresentationFromView:autoreleasedView] dismissViewControllerAnimated:NO completion:nil];

It’s wasteful and unnecessary to use getAutoreleased here; you didn’t add it, but it was wrong. Should just be get(). Autoreleased is only needed when the value is returned from an Objective-C function, not when it’s passed into one. Also there is no reason for the local variable. Should just be:

    if (_view)
        [[UIViewController _viewControllerForFullScreenPresentationFromView:_view.get()] dismissViewControllerAnimated:NO completion:nil];

[Austin]

Created attachment 403374 [details]
Patch

[Darin Adler]

Comment on attachment 403374 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:287
> +    RetainPtr<WKContentView> view = _view.get();
> +    if (view)

OK as is. But could write this:

    if (RetainPtr<WKContentView> view = _view.get())

or this:

    if (auto view = retainPtr(_view.get()))

[Austin]

Created attachment 403394 [details]
Patch

[EWS]

[e-mail] does not have committer permissions according to https://svn.webkit.org/repository/webkit/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

Rejecting attachment 403394 [details] from commit queue.

[EWS]

Committed r263869: <https://trac.webkit.org/changeset/263869>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403394 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65053840>