[Libreoffice-commits] online.git: 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 09:52:48 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 594d925c9e6817dc1ad82e285779eebe381b2945
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 7af1db615d99e346ed7891345be7f5b7a29a91c3
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 2e5a268bbc8dba4f5b2e832d85fd5dcbf6df170e
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 4598f5b..3de2188 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -500,7 +500,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 d4ede7136c7498ee5560a297a35e5c17d074ac55
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 dda262c..4598f5b 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -492,7 +492,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")
                 {
@@ -500,6 +499,9 @@ private:
                 }
                 else
                 {
+                    // Keep track of timestamps of incoming client messages that indicate editing
+                    if (token != "tile")
+                        time(&session->_lastUserInteractionTime);
                     queue->put(payload);
                 }
 
@@ -1381,10 +1383,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");
@@ -1405,10 +1407,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 381bd75fe41886349dbe8b6f8b51450d461dd175
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 f66ec18..dda262c 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -1381,6 +1381,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)
                             {
@@ -1403,6 +1405,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 41a1c0b3f953664edfe448ec60f4d11ceab5e86e
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 5928ad7..f66ec18 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -1367,52 +1367,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::trace("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::trace("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 e87f3ab..dab1e1d 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.
+
 SLEEPFORDEBUGGER        <seconds to sleep>
         sleep <n> seconds in the broken process after starting in
         order to allow a 'sudo gdb' session to 'attach <pid>' to them.


More information about the Libreoffice-commits mailing list