From 05620be4c5d0369d160ac79f4eaee00013229324 Mon Sep 17 00:00:00 2001
From: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Date: Wed, 11 Jan 2017 16:45:14 -0500
Subject: [PATCH] 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   | 11 ++++++++++-
 wsd/ClientSession.hpp   |  4 ++++
 wsd/DocumentBroker.cpp  |  1 +
 wsd/PrisonerSession.cpp |  4 +++-
 wsd/PrisonerSession.hpp |  4 ++++
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 9715f32fb..f24a525c2 100644
--- a/wsd/ClientSession.cpp
+++ b/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&)
     {
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 7952029c5..c946a940d 100644
--- a/wsd/ClientSession.hpp
+++ b/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;
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 7d7bfee5c..f1db30b36 100644
--- a/wsd/DocumentBroker.cpp
+++ b/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.
diff --git a/wsd/PrisonerSession.cpp b/wsd/PrisonerSession.cpp
index 18484134e..16cb760aa 100644
--- a/wsd/PrisonerSession.cpp
+++ b/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.
diff --git a/wsd/PrisonerSession.hpp b/wsd/PrisonerSession.hpp
index 4085fb3a9..5a333a28a 100644
--- a/wsd/PrisonerSession.hpp
+++ b/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