Bug 179797 - REGRESSION(r224179): layer flush now requires sync IPC to compute undo/redo availability in EditorState
Summary: REGRESSION(r224179): layer flush now requires sync IPC to compute undo/redo a...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
Depends on:
Reported: 2017-11-16 14:29 PST by Michael Catanzaro
Modified: 2017-11-16 16:48 PST (History)
4 users (show)

See Also:

Patch (10.73 KB, patch)
2017-11-16 15:18 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2017-11-16 14:29:26 PST
r224179 introduced a performance regression, originally reported at https://bugs.webkit.org/show_bug.cgi?id=168219#c55. This code in WebPage::editorState:

        postLayoutData.canCut = editor.canCut();
        postLayoutData.canCopy = editor.canCopy();
        postLayoutData.canPaste = editor.canPaste();
        postLayoutData.canUndo = editor.canUndo();
        postLayoutData.canRedo = editor.canRedo();

is part of a performance-critical path. (The editor state is computed and sent to the UI process during the layer flush.) But Editor::canUndo and Editor::canRedo are both implemented with sync IPC calls to WebPageProxy in the UI process. WebPageProxy passes them along to PageClientImpl to compute the availability of the commands.

As Simon noted in bug #168219, that's all pointless because this code only exists for the purpose of getting editing command availability to the UI process. In the case of undo and redo, it's not needed at all. I did not realize that when writing the code. So canUndo and canRedo should be removed from EditorState. I don't think any changes are needed to the GTK/WPE WebKitEditorState API. The API can be implemented using WebPageProxy::canUndoRedo instead of EditorState. I think that should be sufficient to avoid the perf regression.
Comment 1 Michael Catanzaro 2017-11-16 15:18:31 PST
Created attachment 327112 [details]
Comment 2 Michael Catanzaro 2017-11-16 15:20:17 PST
This needs owner approval because I added two new methods to WebPageProxy, canUndo and canRedo. That seems better than making canUndoRedo public, since that interface is a bit messy and really intended for message receipt from the web process.
Comment 3 WebKit Commit Bot 2017-11-16 16:48:24 PST
Comment on attachment 327112 [details]

Clearing flags on attachment: 327112

Committed r224945: <https://trac.webkit.org/changeset/224945>
Comment 4 WebKit Commit Bot 2017-11-16 16:48:26 PST
All reviewed patches have been landed.  Closing bug.