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
<rdar://problem/45177807>
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.
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
When it fails, WebEditorClient::canRedo() returns false.
(In reply to Chris Dumez from comment #4) > When it fails, WebEditorClient::canRedo() returns false. I meant WebEditorClient::canUndo().
Created attachment 352361 [details] Patch
Comment on attachment 352361 [details] Patch Clearing flags on attachment: 352361 Committed r237159: <https://trac.webkit.org/changeset/237159>
All reviewed patches have been landed. Closing bug.
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.