[Libreoffice-commits] online.git: Branch 'distro/collabora/cloudsuite-rc1' - 6 commits - loolwsd/LOOLProtocol.hpp loolwsd/LOOLWSD.cpp loolwsd/MasterProcessSession.cpp loolwsd/MasterProcessSession.hpp loolwsd/README.vars

Tor Lillqvist tml at collabora.com
Wed Apr 6 08:54:00 UTC 2016


 loolwsd/LOOLProtocol.hpp         |   51 +++-----------------------
 loolwsd/LOOLWSD.cpp              |   74 +++++++++++++++++++++------------------
 loolwsd/MasterProcessSession.cpp |    2 -
 loolwsd/MasterProcessSession.hpp |    2 -
 loolwsd/README.vars              |    4 ++
 5 files changed, 54 insertions(+), 79 deletions(-)

New commits:
commit 0e5ccfdaa1bb88e336a05d6f6f951423aa2646cb
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Apr 6 11:48:18 2016 +0300

    This is not the place for this documentation any more
    
    Besides, I don't think we actually use such a terminology as that
    comment claimed.

diff --git a/loolwsd/LOOLProtocol.hpp b/loolwsd/LOOLProtocol.hpp
index e4b4cbe..f482041 100644
--- a/loolwsd/LOOLProtocol.hpp
+++ b/loolwsd/LOOLProtocol.hpp
@@ -18,18 +18,6 @@
 
 namespace LOOLProtocol
 {
-    // The frames sent from the client to the server are called
-    // "commands" and those sent from the server to the client are
-    // called "messages". At least until I come up with a better
-    // terminology.
-
-    // I don't want to call the latter "responses"
-    // because they are not necessarily responses to some "request" or
-    // "command". Also "event" would be misleading because that is
-    // typically used for things originated by the user, like key or
-    // mouse events. And in fact, those are here part of the
-    // "commands".
-
     // Protocol Version Number.
     // See protocol.txt.
     constexpr unsigned ProtocolMajorVersionNumber = 0;
commit e7cc3513f7caed993e3cac0653af10cb2ae3aea7
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Apr 6 11:44:29 2016 +0300

    These enums are indeed not used, and are not a good idea, so kill them
    
    To use such enums would be a mistake. It is quite enough to just use
    the message tokens as strings. Duplicating them as enums will just
    lead to the enums getting out of synch (as they already were). We
    would also need functions to covert between the string and enum
    forms. It seems to be hard enough to keep the messages documented in
    protocol.txt.

diff --git a/loolwsd/LOOLProtocol.hpp b/loolwsd/LOOLProtocol.hpp
index b5d8c64..e4b4cbe 100644
--- a/loolwsd/LOOLProtocol.hpp
+++ b/loolwsd/LOOLProtocol.hpp
@@ -30,38 +30,6 @@ namespace LOOLProtocol
     // mouse events. And in fact, those are here part of the
     // "commands".
 
-    // Not sure if these enums will be needed
-    enum class Command
-    {
-        GETTEXTSELECTION,
-        KEY,
-        LOAD,
-        MOUSE,
-        RESETSELECTION,
-        SAVEAS,
-        SELECTGRAPHIC,
-        SELECTTEXT,
-        STATUS,
-        TILE,
-        UNO,
-    };
-
-    enum class Message
-    {
-        CHILD,
-        CURSOR_VISIBLE,
-        ERROR,
-        GRAPHIC_SELECTION,
-        HYPERLINK_CLICKED,
-        INVALIDATE_CURSOR,
-        INVALIDATE_TILES,
-        STATUS,
-        TEXT_SELECTION,
-        TEXT_SELECTION_END,
-        TEXT_SELECTION_START,
-        TILE,
-    };
-
     // Protocol Version Number.
     // See protocol.txt.
     constexpr unsigned ProtocolMajorVersionNumber = 0;
commit 6fe77693331871b0aa2da237609d86681c8b413b
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Apr 6 11:36:33 2016 +0300

    bccu#1399: Factor out check whether message indicates user interaction
    
    Add a function to determine whether a client message indicates user
    interaction. We need that distinction when deciding when to do an
    automatic ("idle" or "auto") save of document being edited.
    
    "Interaction" is a loose term, possibly what we actually want is to
    see whether the user is actively doing an edit that changes the
    contents of meta-data of the document.

diff --git a/loolwsd/LOOLProtocol.hpp b/loolwsd/LOOLProtocol.hpp
index 9a5ac8b..b5d8c64 100644
--- a/loolwsd/LOOLProtocol.hpp
+++ b/loolwsd/LOOLProtocol.hpp
@@ -107,6 +107,13 @@ namespace LOOLProtocol
         return getFirstToken(message.data(), message.size());
     }
 
