This mends several problems from commit
5710c86323.
Change-Id: I1b29f29ca81679608a2692488fa1ef22b2e62dfd
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92032
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
With the change that is reverted here, the editing session on Android
returns without proper cleanup which leads to occassional hangs when
the user tries to open a new editing session quickly.
Also, in the iOS app, with the change that is reverted, when closing
the document we never get the LOK_CALLBACK_UNO_COMMAND_RESULT for the
.uno:Save and thus we never get to save it properly from the system's
point of view.
This reverts commit a73590d81f.
Change-Id: Ia77fe2fd9b59d30c343ca1e10f69d5a434bc3628
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91915
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91965
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Now, finally, I think it works as it should, knock on wood. After an
auto-save (or explicit save by tapping the button), the edited
document does show up with its auto-saved contents if you open it from
iCloud Drive or Nextcloud while it is still open in the iOS app.
What I was missing was that the document hadn't actually been saved by
the core (to the temporary copy) until we get the
LOK_CALLBACK_UNO_COMMAND_RESULT for the .uno:Save. We must call the
-[UIDocumentsaveToURL:forSaveOperation:completionHandler:] only upon
receiving that callback.
Change-Id: I10486f3bc587c871ee2644a0c097493f33baf420
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91880
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
It is enough to call the -[UIDocument
saveToURL:forSaveOperation:completionHandler:] only in
DocumentBroker::sendUnoSave(). And on the other hand, in
-[DocumentViewController bye] we can't want for the
LOOLWSD::lokit_main_mutex as the main queue is needed for parts of
what the saveToURL does.
Also, use a separate copy of the document as the file that is actually
edited by LO core. This matches what the Android app does. I think it
is useful to do this in order to avoid some hangs that I noticed. They
probably were caused by both LO core and the system frameworks
occasionally accessing the same document file at the same time.
Change-Id: Idb65be23a7cb6ad1288fbbd23c7471e0fb8d52f4
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91851
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
It is not enough to just have the core code rewrite the document file.
We need to do the same dance as in -[DocumentViewController bye].
I wonder if it would be cleaner to copy the document file to some
temporary location when opening it, and use only that copy in Online
and core, and then after core has saved it (either for an auto-save,
an explicit save, or when closing the document) do the -[UIDocument
saveToURL:forSaveOperation:completionHandler] thing?
Change-Id: I78fec08a90424780dc6e794347a0acf2c64e1dab
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91741
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
So that it is easy to find it in the log.
Change-Id: I1b367db9e97d7215c46e824998e99048531acb07
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91413
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Add checking up the stack to detect when we have clean queues
and buffers so we can exit fast.
Change-Id: I82c3843f816bbe869094c21f070774e6d034ac65
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90358
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
It took both an std::string and a length. Take a char* and a length
instead.
Change-Id: Id37dfa67fe1baae09b69819680848a0a8a1d80ed
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90552
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Essentially we want to be able to separate low-level socket code
for eg. TCP vs. UDS, from Protocol handling: eg. WebSocketHandler
and client sessions themselves which handle and send messages
which now implement the simple MessageHandlerInterface.
Some helpful renaming too:
s/SocketHandlerInterface/ProtocolHandlerInterface/
Change-Id: I58092b5e0b5792fda47498fb2c875851eada461d
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90138
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Allows comparing tokens with C strings without a heap allocation. Do the
same when comparing two tokens from two different StringVectors.
And use it at all places where operator ==() has an argument, which is a
StringVector::operator []() result.
Change-Id: Id36eff96767ab99b235ecbd12fb14446a3efa869
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90201
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
WebSocketHandler handles this complexity for us now, and for the
forseeable future. Simplify to prepare for larger re-factor.
Change-Id: I73b919885adc358cb6502a13492cdac85c34459c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90059
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
This is meant to reduce lots of small allocations and instead have
pointers into the single string for the various tokens instead.
This has a few requirements, though:
1) It's no longer OK to modify the tokens, changing their length would
invalidate the start/length of other tokens. Rework
DocumentBroker::load() to avoid such mutation.
2) The iterators no longer expose zero-terminated strings, so
Poco::cat() doesn't work anymore: add an own cat() instead and use that
in e.g. ChildSession. The own cat() has the benefit that it won't read
past the end of the array if the begin index is out of bounds to add
more safety.
(This nicely works towards killing Poco usage in general.)
3) If zero-terminated strings for all individual tokens is needed, a
copy has to be made, as done in spawnProcess().
(For all of these requirements, the build fails if there are problems.)
Change-Id: Iea40e4400e630b2d669f5c72aea85cb40edf9a2c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89711
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
The bulk of this commit just changes std::vector<std::string> to
StringVector when we deal with tokens from a websocket message.
The less boring part of it is the new StringVector class, which is a
wrapper around std::vector<std::string>, and provides the same API,
except that operator[] returns a string, not a string&, and this allows
returning an empty string in case that prevents reading past the end of
the underlying array.
This means in case client code forgets to check size() before invoking
operator[], we don't crash. (See the ~3 previous commits which fixed
such crashes.)
Later the ctor could be changed to take a single underlying string to
avoid lots of tiny allocations, that's not yet done in this commit.
Change-Id: I8a6082143a8ac0b65824f574b32104d7889c184f
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89687
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Termination flag is a very harsh way of exiting.
It works in most cases, but not when we have a
modified document. What happens is the following:
Unit-test flags for termination.
During session cleanup we have to save the modified doc.
Because save is in progress we don't 'disconnect' the view.
This leaves the view in loaded state until saving is done.
But because of the termination flag we don't wait for saving.
DocBroker sends 'exit' to child to forcefully exit.
This causes at least one assertion due to active LOKWindows (Sidebar).
Instead of the above, we flag for graceful shutdown from unittests,
and after we wait to cleanup all DocBrokers, we flag for termination.
This way, we get clean shutdown and all assertions/validations
pass, while we guarantee never to deadlock the unittests,
in case we end up waiting forever for shutdown to complete.
Change-Id: I7fc34137ea373e329795b1ed0090261c085e955a
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89308
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Cookies may be passed from the client to the storage,
in which case each user may have its own unique set
of cookies. These cookies are now preserved in the
ClientSession, which is per connection, and are then
passed to the storage to use when communicating with
the WOPI-like backend.
(cherry picked from commit 6022faf3cc9b622b490c3f8ca91efbff8e542414)
Change-Id: Ic2e13fa541a5ee01b7383939bbbf7d46ea75684b
Use a fully reliable uniqueness check, rather than a hash, and get
simpler ids as a bonus. Fetch view data from the session itself
rather than passing it in too.
Change-Id: Ibcd625156b5a98eb280e35d6537b5c8c026d0197
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/86150
Reviewed-by: Mert Tümer <mert.tumer@collabora.com>
Tested-by: Mert Tümer <mert.tumer@collabora.com>
It is painful to check and search manually the PID to attach the LOKit
process when exists several pre-spawned waiting to load a document.
This patch helps to attach the debugger when the LOKit process is about
to load a document then send the "signal SIGUSR1" to resume it.
Change-Id: I3b15bd522c6ef3ef57dc3453b457dcf91f2661b9
Reviewed-on: https://gerrit.libreoffice.org/85430
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Henry Castro <hcastro@collabora.com>
Reviewed-by: Henry Castro <hcastro@collabora.com>
And disable a test that passes for me locally but fails on Jenkins.
Change-Id: Ifcdcedbac259247d784ce66185f95bfbe634381f
Reviewed-on: https://gerrit.libreoffice.org/84603
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
The WOPI multiple encoding magic is unpleasant; best not to try to
unwind it all now.
This fixes Download-As and SllideShow pieces.
cf. c806acb157
Change-Id: Ie06ad386c7a30e0f572e299e50fc949ce41b09f5
Now the iOS app is able to open a document called A#B?C\D&E❤️F🇨🇭G%AAHÖ.doc.
That is, with ‘#’, ‘?’, and ‘&’ that can mess things up when parsing URIs,
non-ASCII and even non-BMP characters, and an actual percent
character in the file name. Is that a complete enough test case? Will see.
Change-Id: I7fc53eaf8188f633d442d14a7f42814553169c71
Reviewed-on: https://gerrit.libreoffice.org/83395
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Tor Lillqvist <tml@collabora.com>
(cherry picked from commit b60ccd60c59efb5a03ea698c0205a8543846d9e2)
Reviewed-on: https://gerrit.libreoffice.org/83664
Tested-by: Aron Budea <aron.budea@collabora.com>
Saving the document on exit is not enough, we also need
to send it to the storage. We now force doing that,
even when there is no modifiction to the document
(i.e. a new version wasn't really saved).
Reviewed-on: https://gerrit.libreoffice.org/81336
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Tested-by: Andras Timar <andras.timar@collabora.com>
(cherry picked from commit f2913f20b03e916ce8a70f927ca3f5655a3768a8)
Change-Id: Ic4e1b1424f32d3141e98c936a51e47c9e4b9f753
Reviewed-on: https://gerrit.libreoffice.org/81576
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The initial child spawning takes significantly longer
than subsequent ones (for obvious reasons) and this
lead to unit-tests being sensitive to the timeout
we use for child spawning. Too short, and we
spawn more than we want on startup, too long
and crash-recovery tests fail (we don't
recover fast enough, as we wait too long before
spawning new children).
Dynamically setting the timeout allows us to give
longer timeout at startup, and reduce it afterwards.
Reviewed-on: https://gerrit.libreoffice.org/81194
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Tested-by: Andras Timar <andras.timar@collabora.com>
(cherry picked from commit 32fa1d95fc2ec65866d0cb47d619885182db7040)
Change-Id: I8423f5c6619e57030ab43d519aaa41d8712c36d3
Reviewed-on: https://gerrit.libreoffice.org/81570
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Using a view that hasn't finished loading
for saving the document will fail, and there
is no recovery from that (by using a loaded
view as fallback, for example).
This can happen when a new view is being loaded
while the last one is closing. We might in this
case end up using the new view to save the document
before closing it, however this will fail if
the new view hasn't loaded yet, potentially
losing some late modifications.
We avoid this by always picking a loaded view
for saving, or the first.
Reviewed-on: https://gerrit.libreoffice.org/80896
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Andras Timar <andras.timar@collabora.com>
(cherry picked from commit 77cbe4e376649dee87dbe37f95a48e9bb6403a0b)
Change-Id: I03ee9c48a9d43487dd90672060870972b98fdc99
Reviewed-on: https://gerrit.libreoffice.org/81567
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This gives users some feedback as to why loading
failed, which in this case is quite useful (as
it is highly unlikely retrying will help, since
the issue is with the document, being too large
or resource intensive).
Reviewed-on: https://gerrit.libreoffice.org/80325
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Tested-by: Andras Timar <andras.timar@collabora.com>
(cherry picked from commit d332e1d4a2999da4f93e7831ffc0d58e96fad1c6)
Change-Id: I01052105b8412023ac7651f3a92cc67fe739fd1a
Reviewed-on: https://gerrit.libreoffice.org/81563
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Michael Meeks <michael.meeks@collabora.com>
The shared_ptr allows the DocumentBroker's and ConvertToBroker to linger
after they are removed from the list, making ConvertToBroker::getInstanceCount
potentially larger than the number of documents transiently. Fix this
with a dispose method called on list removal.
Also make the arithmetic signed, to avoid unfortunate wrapping.
Also when the limit is large, don't show a message whatever happens.
Change-Id: Id2571429de48ae75e851c3fdc49e24a02aaaf6e9
Clients often need to communicate with their wopi hosts when
invoking Action_Save to provide more context when storing the
document in question. Action_Save now support passing arbitrary
string as ExtendedData entry that can be used by client to
pass any context or otherwise flags to the WOPI host, which
will receive it via the X-LOOL-WOPI-ExtendedData custom header.
See reference.html for more details.
Change-Id: I1814d1f3d984a553ffa60cec13d23b014ba59eb3
Reviewed-on: https://gerrit.libreoffice.org/74135
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Tested-by: Andras Timar <andras.timar@collabora.com>
