Instead of manually matching a sent message to a port message, allow adding a serial which is then used to relate a reply back so you just get a neat Promise-based API.
For compatibility, the existing method cannot be changed, as with new extension and older host I will never get a reply, and I cannot tell whether the caller actually asked for the promise or not and if they didn't we would leak the promise resolvers as we never got a reply.
Details
Details
- Reviewers
davidedmundson fvogt - Group Reviewers
Plasma - Commits
- R856:32a4fce2aa74: Allow sending a port message and receive a reply
Placed a sendReply call in a handleData
sendPortMessageWithReply("settings", "getSubsystemStatus").then((message) => {
console.log("got the status", message);
});Diff Detail
Diff Detail
- Repository
- R856 Plasma Browser Integration
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
- Let plugin access the serial so it can store it and send a reply asynchronously later
Comment Actions
IMHO the member variable is really ugly. Messages with and without serial number have to be handled differently anyway, so why not introuce a new handleMessage(event, json, serial) method?
| extension/extension-utils.js | ||
|---|---|---|
| 65 ↗ | (On Diff #63555) | Add a manual wrap before INT32_MAX, just to be safe |
| extension/extension.js | ||
| 109 ↗ | (On Diff #63555) | You could make 0 a valid serial, currently -1 and 0 are both used as flag values but only one is really needed |
| host/abstractbrowserplugin.cpp | ||
| 58 ↗ | (On Diff #63555) | IMO printing a warning and maybe not doing anything is more fitting here than an assert which only works in debug builds and is then fatal. |
Comment Actions
- address review comments
there's now a handleData overload which takes a serial and returns a QJsonObject. You can just return a QJsonObject to reply immediately or store the serial and call sendReply explicitly later
| extension/content-script.js | ||
|---|---|---|
| 693 ↗ | (On Diff #63624) | Don't mind this screwup |
| host/abstractbrowserplugin.h | ||
|---|---|---|
| 56 ↗ | (On Diff #63555) | Why does it say "got hidden" when those two clearly have a different signature? |
| host/abstractbrowserplugin.h | ||
|---|---|---|
| 56 ↗ | (On Diff #63555) | Because only the overridden overload is visible in the child classes. You can fix this by adding using AbstractBrowserPlugin::handleData; to the plugin classes. |
Comment Actions
Didn't add it to settings as I have some follow-up patches that will actually use the other overload, too