Bug 134807 – Add a mechanism to notify the UIProcess when an editing command is done executing

• Component: WebKit2
• Status: RESOLVED FIXED

[Enrica Casucci]

Some editing commands have an effect on some parts of the system that run inside the UIProcess. A good example are the cursor movement commands that require an update of the autocorrection/autosuggestion machinery.
We need a way to reliably know when the command has been executed in the WebProcess.

[Enrica Casucci]

Created attachment 234714 [details]
Patch

[Tim Horton]

mitz makes a good point! WebKit2 IPC has a nice callback mechanism you can use, and it makes sure that callbacks get invalidated e.g. when the process dies, stuff like that! You should probably be using it.

[Enrica Casucci]

(In reply to comment #2)
> mitz makes a good point! WebKit2 IPC has a nice callback mechanism you can use, and it makes sure that callbacks get invalidated e.g. when the process dies, stuff like that! You should probably be using it.

I though about it and it seems heavier than just passing a parameter, but I agree it has the advantage of having the cleanup ready.
Thanks for the feedback, I'll do that.

[Enrica Casucci]

Created attachment 234727 [details]
Patch2

[WebKit Commit Bot]

Attachment 234727 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:380:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:362:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 234727 [details]
Patch2

View in context: https://bugs.webkit.org/attachment.cgi?id=234727&action=review

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2231
> +    _page->executeEditCommand(commandName, [self](const String& string, CallbackBase::Error error) {

I think VoidCallback is sufficient, isn't it? You don't need StringCallback.

[Enrica Casucci]

(In reply to comment #6)
> (From update of attachment 234727 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234727&action=review
> 
> > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2231
> > +    _page->executeEditCommand(commandName, [self](const String& string, CallbackBase::Error error) {
> 
> I think VoidCallback is sufficient, isn't it? You don't need StringCallback.

Probably yes. I don't need to check the command name. It ok with you with that change?

[Tim Horton]

(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 234727 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=234727&action=review
> > 
> > > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2231
> > > +    _page->executeEditCommand(commandName, [self](const String& string, CallbackBase::Error error) {
> > 
> > I think VoidCallback is sufficient, isn't it? You don't need StringCallback.
> 
> Probably yes. I don't need to check the command name. It ok with you with that change?

You ought never need to, because you already know which command it was by the state around the callback.

r=me with that change.

[Enrica Casucci]

Created attachment 234730 [details]
Patch3

[WebKit Commit Bot]

Attachment 234730 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:380:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:362:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Enrica Casucci]

Committed revision 170981.