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
