Bug 213061 - [iOS] editing/pasteboard/dom-paste tests consistently time out
Summary: [iOS] editing/pasteboard/dom-paste tests consistently time out
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-10 17:18 PDT by Wenson Hsieh
Modified: 2020-06-10 19:40 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.93 KB, patch)
2020-06-10 17:39 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2020-06-10 17:18:00 PDT
<rdar://problem/64211161>
Comment 1 Wenson Hsieh 2020-06-10 17:39:30 PDT
Created attachment 401608 [details]
Patch
Comment 2 Darin Adler 2020-06-10 17:42:59 PDT
Comment on attachment 401608 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4252
> +        completionHandler([WKAutocorrectionContext autocorrectionContextWithWebContext:_lastAutocorrectionContext]);

Is it ever OK to do this twice? If not, then can we std::exchange to clear out _lastAutocorrectionContext here?
Comment 3 Wenson Hsieh 2020-06-10 17:45:59 PDT
Comment on attachment 401608 [details]
Patch

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

Thanks for the review!

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4252
>> +        completionHandler([WKAutocorrectionContext autocorrectionContextWithWebContext:_lastAutocorrectionContext]);
> 
> Is it ever OK to do this twice? If not, then can we std::exchange to clear out _lastAutocorrectionContext here?

It’s /theoretically/ okay to do this twice (UIKit could potentially call into this twice in a row while handling a DOM paste request), so I would prefer to not std::exchange it with { }.

(That said, I don’t think multiple calls would happen often in practice, if at all)
Comment 4 Darin Adler 2020-06-10 17:54:47 PDT
Comment on attachment 401608 [details]
Patch

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

>>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4252
>>> +        completionHandler([WKAutocorrectionContext autocorrectionContextWithWebContext:_lastAutocorrectionContext]);
>> 
>> Is it ever OK to do this twice? If not, then can we std::exchange to clear out _lastAutocorrectionContext here?
> 
> It’s /theoretically/ okay to do this twice (UIKit could potentially call into this twice in a row while handling a DOM paste request), so I would prefer to not std::exchange it with { }.
> 
> (That said, I don’t think multiple calls would happen often in practice, if at all)

What was worried about was keeping _lastAutocorrectionContext around indefinitely. But I guess what I missed is that the code above is in a method named cleanUpInteraction.
Comment 5 EWS 2020-06-10 19:40:47 PDT
Committed r262891: <https://trac.webkit.org/changeset/262891>

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