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 <t...@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 <t...@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 <t...@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 <t...@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 <t...@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 <t...@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. _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits