213061 2020-06-10 17:18:00 -0700 [iOS] editing/pasteboard/dom-paste tests consistently time out 2020-06-10 19:40:48 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 wenson_hsieh wenson_hsieh bdakin darin megan_gardner thorton webkit-bug-importer oldest_to_newest 1661454 0 wenson_hsieh 2020-06-10 17:18:00 -0700 <rdar://problem/64211161> 1661457 1 401608 wenson_hsieh 2020-06-10 17:39:30 -0700 Created attachment 401608 Patch 1661460 2 401608 darin 2020-06-10 17:42:59 -0700 Comment on attachment 401608 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? 1661461 3 401608 wenson_hsieh 2020-06-10 17:45:59 -0700 Comment on attachment 401608 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) 1661463 4 401608 darin 2020-06-10 17:54:47 -0700 Comment on attachment 401608 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. 1661480 5 ews-feeder 2020-06-10 19:40:47 -0700 Committed r262891: <https://trac.webkit.org/changeset/262891> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401608. 401608 2020-06-10 17:39:30 -0700 2020-06-10 19:40:48 -0700 Patch bug-213061-20200610173929.patch text/plain 5047 wenson_hsieh U3VidmVyc2lvbiBSZXZpc2lvbjogMjYyODM5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGZiMDM0ZDBhOTBjYTgyNWYx NDM3YmVhMmZjM2Y1YzJhNWY1OWJkYzkuLmQyNzVmMmQ5NzE3MDViMDdhNTA1ZjRlYzE0M2M4N2U0 OTBhMTFiMjIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMzkgQEAKKzIwMjAtMDYtMTAgIFdlbnNvbiBI c2llaCAgPHdlbnNvbl9oc2llaEBhcHBsZS5jb20+CisKKyAgICAgICAgW2lPU10gZWRpdGluZy9w YXN0ZWJvYXJkL2RvbS1wYXN0ZSB0ZXN0cyBjb25zaXN0ZW50bHkgdGltZSBvdXQKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIxMzA2MQorICAgICAgICA8 cmRhcjovL3Byb2JsZW0vNjQyMTExNjE+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgT24gcmVjZW50IHZlcnNpb25zIG9mIGlPUywgc3lzdGVtIGNoYW5n ZXMgaGF2ZSBtb3ZlZCB0aGUgdGltaW5nIG9mIHRoZSBjYWxsIHRvCisgICAgICAgIGAtcmVxdWVz dEF1dG9jb3JyZWN0aW9uQ29udGV4dFdpdGhDb21wbGV0aW9uSGFuZGxlcjpgIHdoZW4gZm9jdXNp bmcgYW4gZWRpdGFibGUgZWxlbWVudC4gVGhpcyByZXN1bHRzIGluIGEKKyAgICAgICAgY29uc2lz dGVudGx5IHJlcHJvZHVjaWJsZSBkZWFkbG9jayBiZXR3ZWVuIHRoZSBVSSBwcm9jZXNzIGFuZCB3 ZWIgcHJvY2VzcyBpZiBhIHBhZ2UgdHJpZXMgdG8gcmVxdWVzdAorICAgICAgICBwcm9ncmFtbWF0 aWMgY2xpcGJvYXJkIGFjY2VzcyBkdXJpbmcgdGhlIGNsaWNrIGV2ZW50IChpbXBvcnRhbnRseSwg aW4gdGhlIHNhbWUgcnVubG9vcCBhcyBlbGVtZW50IGZvY3VzKS4KKworICAgICAgICBXaGF0IGVu ZHMgdXAgaGFwcGVuaW5nIGlzIHRoYXQgdGhlIGF1dG9jb3JyZWN0aW9uIGNvbnRleHQgaXMgcmVx dWVzdGVkIGJ5IHRoZSBVSSBwcm9jZXNzIHNob3J0bHkgYWZ0ZXIgdGhlCisgICAgICAgICh1bmJv dW5kZWQpIHN5bmMgSVBDIG1lc3NhZ2UgZm9yIHRoZSBET00gcGFzdGUgYXJyaXZlcywgc28gd2Ug ZW5kIHVwIHdhaXRpbmcgZm9yZXZlciBmb3IgdGhlIHN5bmMtSVBDLWJsb2NrZWQKKyAgICAgICAg d2ViIHByb2Nlc3MgdG8gcmVzcG9uZC4gTm9ybWFsbHkgdGhpcyBqdXN0IHJlc3VsdHMgaW4gYSAx IHNlY29uZCBoYW5nLCBhbmQgdGhlIERPTSBwYXN0ZSBwcm9jZWVkcyBhcyBub3JtYWwuCisgICAg ICAgIEhvd2V2ZXIgdGhlc2UgbGF5b3V0IHRlc3RzIHB1cnBvc2VmdWxseSBvcHQgb3V0IG9mIHN5 bmMgSVBDIHRpbWVvdXRzIChwcmVjaXNlbHkgc28gdGhhdCB3ZSBjYW4gY2F0Y2ggYnVncyBsaWtl CisgICAgICAgIHRoaXMpLiBTZWUgYmVsb3cgZm9yIG1vcmUgZGV0YWlscy4KKworICAgICAgICAq IFVJUHJvY2Vzcy9pb3MvV0tDb250ZW50Vmlld0ludGVyYWN0aW9uLmg6CisKKyAgICAgICAgS2Vl cCB0cmFjayBvZiB0aGUgbW9zdCByZWNlbnQgYXV0b2NvcnJlY3Rpb24gY29udGV4dCB0aGF0IGFy cml2ZWQgaW4gdGhlIFVJIHByb2Nlc3MuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvaW9zL1dLQ29u dGVudFZpZXdJbnRlcmFjdGlvbi5tbToKKyAgICAgICAgKC1bV0tDb250ZW50VmlldyBjbGVhblVw SW50ZXJhY3Rpb25dKToKKworICAgICAgICBDbGVhciBvdXQgdGhlIGF1dG9jb3JyZWN0aW9uIGNv bnRleHQsIGluIGNhc2UgdGhlIHdlYiBwcm9jZXNzIGNyYXNoZXMuCisKKyAgICAgICAgKC1bV0tD b250ZW50VmlldyByZXF1ZXN0QXV0b2NvcnJlY3Rpb25Db250ZXh0V2l0aENvbXBsZXRpb25IYW5k bGVyOl0pOgorCisgICAgICAgIElmIHRoZSBhdXRvY29ycmVjdGlvbiBjb250ZXh0IGlzIHJlcXVl c3RlZCB3aGlsZSBoYW5kbGluZyBhIERPTSBwYXN0ZSwgd2Uga25vdyB0aGF0IHRoZSBsYXRlc3Qg YXV0b2NvcnJlY3Rpb24KKyAgICAgICAgY29udGV4dCBtdXN0IGJlIHVwIHRvIGRhdGUsIHNpbmNl IHRoZSBhdXRvY29ycmVjdGlvbiBjb250ZXh0IGlzIGNvbXB1dGVkIGFuZCBzZW50IGltbWVkaWF0 ZWx5IGJlZm9yZSBhIERPTSBwYXN0ZQorICAgICAgICByZXF1ZXN0LiBVc2UgdGhpcyB0byBpbW1l ZGlhdGVseSBpbnZva2UgdGhlIGNvbXBsZXRpb24gaGFuZGxlciwgd2l0aG91dCBhbnkgc3luYyBJ UEMgYmFjayB0byB0aGUgd2ViIHByb2Nlc3MuCisKKyAgICAgICAgKC1bV0tDb250ZW50VmlldyBf aGFuZGxlQXV0b2NvcnJlY3Rpb25Db250ZXh0Ol0pOgorCiAyMDIwLTA2LTA5ICBGdWppaSBIaXJv bm9yaSAgPEhpcm9ub3JpLkZ1amlpQHNvbnkuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJl dmVydGluZyByMjYyNzkxLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvaW9z L1dLQ29udGVudFZpZXdJbnRlcmFjdGlvbi5oIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvaW9z L1dLQ29udGVudFZpZXdJbnRlcmFjdGlvbi5oCmluZGV4IGQyYmVlNDQxM2VlMDhjNzYwY2IyYTAw NTJhYmFlMGU5NTBiYjQ2ZTQuLjBhMTA2ZDI5NjY2NTI5MGNhYjc4Nzk2M2UxZmRmY2E2ZWY2NWVh ZjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRWaWV3 SW50ZXJhY3Rpb24uaAorKysgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9pb3MvV0tDb250ZW50 Vmlld0ludGVyYWN0aW9uLmgKQEAgLTUwLDYgKzUwLDcgQEAKICNpbXBvcnQgIldLU2hhcmVTaGVl dC5oIgogI2ltcG9ydCAiV0tTeW50aGV0aWNUYXBHZXN0dXJlUmVjb2duaXplci5oIgogI2ltcG9y dCAiV0tUb3VjaEFjdGlvbkdlc3R1cmVSZWNvZ25pemVyLmgiCisjaW1wb3J0ICJXZWJBdXRvY29y cmVjdGlvbkNvbnRleHQuaCIKICNpbXBvcnQgIl9XS0VsZW1lbnRBY3Rpb24uaCIKICNpbXBvcnQg Il9XS0Zvcm1JbnB1dFNlc3Npb24uaCIKICNpbXBvcnQgPFVJS2l0L1VJVmlldy5oPgpAQCAtOTUs NyArOTYsNiBAQCBjbGFzcyBOYXRpdmVXZWJUb3VjaEV2ZW50OwogY2xhc3MgU21hcnRNYWduaWZp Y2F0aW9uQ29udHJvbGxlcjsKIGNsYXNzIFdlYk9wZW5QYW5lbFJlc3VsdExpc3RlbmVyUHJveHk7 CiBjbGFzcyBXZWJQYWdlUHJveHk7Ci1zdHJ1Y3QgV2ViQXV0b2NvcnJlY3Rpb25Db250ZXh0Owog fQogCiBAY2xhc3MgV2ViRXZlbnQ7CkBAIC0zMDksNiArMzA5LDcgQEAgc3RydWN0IFdLQXV0b0Nv cnJlY3Rpb25EYXRhIHsKICAgICB9OwogICAgIFRhcEhpZ2hsaWdodEluZm9ybWF0aW9uIF90YXBI aWdobGlnaHRJbmZvcm1hdGlvbjsKIAorICAgIFdlYktpdDo6V2ViQXV0b2NvcnJlY3Rpb25Db250 ZXh0IF9sYXN0QXV0b2NvcnJlY3Rpb25Db250ZXh0OwogICAgIFdlYktpdDo6V0tBdXRvQ29ycmVj dGlvbkRhdGEgX2F1dG9jb3JyZWN0aW9uRGF0YTsKICAgICBXZWJLaXQ6OkludGVyYWN0aW9uSW5m b3JtYXRpb25BdFBvc2l0aW9uIF9wb3NpdGlvbkluZm9ybWF0aW9uOwogICAgIFdlYktpdDo6Rm9j dXNlZEVsZW1lbnRJbmZvcm1hdGlvbiBfZm9jdXNlZEVsZW1lbnRJbmZvcm1hdGlvbjsKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRWaWV3SW50ZXJhY3Rp b24ubW0gYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9pb3MvV0tDb250ZW50Vmlld0ludGVyYWN0 aW9uLm1tCmluZGV4IGQyYjRmNzM4ZWQ0ZTk5NmU2ODViMzZjZTA3OTcyZWJmMmNjYzkzODEuLmIx YjkwMDgyYjhlYWYwYTdlZjQ2MDcwZTQzYTRmN2Y0MjNlMDViMjQgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRWaWV3SW50ZXJhY3Rpb24ubW0KKysrIGIv U291cmNlL1dlYktpdC9VSVByb2Nlc3MvaW9zL1dLQ29udGVudFZpZXdJbnRlcmFjdGlvbi5tbQpA QCAtOTI3LDYgKzkyNyw4IEBAIC0gKHZvaWQpY2xlYW5VcEludGVyYWN0aW9uCiAgICAgX2FkZGl0 aW9uYWxDb250ZXh0Rm9yU3Ryb25nUGFzc3dvcmRBc3Npc3RhbmNlID0gbmlsOwogICAgIF93YWl0 aW5nRm9yRWRpdERyYWdTbmFwc2hvdCA9IE5POwogCisgICAgX2xhc3RBdXRvY29ycmVjdGlvbkNv bnRleHQgPSB7IH07CisKICNpZiBVU0UoVUlLSVRfS0VZQk9BUkRfQURESVRJT05TKQogICAgIF9j YW5kaWRhdGVWaWV3TmVlZHNVcGRhdGUgPSBOTzsKICAgICBfc2VlbkhhcmR3YXJlS2V5RG93bklu Tm9uRWRpdGFibGVFbGVtZW50ID0gTk87CkBAIC00MjQ2LDYgKzQyNDgsMTEgQEAgLSAodm9pZCly ZXF1ZXN0QXV0b2NvcnJlY3Rpb25Db250ZXh0V2l0aENvbXBsZXRpb25IYW5kbGVyOih2b2lkIChe KShVSVdLQXV0b2NvcnIKICAgICAgICAgcmV0dXJuOwogICAgIH0KIAorICAgIGlmIChfZG9tUGFz dGVSZXF1ZXN0SGFuZGxlcikgeworICAgICAgICBjb21wbGV0aW9uSGFuZGxlcihbV0tBdXRvY29y cmVjdGlvbkNvbnRleHQgYXV0b2NvcnJlY3Rpb25Db250ZXh0V2l0aFdlYkNvbnRleHQ6X2xhc3RB dXRvY29ycmVjdGlvbkNvbnRleHRdKTsKKyAgICAgICAgcmV0dXJuOworICAgIH0KKwogICAgIC8v IEZJWE1FOiBSZW1vdmUgdGhlIHN5bmNocm9ub3VzIGNhbGwgd2hlbiA8cmRhcjovL3Byb2JsZW0v MTYyMDcwMDI+IGlzIGZpeGVkLgogICAgIGNvbnN0IGJvb2wgdXNlU3luY1JlcXVlc3QgPSB0cnVl OwogCkBAIC00MjY1LDYgKzQyNzIsNyBAQCAtICh2b2lkKXJlcXVlc3RBdXRvY29ycmVjdGlvbkNv bnRleHRXaXRoQ29tcGxldGlvbkhhbmRsZXI6KHZvaWQgKF4pKFVJV0tBdXRvY29ycgogCiAtICh2 b2lkKV9oYW5kbGVBdXRvY29ycmVjdGlvbkNvbnRleHQ6KGNvbnN0IFdlYktpdDo6V2ViQXV0b2Nv cnJlY3Rpb25Db250ZXh0Jiljb250ZXh0CiB7CisgICAgX2xhc3RBdXRvY29ycmVjdGlvbkNvbnRl eHQgPSBjb250ZXh0OwogICAgIFtzZWxmIF9pbnZva2VQZW5kaW5nQXV0b2NvcnJlY3Rpb25Db250 ZXh0SGFuZGxlcjpbV0tBdXRvY29ycmVjdGlvbkNvbnRleHQgYXV0b2NvcnJlY3Rpb25Db250ZXh0 V2l0aFdlYkNvbnRleHQ6Y29udGV4dF1dOwogfQogCg==