Summary: | REGRESSION (r236512): [ Mac WK2 ] Layout Test editing/undo/undo-smart-delete-word.html is flaky | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Truitt Savell <tsavell> | ||||
Component: | Tools / Tests | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, cdumez, commit-queue, ggaren, lforschler, realdawei, rniwa, ryanhaddad, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=58055 | ||||||
Attachments: |
|
Description
Truitt Savell
2018-10-08 15:56:55 PDT
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. |