Bug 210230

Created attachment 395890 [details]
Patch

<rdar://problem/60385504>

Comment on attachment 395890 [details]
Patch

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

Any way to make a test for this?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6669
> +- (void)preserveFocus
> +{
> +    [_webView _incrementFocusPreservationCount];
> +}
> +
> +- (void)releaseFocus
> +{
> +    [_webView _decrementFocusPreservationCount];
> +}

Is there any risk that a WKContentView could be deallocated before releaseFocus is called, or have _webView set to null or any new value before this is called?

Often, designs like this one can be dangerous if a stale count can be left behind. There are techniques to make them more resilient, like using objects and weak pointers, that are most costly but more foolproof.

> Source/WebKit/UIProcess/ios/forms/WKFormInputControl.mm:341
> +        if (_view.focusedElementInformation.elementType == InputType::Time || _view.focusedElementInformation.elementType == InputType::DateTimeLocal)
> +            [_view releaseFocus];

Maybe we could use a boolean _preservingFocus rather than repeating this check here. That way there’s no risk of the check in controlBeginEditing being out of sync with this one.

Created attachment 396022 [details]
Patch for landing

Comment on attachment 396022 [details]
Patch for landing

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

> Source/WebKit/UIProcess/ios/forms/WKFormInputControl.mm:393
> +            _preservingFocus = YES;
> +            [_view preserveFocus];

Could harden this slightly by wrapping this in:

    if (!_preservingFocus) {
    }

Committed r259840: <https://trac.webkit.org/changeset/259840>

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