wsd: autosave on disconnecting based on loaded sessions only

When a client disconnects, we autosave only when there
are no other sessions. The idea being that it'd be
wasteful to save on every client disconnect, so long
that we have a later chance of saving.

This doesn't hold, however, when the only other
remaining session is one that had just connected
and hasn't loaded a view yet. WSD wouldn't
know about this, but when unloading the
disconnecting session, Kit will exit the process
as the last view is gone, losing all unsaved changes.

This patch tracks the sessions in WSD that have
loaded a view and takes that into account when
deciding to autosave on disconnect or not, thereby
fixing this corner case that may result in data loss.

Change-Id: I699721f2d710128ed4db65943b9357dff54380f5
Reviewed-on: https://gerrit.libreoffice.org/33126
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>

wsd/ClientSession.cpp

@@ -38,6 +38,7 @@ ClientSession::ClientSession(const std::string& id,
     _isReadOnly(readOnly),
     _isDocumentOwner(false),
     _loadPart(-1),
+    _isLoadRequested(false),
     _stop(false)
 {
     LOG_INF("ClientSession ctor [" << getName() << "].");
@@ -57,7 +58,11 @@ ClientSession::~ClientSession()
     {
         _senderThread.join();
     }
+}
 
+bool ClientSession::isLoaded() const
+{
+    return _isLoadRequested && _peer && _peer->gotStatus();
 }
 
 void ClientSession::bridgePrisonerSession()
@@ -279,7 +284,11 @@ bool ClientSession::loadDocument(const char* /*buffer*/, int /*length*/, StringT
             oss << " options=" << _docOptions;
 
         const auto loadRequest = oss.str();
-        return forwardToChild(loadRequest, docBroker);
+        if (forwardToChild(loadRequest, docBroker))
+        {
+            _isLoadRequested = true;
+            return true;
+        }
     }
     catch (const Poco::SyntaxException&)
     {

wsd/ClientSession.hpp

@@ -35,6 +35,8 @@ public:
     void setReadOnly();
     bool isReadOnly() const { return _isReadOnly; }
 
+    bool isLoaded() const;
+
     /// Create and connect Prisoner Session between DocumentBroker and us.
     void bridgePrisonerSession();
     std::shared_ptr<PrisonerSession> getPeer() const { return _peer; }
@@ -147,6 +149,8 @@ private:
 
     int _loadPart;
 
+    bool _isLoadRequested;
+
     /// Wopi FileInfo object
     std::unique_ptr<WopiStorage::WOPIFileInfo> _wopiFileInfo;
 

wsd/DocumentBroker.cpp

@@ -949,6 +949,7 @@ bool DocumentBroker::startDestroy(const std::string& id)
         for (const auto& it : _sessions)
         {
             if (it.second->getId() != id &&
+                it.second->isLoaded() &&
                 !it.second->isReadOnly())
             {
                 // Found another editable.

wsd/PrisonerSession.cpp

@@ -37,7 +37,8 @@ PrisonerSession::PrisonerSession(std::shared_ptr<ClientSession> clientSession,
     Session("ToPrisoner-" + clientSession->getId(), clientSession->getId(), nullptr),
     _docBroker(std::move(docBroker)),
     _peer(clientSession),
-    _curPart(0)
+    _curPart(0),
+    _gotStatus(false)
 {
     LOG_INF("PrisonerSession ctor [" << getName() << "].");
 }
@@ -170,6 +171,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
         }
         else if (tokens[0] == "status:")
         {
+            _gotStatus = true;
             _docBroker->setLoaded();
 
             // Forward the status response to the client.

wsd/PrisonerSession.hpp

@@ -27,6 +27,9 @@ public:
 
     virtual ~PrisonerSession();
 
+    /// Returns true if we've got status message.
+    bool gotStatus() const { return _gotStatus; }
+
 private:
     /// Handle messages from the Kit to the client.
     virtual bool _handleInput(const char* buffer, int length) override;
@@ -38,6 +41,7 @@ private:
     std::shared_ptr<DocumentBroker> _docBroker;
     std::weak_ptr<ClientSession> _peer;
     int _curPart;
+    bool _gotStatus;
 };
 
 #endif