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>
Tiles can be rendered in large batches and sent back
as combined payloads, all reduce latency and overhead
significantly.
Initial tests show a reduction in total latency by 2x.
This without sending tiles back to the client in combined
form, which will reduce latency further.
Change-Id: Iee06503f2a6c741fadb2c890266ea514c809c0dc
Reviewed-on: https://gerrit.libreoffice.org/25339
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>
Reading from the socket in the test is not
thread-safe, and was causing all sorts of
problems.
The new code adds a test API and reads the
incoming data through it and not directly
from the socket. In addition, the read is
synchronized.
Change-Id: Id13821a40a59e32fd8a14f733a47306aee42ada8
Reviewed-on: https://gerrit.libreoffice.org/25244
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
To avoid persisting the same document many times over
we check the jailed file timestamp. This logic
doesn't need to be more complex than simply invalidating
the last timestamp upon issuing .uno:Save and setting
it to the file's actual timestamp upon persisting.
Change-Id: I4afdf8da93fed438d4cbcd6de8dc14d52172ac9c
Reviewed-on: https://gerrit.libreoffice.org/24811
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Relying on the filesystem to tell us when the document
was last modified (to decide whether to upload to storage or not,)
proved unreliable.
Now we always upload to storage if there is only one client.
This both minimizes the risk and also avoids the file timestamp
check as a workaround to the problem of re-uploading documents
as many time as there were clients. Since with one client we
can only upload no more than once per save, which is reasonable.
Furthermore, when a client disconnects we auto-save automatically
as a matter of precaution. However, when there are other clients
still connected, we don't wait for the save to complete, rather
we let that job to the very last one.
Change-Id: I94a2e4bddaed30a6c9c0e69f8006667d33c5b8ee
Reviewed-on: https://gerrit.libreoffice.org/24767
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
The tile and tilecombine messages apparently have optional
appendages at their rear ends. Not one, but two (at least).
While the first (timestamp) seems to be truely optional
(in the sense that leaving it out doesn't break anything,)
the same can't be said of the second (id).
For Impress slides this id is used to identify the slide
to which the tile belongs. Or rather the slide being
rendered, as it seems meaningful only for the slide
thumbnails.
Previously the complete arguments of tile were copied
verbatim from the input to the output (i.e. back to the
client) and so any extra payload was also echoed back.
But when id is missing (when expected) loleaflet not
only fails to show these tiles (understandably,) but
it also fails to show the scrollbar for said slide
thumbnails altogether!
With the new logic to move the tile communication to
the child socket instead of the clients, the arguments
are parsed and then serialized back in the response.
So all fields must be explicitly known in advance.
This change is necessary because tilecombine is broken
to tile commands and so both share common code. This
means that echoing back the request verbatim will
break loleaflet since tilecombine arguments (which
is a list) is not a valid response (which has the
format of tile). So the internal representation
has to be something neutral/common.
The fix is to parse the timestamp and id only when
provided and to echo back the id only in that case.
Change-Id: Ic97cf0de4083d412a4b3543e8b9a8713ac27a27c
Reviewed-on: https://gerrit.libreoffice.org/24669
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
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>
...child from DocumentBroker""
Restore the communication with child from DocumentBroker.
This reverts commit 20ab6e8ae7.
Change-Id: I248bededff7074d8fb482b2cdd172048f80c02b2
Reviewed-on: https://gerrit.libreoffice.org/24639
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Autosave should only save when the user has been idle
for a certain time, or the periodic autosave time elapses.
The document is considered for autosave only when it's modified.
Change-Id: Ia239173ff6636e52c1a2b7e1f6bf9bd6860175ed
Reviewed-on: https://gerrit.libreoffice.org/24602
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The WebSocket that each child created with WSD is not used
except to request the child to load the document a client
requests. Beyond this point, it was not utilized for anything.
In fact, there are no handlers in WSD for messages coming
from the child; it is a one-way communication.
That is until now. With the move to unify communication
between WSD and each child, DocumentBroker can now
receive and handle messages from its ChildProcess.
Change-Id: Ie7f030a92db8303cd7087fff2325f136a49bc7fc
Reviewed-on: https://gerrit.libreoffice.org/24581
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Due to filesystem timestamp precision and other factors
we assume a timestamp match within 10 seconds to mean
the document has been recently saved and store it.
A document has to have an older than 10 seconds
modified timestamp compared to our last timestamp
to be deemed unchanged in the interim and skipped
from storing again.
Change-Id: I39b4bf64b221ba30dc7b818a330e779a2d0ecbd4
Reviewed-on: https://gerrit.libreoffice.org/24472
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
DocumentBroker is a central document management object.
Using it to communicate with clients leaves it open
to the whims of slow connections. When its lock
is help for a long time, all clients stall, giving
users a very poor experience.
The culprit in this case was takeEditLock, which
sent to all clients the new edit lock state.
To avoid this, DocumentBroker now sends this
state to the children (via a loopback socket,)
which process messages in a separate process and
each on its own queue thread. The children then
in turn echo this edit lock state back to the
clients. This communication back is done on the
prisoner socket thread, which doesn't lock or
stall any shared objects or threads.
Change-Id: I475f6b3ecac9ae2a689bd30f43d416871aa0e384
Reviewed-on: https://gerrit.libreoffice.org/24420
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This makes for amore compact API and avoids
a race between issuing the save and waiting for it.
Also added force flag and autoSave now checks the
modified state of the document. If a document is
not modified, nor save forced, autoSave checks
the last activity on the document and only
if there is any since last save does it issue
a save command.
Change-Id: I962e36df18d7edf5f658992e97b5def5f6247dc3
Reviewed-on: https://gerrit.libreoffice.org/24382
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When multiple users have a document open,
save notficiations are broadcast to all.
Each session then tries to store the document
to the Storage when only the first should suffice.
A new file modified-time member tracks the file's
timestamp and only persists when it changes,
thereby avoid excessive stores.
Change-Id: I138f1aa812963a2120a1fcac763dfacccc542c1a
Reviewed-on: https://gerrit.libreoffice.org/24381
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...
This also avoids the feedback loop that results from the kit
thinking the previously inactive client is now active and
sending commands (.uno:Save).
Change-Id: I47074b35a922da15592d550032d494ba1efab83e
Reviewed-on: https://gerrit.libreoffice.org/24287
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
