D23099: Allow sending a port message and receive a reply

Fabian Vogt noreply at phabricator.kde.org
Mon Aug 12 19:23:12 BST 2019


fvogt added a comment.


  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?

INLINE COMMENTS

> extension-utils.js:65
> +        message.event = event;
> +        message.serial = ++currentMessageSerial;
> +        pendingMessageReplyResolvers[message.serial] = resolve;

Add a manual wrap before INT32_MAX, just to be safe

> extension.js:109
>  
> -        if (!subsystem || !action) {
> +        if (!replyToSerial && (!subsystem || !action)) {
>              return;

You could make 0 a valid serial, currently -1 and 0 are both used as flag values but only one is really needed

> abstractbrowserplugin.cpp:58
> +    }
> +    Q_ASSERT(requestSerial > 0);
> +

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.

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D23099

To: broulik, #plasma, davidedmundson, fvogt
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190812/978e81f5/attachment.html>


More information about the Plasma-devel mailing list