Tile invalidation and painting can race with one another.
A race when the user types two characters in quick succession:
1. After the first key press, the tile is invalidated.
2. The client request the tile on receiving the invalidate.
3. TileCache doesn't find it, and adds subscription.
A. Sometime before rendering, the second key press is received.
B. This invalidates the very same tile.
C. The client request the same tile.
D. TileCache finds a subscription and ignores the new one.
4. The tile is rendered and sent back.
5. Subscription is found and the tile is forwarded to clients.
6. Subcription is removed as the request is fullfilled.
E. The second tile is rendered and sent back.
F. TileCache finds no subscription and the tile is dropped.
End result: Only the first character appears on the screen.
Versioning fixes the above situation by making sure that in
step 5 the subscription will show to belong to a different
(and newer version) and so the tile will be ignored.
Instead, at F the TileCache will find both subscription
and a matching version and the lastest version will always
be sent back to the client.
Change-Id: I7d7fe1407cda1908d794683c3ce4c2fd18609a2f
Reviewed-on: https://gerrit.libreoffice.org/25341
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
A source file (.cpp) must include its own header first.
This insures that the header is self-contained and
doesn't depend on arbitrary (and accidental) includes
before it to compile.
Furthermore, system headers should go next, followed by
C then C++ headers, then libraries (Poco, etc) and, finally,
project headers come last.
This makes sure that headers and included in the same dependency
order to avoid side-effects. For example, Poco should never rely on
anything from our project in the same way that a C header should
never rely on anything in C++, Poco, or project headers.
Also, includes ought to be sorted where possible, to improve
readability and avoid accidental duplicates (of which there
were a few).
Change-Id: I62cc1343e4a091d69195e37ed659dba20cfcb1ef
Reviewed-on: https://gerrit.libreoffice.org/25262
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Obviously this is dangerous, since the id is not part of the
subscription key (the filename) so different clients could
have different ids on the same part, but in practice I
expect this not to happen. Though that clearly depends on
how clients use the id.
Change-Id: I52a0b043c9b5e5ad1111b692e4216cc9ffec5b2b
Reviewed-on: https://gerrit.libreoffice.org/24680
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
bccu#1433, bccu#1757 related.
Piggyback editlock information to tile messages so that kit can
use that information to allow changing parts only for messages
with editlock.
... and decline tile render request for tile messages without editlock
information
Change-Id: I9cedb870cd977741375665cb258d04c818481a14
WSD's DocumentBroker and Kit's Document now handle
the communication of tiles as well as all aspects
of rendering, caching, unifying requests and
distributing results etc.
Change-Id: Ie22fbaaae26b91185ee348ec7077f30430eec0f6
Reviewed-on: https://gerrit.libreoffice.org/24640
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
All changes are supposed to be persistent. This simplifies the tile
caching code quite a lot.
The TileCache object no longer needs to keep any state whether the
document is being edited or whether it has been modified without
saving etc.
Update the modtime.txt file after saving the document. Otherwise the
tile cache would wrongly be considered invalid next time.
As a sanity check, we put a flag file 'unsaved.txt' into the cache
directory whenever we get a callback that indicates the document has
been modified, and remove it when the document is saved. If the flag
file is present when we take an existing tile cache into use, we can't
trust it.
Even after these changes, we still don't use an existing tile cache as
much (or at all?) as we could, though. The INVALIDATE_TILES EMPTY
callback that LO does early on in a conection causes us to remove all
cached tiles...
Keep track of tiles being rendered in TileCache, and when asked to
render the same tile as is already being rendered, just "subscribe" to
the existing ongoing rendering. When a tile has been rendered and is
being sent out to clients, check if there are "subscriptions" and send
it to them, too.
One problem is that if the client that caused a tile rendering to be
initiated goes away before the rendering has completed, it will never
complete, and the subscribers are left without the tile.
Change-Id: Icca237876a0f466c29eb5bf60ffd4da3d9d68600
Reviewed-on: https://gerrit.libreoffice.org/24228
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Sorry, could not resist. Obviously not very important.
In retrospect, maybe it would have been better to have as policy to
*not* use any 'using Poco::Foo'. Now there is an inconsistent mix of
writing out the complete type and using a 'using'. Plus copy-pasted
long lists of 'usings'. And of course, one should never have 'using'
in an include file. Oh well.
Otherwise we have a problem when cached information is different than
the actual one maintained by LOKitDocument, keeping the whole
server setup in an inconsistent state.
For example, when client reloads the browser session quickly
after setting the current part (to say 3) of an opened part document.
If the reload occurs soon enough before the 'setclientpart'
message originated from the client is acknowledged by the server,
then the next reload would get the cached 'status' entry asking
the client to set 'currentpart' to something else leading to the
generation of another 'setclientpart' message. This thing would
go on, and we would run into a loop, as I have for the last few
hours.
Change-Id: Ia6260dfb772f2e3f023572aa060fd7da92b196c8
Reviewed-on: https://gerrit.libreoffice.org/22272
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Now we have 2 caching directories: 'persistent', and 'editing'.
The Persistent cache always represents the document as it is saved, the
Editing one caches the current edits. The Editing state is copied to
Persistent on save. The Editing state is destroyed on reload.
Remove any intersecting cached tiles. It is the parent process that handles
the tile cache, so it must look for invalidatetiles: messages, too, before
passing them on to the client. To know for which part we should remove tiles,
add an "ephemeral" curpart: message that the child process sends to the parent
process before the invalidatetils: message.
Just call the getStatus() function directly in the child process, which will
always cause a complete status: message to be sent to the client. No
documentsizechanged: messages now sent to the client at all.
When a child process sends a documentsizechanged: message to the parent, to be
forwarded to the client, parse it and update a cached status of the doucment,
if available, and send an updated status: message to the client instead.
Note that clients should not rely on getting only status: messages and never
documentsizechanged: messages, though; the cache might be cleaned at any time
even while the server is running. If there is no cached status of the document
to update and re-use, we have to forward the documentsizechanged: message as
such to the client.
The JS code always passes in 0 for now. The server parses the parameter and
calls LibreOfficeKitDocument::setPart() before calling paintTile().
Probably also the status, key, mouse and selection messages will need a part
number. The intent is after all that the protocol is as stateless as
possible. (So maybe we should also pass the document URL in each message?)
