ios/Mobile/CODocument.h | 1
ios/Mobile/CODocument.mm | 20 +++++++++----------
ios/Mobile/DocumentViewController.mm | 36 +++++++++++++++++++----------------
wsd/DocumentBroker.cpp | 8 +++----
4 files changed, 35 insertions(+), 30 deletions(-)
New commits:
commit 0930286e2d8fd2ba1e4820758246ee3766040254
Author: Tor Lillqvist <t...@collabora.com>
AuthorDate: Tue Apr 7 21:21:55 2020 +0300
Commit: Tor Lillqvist <t...@collabora.com>
CommitDate: Wed Apr 8 00:01:30 2020 +0200
Fix problems after my 293f4913d2cdfe5385e2cdc0e3bebde281da1578
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 <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Tor Lillqvist <t...@collabora.com>
diff --git a/ios/Mobile/CODocument.h b/ios/Mobile/CODocument.h
index 82e2e80e3..79f772a1b 100644
--- a/ios/Mobile/CODocument.h
+++ b/ios/Mobile/CODocument.h
@@ -18,6 +18,7 @@
@interface CODocument : UIDocument {
@public
int fakeClientFd;
+ NSURL *copyFileURL;
}
@property (weak) DocumentViewController *viewController;
diff --git a/ios/Mobile/CODocument.mm b/ios/Mobile/CODocument.mm
index 095d449ea..5dcd9b136 100644
--- a/ios/Mobile/CODocument.mm
+++ b/ios/Mobile/CODocument.mm
@@ -37,14 +37,7 @@
@implementation CODocument
- (id)contentsForType:(NSString*)typeName error:(NSError **)errorPtr {
- // Somehow this doesn't feel right, creating an NSFileWrapper around the
file that was given to
- // us for loadFromContents. I get the vague feeling that the file is
perhaps just a temporary
- // data container created by the system for us to be used while loading
the document data, and
- // not the actual permanent document, especially in the case of things
like NextCloud. Or is it?
- // Is saving back to the file (which we have already saved to in the core
code by the time we
- // get here) correct? This does seem to work, though. Sadly the Apple
documentation is a bit
- // lacking about how these things *really* work.
- return [[NSFileWrapper alloc] initWithURL:[self fileURL] options:0
error:errorPtr];
+ return [NSData dataWithContentsOfFile:[copyFileURL path] options:0
error:errorPtr];
}
- (BOOL)loadFromContents:(id)contents ofType:(NSString *)typeName
error:(NSError **)errorPtr {
@@ -55,11 +48,18 @@
return YES;
fakeClientFd = fakeSocketSocket();
- NSString *uri = [[self fileURL] absoluteString];
+
+ copyFileURL = [[[NSFileManager defaultManager] temporaryDirectory]
URLByAppendingPathComponent:[[[self fileURL] path] lastPathComponent]];
+
+ NSError *error;
+ [[NSFileManager defaultManager] removeItemAtURL:copyFileURL error:nil];
+ [[NSFileManager defaultManager] copyItemAtURL:[self fileURL]
toURL:copyFileURL error:&error];
+ if (error != nil)
+ return NO;
NSURL *url = [[NSBundle mainBundle] URLForResource:@"loleaflet"
withExtension:@"html"];
NSURLComponents *components = [NSURLComponents componentsWithURL:url
resolvingAgainstBaseURL:NO];
- components.queryItems = @[ [NSURLQueryItem queryItemWithName:@"file_path"
value:uri],
+ components.queryItems = @[ [NSURLQueryItem queryItemWithName:@"file_path"
value:[copyFileURL absoluteString]],
[NSURLQueryItem
queryItemWithName:@"closebutton" value:@"1"],
[NSURLQueryItem queryItemWithName:@"permission"
value:@"edit"],
[NSURLQueryItem queryItemWithName:@"lang"
value:app_locale]
diff --git a/ios/Mobile/DocumentViewController.mm
b/ios/Mobile/DocumentViewController.mm
index 94dbd839c..97ac7ab96 100644
--- a/ios/Mobile/DocumentViewController.mm
+++ b/ios/Mobile/DocumentViewController.mm
@@ -348,7 +348,7 @@ static IMP standardImpOfInputAccessoryView = nil;
// First we simply send it the URL. This corresponds to the GET
request with Upgrade to
// WebSocket.
- std::string url([[[self.document fileURL] absoluteString]
UTF8String]);
+ std::string url([[self.document->copyFileURL absoluteString]
UTF8String]);
p.fd = self.document->fakeClientFd;
p.events = POLLOUT;
fakeSocketPoll(&p, 1, -1);
@@ -469,21 +469,25 @@ static IMP standardImpOfInputAccessoryView = nil;
// Close one end of the socket pair, that will wake up the forwarding
thread above
fakeSocketClose(closeNotificationPipeForForwardingThread[0]);
- // I suspect that what this will do (in -[CODocument
contentsForType:error:]) is to read the
- // contents of the file that LO core already saved, and overwrite it with
the same contents.
- [self.document saveToURL:[self.document fileURL]
- forSaveOperation:UIDocumentSaveForOverwriting
- completionHandler:^(BOOL success) {
- LOG_TRC("save completion handler gets " << (success?"YES":"NO"));
- }];
-
- // Wait for lokit_main thread to exit
- std::lock_guard<std::mutex> lock(LOOLWSD::lokit_main_mutex);
-
- theSingleton = nil;
-
- // And only then let the document browsing view show up again
- [self dismissDocumentViewController];
+ // We can't wait for the LOOLWSD::lokit_main_mutex directly here because
this is called on the
+ // main queue and the main queue must be ready to execute code dispatched
by the system APIs
+ // used to do document saving.
+ dispatch_async(dispatch_get_global_queue( DISPATCH_QUEUE_PRIORITY_DEFAULT,
0),
+ ^{
+ // Wait for lokit_main thread to exit
+ std::lock_guard<std::mutex>
lock(LOOLWSD::lokit_main_mutex);
+
+ theSingleton = nil;
+
+ [[NSFileManager defaultManager]
removeItemAtURL:self.document->copyFileURL error:nil];
+
+ // And only then let the document browsing view show up
again. The
+ // dismissViewControllerAnimated must be done on the
main queue.
+ dispatch_async(dispatch_get_main_queue(),
+ ^{
+ [self dismissDocumentViewController];
+ });
+ });
}
+ (DocumentViewController*)singleton {
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 0d5ce909d..4287624e9 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1289,14 +1289,14 @@ bool DocumentBroker::sendUnoSave(const std::string&
sessionId, bool dontTerminat
forwardToChild(sessionId, command);
_lastSaveRequestTime = std::chrono::steady_clock::now();
#ifdef IOS
- // We need to do this so that file provider extensions notice. Just
like in
- // -[DocumentViewController bye] I suspect that will read the file and
then overwrite it
- // with the same contents, but oh well.
+ // We need to do this here, also for auto-save, so that file provider
extensions notice.
+
CODocument *document = [[DocumentViewController singleton] document];
+
[document saveToURL:[[[DocumentViewController singleton] document]
fileURL]
forSaveOperation:UIDocumentSaveForOverwriting
completionHandler:^(BOOL success) {
- LOG_TRC("save completion handler gets " <<
(success?"YES":"NO"));
+ LOG_TRC("DocumentBroker::sendUnoSave() save completion handler
gets " << (success?"YES":"NO"));
}];
#endif
return true;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
