[Embedder] Send key data through message channel #29638
Conversation
flutter-dashboard
bot
added
embedder
|
We should document the protocol somewhere so that new embedders know what to implement. |
@Hixie: The public embedder API defines this in the
Ah, I thought you meant
Awesome. Yeah, this approach makes sense. |
|
Awesome. Thank you. I'll make some final changes before sending over for reviewing. |
shell/platform/embedder/embedder.cc
Outdated
| FlutterEngineResult result2 = | ||
| FlutterPlatformMessageReleaseResponseHandle(engine, response_handle); |
There was a problem hiding this comment.
nit: I think we can move this down below the if right? Also, please use a more descriptive name than result2, or, alternatively, just change the return result2 to return FlutterPlatofrmMessageReleaseResponseHandle(engine, response_handle);.
There was a problem hiding this comment.
No. It's exactly what this awkward structure means: Even if the SendPlatformMessage call fails, we still need to release the response handle (basically a manual try...finally). I'll add a comment for it.
There was a problem hiding this comment.
Ahh ok, it's doing some kind of RAII thing here?
At any rate, it should be named better than result2 :)
|
What's the impact on latency here, if any? I'm curious about why we don't experience the same problem(s) with pointer data packets too. |
I don't think it impact latency at all, since the old way is basically implementing a parallel platform messenger.
The keyboard system uses a much stricter regularization check, which asserts for any irregular event sequence, while the pointer data converter is much more tolerant and will fill the gap if possible. |
|
Gold has detected about 69 new digest(s) on patchset 15. |
dkwingsmt
force-pushed
the
key-data-through-message
branch
from
8c70560 to
bba38c0
Compare
fluttergithubbot
force-pushed
the
master
branch
from
aa7ab8d to
29ea28d
Compare
6 participants
This PR changes how embedder API's
SendKeyDatasendsui.KeyDatato the framework.Currently the packet is sent over FFI through a series of
DispatchKeyDatamethods that drill all the way fromWindowtoEmbedderEngine. Not only is it cumbersome, but it also bypasses the existing buffering mechanism, leading to a crash as described in flutter/flutter#89748 (comment).This PR changes the packet to be sent over the existing platform messenger, reusing the entirety of its code path and functionalities while keeping the embedder API unchanged.
This partially fixes flutter/flutter#89748 (comment). Since the buffer size is only 1, if 2 or more events are sent during the restart (which usually means holding multiple keys) the crash will still occur. There's a way to resize the buffer, but I haven't found a way to actually do it yet, and will explore as the next step.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.