190375 2018-10-08 15:56:55 -0700 REGRESSION (r236512): [ Mac WK2 ] Layout Test editing/undo/undo-smart-delete-word.html is flaky 2018-10-16 08:02:59 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=58055 InRadar P2 Normal --- 1 tsavell cdumez ap cdumez commit-queue ggaren lforschler realdawei rniwa ryanhaddad webkit-bug-importer oldest_to_newest 1467239 0 tsavell 2018-10-08 15:56:55 -0700 The following layout test is flaky on Mac WK2 editing/undo/undo-smart-delete-word.html Probable cause: This is fallout from https://trac.webkit.org/changeset/236512/webkit. reproduced with command: run-webkit-tests --root testbuild-236512 editing/undo/undo-smart-delete-word.html --iterations 500 -f No failure occurs on r236511 Flakiness Dashboard: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fundo%2Fundo-smart-delete-word.html Diff: --- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/editing/undo/undo-smart-delete-word-expected.txt +++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/editing/undo/undo-smart-delete-word-actual.txt @@ -1,4 +1,8 @@ +CONSOLE MESSAGE: line 28: [object Text] +CONSOLE MESSAGE: line 29: 4 +CONSOLE MESSAGE: line 30: [object Text] +CONSOLE MESSAGE: line 31: 4 Tests: Select a word via double-click. Delete. Then undo the delete. The space that got smart deleted should now be selected. -foo bar baz -PASSED +foo baz +FAILED 1467883 1 webkit-bug-importer 2018-10-10 16:40:43 -0700 <rdar://problem/45177807> 1468212 2 cdumez 2018-10-11 15:30:51 -0700 It seems to be related to the following IPC: m_page->process().send(Messages::WebPage::UnapplyEditCommand(m_commandID), m_page->pageID(), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply); Which sometimes gets processed out of order. 1469372 3 cdumez 2018-10-15 12:16:29 -0700 If I dump editing callback, I see the following diff when it fails: @@ -8,10 +8,11 @@ EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 4 of #text > DIV > BODY > HTML > #document to 4 of #text > DIV > BODY > HTML > #document toDOMRange:range from 4 of #text > DIV > BODY > HTML > #document to 4 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification -EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 4 of #text > DIV > BODY > HTML > #document to 4 of #text > DIV > BODY > HTML > #document toDOMRange:range from 4 of #text > DIV > BODY > HTML > #document to 3 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE -EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification -EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification +CONSOLE MESSAGE: line 30: [object Text] +CONSOLE MESSAGE: line 31: 4 +CONSOLE MESSAGE: line 32: [object Text] +CONSOLE MESSAGE: line 33: 4 Tests: Select a word via double-click. Delete. Then undo the delete. The space that got smart deleted should now be selected. -foo bar baz -PASSED +foo baz +FAILED 1469374 4 cdumez 2018-10-15 12:20:58 -0700 When it fails, WebEditorClient::canRedo() returns false. 1469375 5 cdumez 2018-10-15 12:21:18 -0700 (In reply to Chris Dumez from comment #4) > When it fails, WebEditorClient::canRedo() returns false. I meant WebEditorClient::canUndo(). 1469390 6 352361 cdumez 2018-10-15 12:35:52 -0700 Created attachment 352361 Patch 1469562 7 352361 commit-queue 2018-10-15 17:33:04 -0700 Comment on attachment 352361 Patch Clearing flags on attachment: 352361 Committed r237159: <https://trac.webkit.org/changeset/237159> 1469563 8 commit-queue 2018-10-15 17:33:05 -0700 All reviewed patches have been landed. Closing bug. 1469698 9 ggaren 2018-10-16 08:02:59 -0700 I find “DispatchMessageEvenWhenWaitingForSyncReply” to be a very hard to digest phrase. One reason is that it’s a send option, but it actually governs the behavior of the recipient and not the sender. I’m not sure how to fix that. Maybe a better name would be “CanDispatchDuringSyncWait”. And maybe a better design would be for the sync messages to explicitly list the async message types they depend on. So, for example, the undo message would specify [ RegisterEditCommandForUndo ] as the list of message types to process before it during a sync wait interruption. In the normal case this would do nothing. But in the sync wait interruption case for undo (and only undo), this would scan the queue and process all RegisterEditCommandForUndo messages eagerly. 352361 2018-10-15 12:35:52 -0700 2018-10-15 17:33:04 -0700 Patch bug-190375-20181015123551.patch text/plain 2829 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjM3MTEzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDI3MjA0OWY3NTE3ZDFkODFm YjM4MWVlNTllMmY4YWUzODg2MTYwOTQuLjk2ZDIzZTQ2OWFmNTZkYjZkM2I4NTk2N2JkZDRmZmM2 YzlhZTkyOTYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMzEgQEAKKzIwMTgtMTAtMTUgIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OIChyMjM2NTEyKTog WyBNYWMgV0syIF0gTGF5b3V0IFRlc3QgZWRpdGluZy91bmRvL3VuZG8tc21hcnQtZGVsZXRlLXdv cmQuaHRtbCBpcyBmbGFreQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTkwMzc1CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS80NTE3NzgwNz4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgdGVzdCBpcyBk b2luZyB0aGUgZm9sbG93aW5nOgorICAgICAgICA+IGRvY3VtZW50LmV4ZWNDb21tYW5kKCJEZWxl dGUiKTsgZG9jdW1lbnQuZXhlY0NvbW1hbmQoIlVuZG8iKTsKKworICAgICAgICBCb3RoIG9wZXJh dGlvbiBhcmUgc3luY2hyb25vdXMgYW5kIHJlbHkgb24gc3luY2hyb25vdXMgSVBDIGZyb20gdGhl IFdlYlByb2Nlc3MgdG8gdGhlIFVJUHJvY2Vzcy4KKyAgICAgICAgSG93ZXZlciwgZm9yIHRoZSB1 bmRvIG9wZXJhdGlvbiB0byBzdWNjZWVkLCB0aGUgZmlyc3QgY29tbWFuZCBuZWVkIHRvIGhhdmUg YmVlbiByZWdpc3RlcmVkIHdpdGgKKyAgICAgICAgd2l0aCB0aGUgVUlQcm9jZXNzIHZpYSB0aGUg V2ViUGFnZVByb3h5OjpSZWdpc3RlckVkaXRDb21tYW5kRm9yVW5kbyBJUEMgdG8gdGhlIFVJUHJv Y2Vzcywgd2hpY2gKKyAgICAgICAgaXMgYXN5bmNocm9ub3VzLgorCisgICAgICAgIEFmdGVyIHIy MzY1MTIsIHRoZSBVSVByb2Nlc3Mgbm8gbG9uZ2VyIHByb2Nlc3NlcyBpbmNvbWluZyBzeW5jIC8g YXN5bmMgSVBDIGluIG9yZGVyIGFuZCB0aHVzLAorICAgICAgICBpdCBpcyBwb3NzaWJsZSBmb3Ig dGhlIFdlYlBhZ2VQcm94eTo6RXhlY3V0ZVVuZG9SZWRvIHN5bmNocm9ub3VzIElQQyB0byBnZXQg cHJvY2Vzc2VkIGJ5IHRoZQorICAgICAgICBVSVByb2Nlc3MgKmJlZm9yZSogdGhlIFdlYlBhZ2VQ cm94eTo6UmVnaXN0ZXJFZGl0Q29tbWFuZEZvclVuZG8gYXN5bmNocm9ub3VzIElQQyBmb3IgdGhl CisgICAgICAgIHByZXZpb3VzICJEZWxldGUiIGNvbW1hbmQuIEFzIGEgcmVzdWx0LCBjYW5VbmRv IHdvdWxkIHJldHVybiBmYWxzZSBhbmQgd2Ugd291bGQgZmFpbCB0byB1bmRvLgorCisgICAgICAg IFRvIGFkZHJlc3MgdGhlIGlzc3VlLCB1c2UgU2VuZE9wdGlvbjo6RGlzcGF0Y2hNZXNzYWdlRXZl bldoZW5XYWl0aW5nRm9yU3luY1JlcGx5IGZsYWcgd2hlbiBzZW5kaW5nCisgICAgICAgIHRoZSBX ZWJQYWdlUHJveHk6OlJlZ2lzdGVyRWRpdENvbW1hbmRGb3JVbmRvIElQQywgc28gdGhhdCBpdCBn ZXRzIHByb2Nlc3NlZCBpbiBvcmRlciB3aXRoIHJlZ2FyZHMKKyAgICAgICAgdG8gc3Vycm91bmRp bmcgc3luY2hyb25vdXMgSVBDLgorCisgICAgICAgICogV2ViUHJvY2Vzcy9XZWJDb3JlU3VwcG9y dC9XZWJFZGl0b3JDbGllbnQuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJFZGl0b3JDbGllbnQ6 OnJlZ2lzdGVyVW5kb1N0ZXApOgorCiAyMDE4LTEwLTE1ICBDaHJpcyBEdW1leiAgPGNkdW1lekBh cHBsZS5jb20+CiAKICAgICAgICAgRXhwZXJpbWVudDogdGFyZ2V0PV9ibGFuayBvbiBhbmNob3Jz IHNob3VsZCBpbXBseSByZWw9bm9vcGVuZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvV2Vi UHJvY2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJFZGl0b3JDbGllbnQuY3BwIGIvU291cmNlL1dlYktp dC9XZWJQcm9jZXNzL1dlYkNvcmVTdXBwb3J0L1dlYkVkaXRvckNsaWVudC5jcHAKaW5kZXggOWQw ODNjNDFiZmUxNWFjMzE3N2Y2MTcxYTA3NTljYjIzYTVjNjA1NS4uNTk2OTM1ZDZjOGMzNjM0ODZh YzJlZmQwZmRjNWRiMmRjZTBlOGU1MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9XZWJQcm9j ZXNzL1dlYkNvcmVTdXBwb3J0L1dlYkVkaXRvckNsaWVudC5jcHAKKysrIGIvU291cmNlL1dlYktp dC9XZWJQcm9jZXNzL1dlYkNvcmVTdXBwb3J0L1dlYkVkaXRvckNsaWVudC5jcHAKQEAgLTMwMCw3 ICszMDAsNyBAQCB2b2lkIFdlYkVkaXRvckNsaWVudDo6cmVnaXN0ZXJVbmRvU3RlcChVbmRvU3Rl cCYgc3RlcCkKICAgICBhdXRvIGVkaXRBY3Rpb24gPSBzdGF0aWNfY2FzdDx1aW50MzJfdD4od2Vi U3RlcC0+c3RlcCgpLmVkaXRpbmdBY3Rpb24oKSk7CiAKICAgICBtX3BhZ2UtPmFkZFdlYlVuZG9T dGVwKHdlYlN0ZXAtPnN0ZXBJRCgpLCB3ZWJTdGVwLnB0cigpKTsKLSAgICBtX3BhZ2UtPnNlbmQo TWVzc2FnZXM6OldlYlBhZ2VQcm94eTo6UmVnaXN0ZXJFZGl0Q29tbWFuZEZvclVuZG8od2ViU3Rl cC0+c3RlcElEKCksIGVkaXRBY3Rpb24pKTsKKyAgICBtX3BhZ2UtPnNlbmQoTWVzc2FnZXM6Oldl YlBhZ2VQcm94eTo6UmVnaXN0ZXJFZGl0Q29tbWFuZEZvclVuZG8od2ViU3RlcC0+c3RlcElEKCks IGVkaXRBY3Rpb24pLCBtX3BhZ2UtPnBhZ2VJRCgpLCBJUEM6OlNlbmRPcHRpb246OkRpc3BhdGNo TWVzc2FnZUV2ZW5XaGVuV2FpdGluZ0ZvclN5bmNSZXBseSk7CiB9CiAKIHZvaWQgV2ViRWRpdG9y Q2xpZW50OjpyZWdpc3RlclJlZG9TdGVwKFVuZG9TdGVwJikK