wsd: track modifying user commands

There is a race between the time of modifying
a document, receiving the modified flag, and
saving. This can happen when, for example,
the user modifies the document and closes
immediately. In that case, when uploading
we will not have the modified flag and will
not set the User-Modified attribute.

While this isn't 100% accurate, and it can
never be, it's still better to be conservative
and flag a version in storage as user-modified
than otherwise.

Change-Id: Ia504a7cddd4839bcbfeaaf9bf6c90ed8b68efa91
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>

4 files changed, 47 insertions, 0 deletions

diff --git a/common/Protocol.hpp b/common/Protocol.hpp
index 1ddaac0475..eb091c38a7 100644
--- a/common/Protocol.hpp
+++ b/common/Protocol.hpp
@@ -230,6 +230,29 @@ namespace COOLProtocol
                 token != "userinactive");
     }
 
+    /// Returns true if the token is a likely document modifying command.
+    /// This is never 100% accurate, but it is needed to filter out tokens
+    /// that certainly do not modify the document, such as 'load' and 'save'
+    /// commands. Some commands are certainly modifying, e.g. 'key', others
+    /// can only potentially be modifying, e.g. 'mouse' while dragging.
+    /// Note: this is only used when we don't have the modified flag from
+    /// Core so we flag the document as user-modified more accurately.
+    inline bool tokenIndicatesDocumentModification(const std::string& token)
+    {
+        // These keywords are chosen to cover the largest set of
+        // commands that may potentially modify the document.
+        // We need to assume modification rather than not.
+        return (
+            token.find("mouse") != std::string::npos || token.find("key") != std::string::npos ||
+            token.find("command") != std::string::npos ||
+            token.find("select") != std::string::npos || token.find("set") != std::string::npos ||
+            token.find("uno") != std::string::npos || token.find("input") != std::string::npos ||
+            token.find("move") != std::string::npos || token.find("paste") != std::string::npos ||
+            token.find("insert") != std::string::npos ||
+            token.find("resize") != std::string::npos ||
+            token.find("remove") != std::string::npos || token.find("sign") != std::string::npos);
+    }
+
     /// Returns the first line of a message.
     inline
     std::string getFirstLine(const char *message, const int length)
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index a796d62ea9..fe093ebc26 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -454,6 +454,10 @@ bool ClientSession::_handleInput(const char *buffer, int length)
         // Keep track of timestamps of incoming client messages that indicate user activity.
         updateLastActivityTime();
         docBroker->updateLastActivityTime();
+        if (COOLProtocol::tokenIndicatesDocumentModification(tokens[0]))
+        {
+            docBroker->updateLastModifyingActivityTime();
+        }
 
         if (isWritable() && isViewLoaded())
         {
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 9f66ce1447..7ac387b0fa 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -3591,7 +3591,10 @@ void DocumentBroker::dumpState(std::ostream& os)
     os << "\n  canSave: " << name(canSaveToDisk());
     os << "\n  canUpload: " << name(canUploadToStorage());
     os << "\n  needToUpload: " << name(needToUploadToStorage());
+    os << "\n  lastActivityTime: " << Util::getTimeForLog(now, _lastActivityTime);
     os << "\n  haveActivityAfterSaveRequest: " << haveActivityAfterSaveRequest();
+    os << "\n  lastModifyActivityTime: " << Util::getTimeForLog(now, _lastModifyActivityTime);
+    os << "\n  haveModifyActivityAfterSaveRequest: " << haveModifyActivityAfterSaveRequest();
     os << "\n  isViewFileExtension: " << _isViewFileExtension;
 
     if (_limitLifeSeconds > std::chrono::seconds::zero())
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 298adfbba0..6f9a4100a0 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -427,6 +427,12 @@ public:
 
     void updateLastActivityTime();
 
+    /// Sets the last activity timestamp that is most likely to modify the document.
+    void updateLastModifyingActivityTime()
+    {
+        _lastModifyActivityTime = std::chrono::steady_clock::now();
+    }
+
     /// This updates the editing sessionId which is used for auto-saving.
     void updateEditingSessionId(const std::string& viewId)
     {
@@ -609,6 +615,13 @@ private:
         return _saveManager.lastSaveRequestTime() < _lastActivityTime;
     }
 
+    /// True if there has been potentially *document-modifying* activity from a client
+    /// after we last *requested* saving, since there are race conditions while saving.
+    bool haveModifyActivityAfterSaveRequest() const
+    {
+        return _saveManager.lastSaveRequestTime() < _lastModifyActivityTime;
+    }
+
     /// Encodes whether or not saving is needed.
     STATE_ENUM(NeedToSave,
                No, //< No need to save, data up-to-date.
@@ -1291,6 +1304,10 @@ private:
     int _debugRenderedTileCount;
 
     std::chrono::steady_clock::time_point _lastActivityTime;
+
+    /// Time of the last interactive event that very likely modified the document.
+    std::chrono::steady_clock::time_point _lastModifyActivityTime;
+
     std::chrono::steady_clock::time_point _threadStart;
     std::chrono::milliseconds _loadDuration;
     std::chrono::milliseconds _wopiDownloadDuration;