cool#11064 kit: check for callbacks & wsd socket in the anyInput callback

[vmiklos]

Open the 300 pages bugdoc, go to the end of page 1 (before a page
break), press enter, a page 2 is inserted and we get new tiles after a
delay:
15:08:33.803 global.js:566 1738073313803 OUTGOING: key type=up char=0 key=1280
...
15:08:34.249 global.js:566 1738073314249 INCOMING: delta: nviewid=1000 part=0 ...
-> 446 ms.

The reason this happens is because our anyInput callback tells core to
finish the idle Writer layout first and only then render the tiles for
the visible area, which is a problem similar to what commit
930cd61 (Related: cool#9735 kit: use
the new registerAnyInputCallback() LOK API, 2024-08-26) fixed.

Fix this problem by improving the anyInput callbacks to take 2 more
"inputs" into account:

1. LOK_CALLBACK_INVALIDATE_TILES went to KitQueue::_callbacks and this
   was only processed in the next main loop iteration, so checks for
   this in the anyInput callback, which partly reverts commit
   2823933 (anyInput - should only depend
   on incoming messages from coolwsd., 2024-12-10).
2. Now that the invalidation is processed, it's sent to the wsd process
   and we get a tile request on the wsd -> kit socket, in the kit
   process. Fix this problem by doing a poll() on that socket as the
   already existing FIXME comment suggested.

With this, we paint tiles for the visible area before calculating pages
3..300 in Writer:
10:15:47.441 global.js:566 1738314947441 OUTGOING: key type=input char=13 key=1280
...
10:15:47.476 global.js:566 1738314947476 INCOMING: delta: nviewid=1000 part=0 ...
-> 24ms

Signed-off-by: Miklos Vajna [email protected]
Change-Id: I668d2ebf341b628841e0a637fee9e476eeddd179

[vmiklos]

@caolanm could you please review this? Thanks.

Please don't merge it, though; I believe this belongs to red, re-arranging events is always tricky.

CC @mmeeks

[caolanm]

And we definitely just want to look at the first one, not all of then (or all of minus trailing wakeup) ?

[vmiklos]

This is for the kit case where (I think) it's guaranteed that we have exactly one incoming socket (from the wsd), but let me assert that, so it's more explicit.

In theory a "just poll" mode with multiple sockets would be interesting, but I don't see a practical need for that yet.

[mmeeks]

Interesting - I'll need to read this. Typically such pieces of code parse & prioritize the incoming messages and stick them into a queue - rather than acting on them immediately in this 'AnyInput' type use-case.

We do quite a bit of queue-ing of incoming things anyhow becuase we need to disable incoming events during eg. loading / progress-bar rendering callbacks; so I would suggest we re-use that and process these things not just look to see if there is data on the socket; so slurp, queue-up, ideally de-duplicate and then use the state of the incoming queue (?) - but - (rudely) not read anything byt Caolan's comment yet ;-) will get back to this.

[vmiklos]

This is for the kit case where (I think) it's guaranteed that we have exactly one incoming socket (from the wsd)

I was wrong, Document::forkToSave() calls insertNewSocket() for the child socket, so we can have multiple sockets for the background save, but it seems the wsd socket is always the same in the array, seeing that https://github.com/collaboraonline/online/blob/c26e006e16a5110dccbb8ca63d3a5f32dbe1f834/net/Socket.cpp#L559 inserts at the end, I think the _pollFds[0].revents reading is fine at the end.

Typically such pieces of code parse & prioritize the incoming messages and stick them into a queue - rather than acting on them immediately

I think I had that normal poll() before 7ca2677 and I removed it because you were concerned about we may do too much processing there and #9974 (comment) suggests we may have a re-entrancy hazard if we do "poll + queue" instead of just "poll".

Am I correct that then this "just poll" way is correct or did I miss something? Thanks.