+    inline
+    bool tokenIndicatesUserInteraction(const std::string& token)
+    {
+        return (token != "tile" &&
+                token != "status");
+    }
+
     /// Returns the first line of a message.
     inline
     std::string getFirstLine(const char *message, const int length)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 7329e85..24c9642 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -485,7 +485,7 @@ private:
                 else
                 {
                     // Keep track of timestamps of incoming client messages that indicate editing
-                    if (token != "tile")
+                    if (tokenIndicatesUserInteraction(token))
                         time(&session->_lastUserInteractionTime);
                     queue->put(payload);
                 }
commit 67df2a81d71f92d820c4cbcb01ebb03185896267
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Apr 6 10:28:04 2016 +0300

    bccu#1399: Restrict the meaning of the 'last message' timestamp and rename
    
    It should keep track only of messages that indicate explicit editing
    actions by the user. For now, treat anything except 'tile' messages as
    such.

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 1208873..7329e85 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -477,7 +477,6 @@ private:
         IoUtil::SocketProcessor(ws, response,
                 [&session, &queue, &normalShutdown](const std::vector<char>& payload)
             {
-                time(&session->_lastMessageTime);
                 const auto token = LOOLProtocol::getFirstToken(payload);
                 if (token == "disconnect")
                 {
@@ -485,6 +484,9 @@ private:
                 }
                 else
                 {
+                    // Keep track of timestamps of incoming client messages that indicate editing
+                    if (token != "tile")
+                        time(&session->_lastUserInteractionTime);
                     queue->put(payload);
                 }
 
@@ -1332,10 +1334,10 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
                         std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
                         for (auto& sessionIt: brokerIt.second->_wsSessions)
                         {
-                            // If a message has arrived from the client since we last did an idle save,
-                            // and it is more than 30 seconds since the message arrived, do a save.
-                            if (sessionIt.second->_lastMessageTime > sessionIt.second->_idleSaveTime &&
-                                sessionIt.second->_lastMessageTime < now - 30)
+                            // If no editing done by the client since we last did an idle save,
+                            // and it is more than 30 seconds since the last edit, do an idle save.
+                            if (sessionIt.second->_lastUserInteractionTime > sessionIt.second->_idleSaveTime &&
+                                sessionIt.second->_lastUserInteractionTime < now - 30)
                             {
                                 Log::info("Idle save triggered for session " + sessionIt.second->getId());
                                 sessionIt.second->getQueue()->put("uno .uno:Save");
@@ -1356,10 +1358,10 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
                         std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
                         for (auto& sessionIt: brokerIt.second->_wsSessions)
                         {
-                            // If messages have arrived from the client since we last did an idle or auto
-                            // save, do a save.
-                            if (sessionIt.second->_lastMessageTime >= sessionIt.second->_idleSaveTime &&
-                                sessionIt.second->_lastMessageTime >= sessionIt.second->_autoSaveTime)
+                            // If some editing by from the client since we last did an (idle or
+                            // auto) save, do an auto save.
+                            if (sessionIt.second->_lastUserInteractionTime >= sessionIt.second->_idleSaveTime &&
+                                sessionIt.second->_lastUserInteractionTime >= sessionIt.second->_autoSaveTime)
                             {
                                 Log::info("Auto-save triggered for session " + sessionIt.second->getId());
                                 sessionIt.second->getQueue()->put("uno .uno:Save");
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index 4544063..0072b8d 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -39,7 +39,7 @@ MasterProcessSession::MasterProcessSession(const std::string& id,
                                            std::shared_ptr<DocumentBroker> docBroker,
                                            std::shared_ptr<BasicTileQueue> queue) :
     LOOLSession(id, kind, ws),
-    _lastMessageTime(0),
+    _lastUserInteractionTime(0),
     _idleSaveTime(0),
     _autoSaveTime(0),
     _curPart(0),
diff --git a/loolwsd/MasterProcessSession.hpp b/loolwsd/MasterProcessSession.hpp
index da7105f..c5b962c 100644
--- a/loolwsd/MasterProcessSession.hpp
+++ b/loolwsd/MasterProcessSession.hpp
@@ -60,7 +60,7 @@ public:
     static std::mutex AvailableChildSessionMutex;
     static std::condition_variable AvailableChildSessionCV;
 
-    time_t _lastMessageTime;
+    time_t _lastUserInteractionTime;
     time_t _idleSaveTime;
     time_t _autoSaveTime;
 
commit 8d706860f67cfc1f80146253254db89390243e38
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Apr 6 10:23:13 2016 +0300

    Add some clarifying comments

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 80e08ac..1208873 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -1332,6 +1332,8 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
                         std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
                         for (auto& sessionIt: brokerIt.second->_wsSessions)
                         {
+                            // If a message has arrived from the client since we last did an idle save,
+                            // and it is more than 30 seconds since the message arrived, do a save.
                             if (sessionIt.second->_lastMessageTime > sessionIt.second->_idleSaveTime &&
                                 sessionIt.second->_lastMessageTime < now - 30)
                             {
@@ -1354,6 +1356,8 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
                         std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
                         for (auto& sessionIt: brokerIt.second->_wsSessions)
                         {
+                            // If messages have arrived from the client since we last did an idle or auto
+                            // save, do a save.
                             if (sessionIt.second->_lastMessageTime >= sessionIt.second->_idleSaveTime &&
                                 sessionIt.second->_lastMessageTime >= sessionIt.second->_autoSaveTime)
                             {
commit c3c49129f0ffd14b3120971da87f3418809c9f63
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Apr 6 10:15:16 2016 +0300

    Introduce LOOL_NO_AUTOSAVE environment variable

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index e59bb53..80e08ac 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -1318,52 +1318,54 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
         }
         else // pid == 0, no children have died
         {
-            time_t now = time(NULL);
-            if (now >= last30SecCheck + 30)
+            if (!std::getenv("LOOL_NO_AUTOSAVE"))
             {
-                Log::debug("30-second check");
-                last30SecCheck = now;
-
-                std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
-                for (auto& brokerIt : docBrokers)
+                time_t now = time(NULL);
+                if (now >= last30SecCheck + 30)
                 {
-                    std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
-                    for (auto& sessionIt: brokerIt.second->_wsSessions)
+                    Log::debug("30-second check");
+                    last30SecCheck = now;
+
+                    std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
+                    for (auto& brokerIt : docBrokers)
                     {
-                        if (sessionIt.second->_lastMessageTime > sessionIt.second->_idleSaveTime &&
-                            sessionIt.second->_lastMessageTime < now - 30)
+                        std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
+                        for (auto& sessionIt: brokerIt.second->_wsSessions)
                         {
-                            Log::info("Idle save triggered for session " + sessionIt.second->getId());
-                            sessionIt.second->getQueue()->put("uno .uno:Save");
-
-                            sessionIt.second->_idleSaveTime = now;
+                            if (sessionIt.second->_lastMessageTime > sessionIt.second->_idleSaveTime &&
+                                sessionIt.second->_lastMessageTime < now - 30)
+                            {
+                                Log::info("Idle save triggered for session " + sessionIt.second->getId());
+                                sessionIt.second->getQueue()->put("uno .uno:Save");
+
+                                sessionIt.second->_idleSaveTime = now;
+                            }
                         }
                     }
                 }
-            }
-            if (now >= lastFiveMinuteCheck + 300)
-            {
-                Log::debug("Five-minute check");
-                lastFiveMinuteCheck = now;
-
-                std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
-                for (auto& brokerIt : docBrokers)
+                if (now >= lastFiveMinuteCheck + 300)
                 {
-                    std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
-                    for (auto& sessionIt: brokerIt.second->_wsSessions)
+                    Log::debug("Five-minute check");
+                    lastFiveMinuteCheck = now;
+
+                    std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
+                    for (auto& brokerIt : docBrokers)
                     {
-                        if (sessionIt.second->_lastMessageTime >= sessionIt.second->_idleSaveTime &&
-                            sessionIt.second->_lastMessageTime >= sessionIt.second->_autoSaveTime)
+                        std::unique_lock<std::mutex> sessionsLock(brokerIt.second->_wsSessionsMutex);
+                        for (auto& sessionIt: brokerIt.second->_wsSessions)
                         {
-                            Log::info("Auto-save triggered for session " + sessionIt.second->getId());
-                            sessionIt.second->getQueue()->put("uno .uno:Save");
-
-                            sessionIt.second->_autoSaveTime = now;
+                            if (sessionIt.second->_lastMessageTime >= sessionIt.second->_idleSaveTime &&
+                                sessionIt.second->_lastMessageTime >= sessionIt.second->_autoSaveTime)
+                            {
+                                Log::info("Auto-save triggered for session " + sessionIt.second->getId());
+                                sessionIt.second->getQueue()->put("uno .uno:Save");
+
+                                sessionIt.second->_autoSaveTime = now;
+                            }
                         }
                     }
                 }
             }
-
             sleep(MAINTENANCE_INTERVAL*2);
         }
     }
diff --git a/loolwsd/README.vars b/loolwsd/README.vars
index 59108ab..03a5371 100644
--- a/loolwsd/README.vars
+++ b/loolwsd/README.vars
@@ -16,6 +16,10 @@ LOOL_LOGLEVEL           <level>
 		error, warning, notice, information, debug,
 		trace
 
+LOOL_NO_AUTOSAVE        <set/unset>
+        if set avoids automatic saving of the document being
+        edited.
+
 LOK_NO_PREINIT          <set/unset>
         set this to disable pre-initialization of LOK instances.
 


More information about the Libreoffice-commits mailing list