[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4-0' - common/Session.cpp common/Session.hpp kit/Kit.cpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

Michael Meeks (via logerrit) logerrit at kemper.freedesktop.org
Mon Jan 20 12:32:06 UTC 2020


 common/Session.cpp     |   16 -------------
 common/Session.hpp     |   51 ++++++++++++++++++++++++++++++++++--------
 kit/Kit.cpp            |   59 ++++++++++++++++++++++++++-----------------------
 wsd/ClientSession.cpp  |   23 ++++++++++++++-----
 wsd/ClientSession.hpp  |    2 +
 wsd/DocumentBroker.cpp |    5 ----
 wsd/DocumentBroker.hpp |    3 +-
 wsd/LOOLWSD.cpp        |    5 +++-
 8 files changed, 101 insertions(+), 63 deletions(-)

New commits:
commit bcb6cf82edcee8072bffa4045c5abfc82f41c579
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Thu Jan 2 21:11:54 2020 +0000
Commit:     Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Jan 20 13:31:47 2020 +0100

    watermarking: create SessionMap template to canonicalize views.
    
    Use a fully reliable uniqueness check, rather than a hash, and get
    simpler ids as a bonus. Fetch view data from the session itself
    rather than passing it in too.
    
    Change-Id: Ibcd625156b5a98eb280e35d6537b5c8c026d0197
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/86150
    Reviewed-by: Mert Tümer <mert.tumer at collabora.com>
    Tested-by: Mert Tümer <mert.tumer at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/87064
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/common/Session.cpp b/common/Session.cpp
index 414d35869..116d2786f 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -255,22 +255,6 @@ void Session::getIOStats(uint64_t &sent, uint64_t &recv)
     }
 }
 
- void Session::setHash(const std::string& text)
- {
-     int hash = 0x811C9DC5;
-     int prime = 0x1000193;
-
-     if (!text.empty())
-     {
-        for (unsigned int i = 0; i < text.length(); ++i)
-        {
-            hash += hash ^ text[i];
-            hash *= prime;
-        }
-     }
-     _hash = abs(hash);
- }
-
 void Session::dumpState(std::ostream& os)
 {
     WebSocketHandler::dumpState(os);
diff --git a/common/Session.hpp b/common/Session.hpp
index d70d687c2..5ffc22d10 100644
--- a/common/Session.hpp
+++ b/common/Session.hpp
@@ -14,7 +14,9 @@
 #include <cassert>
 #include <memory>
 #include <mutex>
+#include <map>
 #include <ostream>
+#include <type_traits>
 
 #include <Poco/Buffer.h>
 #include <Poco/Path.h>
@@ -29,6 +31,39 @@
 #include "TileCache.hpp"
 #include "WebSocketHandler.hpp"
 
+class Session;
+
+template<class T>
+class SessionMap : public std::map<std::string, std::shared_ptr<T> >
+{
+    std::map<std::string, int> _canonicalIds;
+public:
+    SessionMap() {
+        static_assert(std::is_base_of<Session, T>::value, "sessions must have base of Session");
+    }
+    /// Generate a unique key for this set of view properties
+    int getCanonicalId(const std::string &viewProps)
+    {
+        if (viewProps.empty())
+            return 0;
+        for (auto &it : _canonicalIds) {
+            if (it.first == viewProps)
+                return it.second;
+        }
+        size_t id = _canonicalIds.size() + 1;
+        _canonicalIds[viewProps] = id;
+        return id;
+    }
+    std::shared_ptr<T> findByCanonicalId(int id)
+    {
+        for (auto &it : *this) {
+            if (it.second->getCanonicalViewId() == id)
+                return it.second;
+        }
+        return std::shared_ptr<T>();
+    }
+};
+
 /// Base class of a WebSocket session.
 class Session : public WebSocketHandler
 {
@@ -122,11 +157,11 @@ public:
 
     const std::string& getJailedFilePathAnonym() const { return _jailedFilePathAnonym; }
 
-    int getHash() { return _hash; }
-
-    void setHash(const std::string& text);
-
-    void setHash(const int hash) { _hash = hash; };
+    int  getCanonicalViewId() { return _canonicalViewId; }
+    template<class T> void recalcCanonicalViewId(SessionMap<T> &map)
+    {
+        _canonicalViewId = map.getCanonicalId(_watermarkText);
+    }
 
 
 protected:
@@ -220,10 +255,8 @@ private:
     /// Language for the document based on what the user has in the UI.
     std::string _lang;
 
-    /// Hash for normalizedViewId which is basically an identity for the tile to
-    /// choose what to render on and send it to its subscribers
-    /// it is the close-to-unique integer representation of a string like Watermarks etc.
-    int _hash;
+    /// the canonical id unique to the set of rendering properties of this session
+    int _canonicalViewId;
 };
 
 #endif
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index e449a9b95..c1477736c 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -856,6 +856,20 @@ public:
             return;
         }
 
+        // Find a session matching our view / render settings.
+        const auto session = _sessions.findByCanonicalId(tile.getNormalizedViewId());
+        if (!session)
+        {
+            LOG_ERR("Session is not found. Maybe exited after rendering request.");
+            return;
+        }
+
+#ifdef FIXME_RENDER_SETTINGS
+        // if necessary select a suitable rendering view eg. with 'show non-printing chars'
+        if (tile.getNormalizedViewId())
+            _loKitDocument->setView(session->getViewId());
+#endif
+
         const double area = tile.getWidth() * tile.getHeight();
         Timestamp timestamp;
         _loKitDocument->paintPartTile(pixmap.data(), tile.getPart(),
@@ -868,16 +882,6 @@ public:
                 " ms (" << area / elapsed << " MP/s).");
         const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode());
 
-        int nViewId = tile.getNormalizedViewId();
-        const auto it = std::find_if(_sessions.begin(), _sessions.end(), [nViewId](const std::pair<std::string, std::shared_ptr<ChildSession>>& val){ return (val.second)->getHash() == nViewId; });
-        const auto& session = it->second;
-
-        if (it == _sessions.end())
-        {
-            LOG_ERR("Session is not found. Maybe exited after rendering request.");
-            return;
-        }
-
         int pixelWidth = tile.getWidth();
         int pixelHeight = tile.getHeight();
 
@@ -968,6 +972,21 @@ public:
             return;
         }
 
+        // Find a session matching our view / render settings.
+        const auto session = _sessions.findByCanonicalId(tileCombined.getNormalizedViewId());
+        if (!session)
+        {
+            LOG_ERR("Session is not found. Maybe exited after rendering request.");
+            return;
+        }
+
+#ifdef FIXME_RENDER_SETTINGS
+        // if necessary select a suitable rendering view eg. with 'show non-printing chars'
+        if (tileCombined.getNormalizedViewId())
+            _loKitDocument->setView(session->getViewId());
+#endif
+
+        // Render the whole area
         const double area = pixmapWidth * pixmapHeight;
         Timestamp timestamp;
         LOG_TRC("Calling paintPartTile(" << (void*)pixmap.data() << ")");
@@ -982,16 +1001,6 @@ public:
                 " rendered in " << (elapsed/1000.) << " ms (" << area / elapsed << " MP/s).");
         const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode());
 
-        int nViewId = tileCombined.getNormalizedViewId();
-        const auto it = std::find_if(_sessions.begin(), _sessions.end(), [nViewId](const std::pair<std::string, std::shared_ptr<ChildSession>>& val){ return (val.second)->getHash() == nViewId; });
-        const auto& session = it->second;
-
-        if (it == _sessions.end())
-        {
-            LOG_ERR("Session is not found. Maybe exited after rendering request.");
-            return;
-        }
-
         std::vector<char> output;
         output.reserve(pixmapSize);
 
@@ -1653,13 +1662,9 @@ private:
                 sessionId << "] loaded view [" << viewId << "]. Have " <<
                 viewCount << " view" << (viewCount != 1 ? "s." : "."));
 
-        if (!watermarkText.empty())
-        {
+        if (session->hasWatermark())
             session->_docWatermark.reset(new Watermark(_loKitDocument, watermarkText, session));
-            session->setHash(watermarkText);
-        }
-        else
-            session->setHash(0);
+        session->recalcCanonicalViewId(_sessions);
 
         return _loKitDocument;
     }
@@ -1984,7 +1989,7 @@ private:
     int _editorId;
     bool _editorChangeWarning;
     std::map<int, std::unique_ptr<CallbackDescriptor>> _viewIdToCallbackDescr;
-    std::map<std::string, std::shared_ptr<ChildSession>> _sessions;
+    SessionMap<ChildSession> _sessions;
 
     std::map<int, std::chrono::steady_clock::time_point> _lastUpdatedAt;
     std::map<int, int> _speedCount;
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 42b8a5f73..3e23e356a 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -14,6 +14,7 @@
 #include <fstream>
 #include <sstream>
 #include <memory>
+#include <unordered_map>
 
 #include <Poco/Net/HTTPResponse.h>
 #include <Poco/StreamCopier.h>
@@ -35,6 +36,9 @@ using namespace LOOLProtocol;
 using Poco::Path;
 using Poco::StringTokenizer;
 
+static std::mutex GlobalSessionMapMutex;
+static std::unordered_map<std::string, std::weak_ptr<ClientSession>> GlobalSessionMap;
+
 ClientSession::ClientSession(const std::string& id,
                              const std::shared_ptr<DocumentBroker>& docBroker,
                              const Poco::URI& uriPublic,
@@ -69,10 +73,19 @@ ClientSession::ClientSession(const std::string& id,
 
 }
 
+void ClientSession::construct()
+{
+    std::unique_lock<std::mutex> lock(GlobalSessionMapMutex);
+    GlobalSessionMap[getId()] = shared_from_this();
+}
+
 ClientSession::~ClientSession()
 {
     const size_t curConnections = --LOOLWSD::NumConnections;
     LOG_INF("~ClientSession dtor [" << getName() << "], current number of connections: " << curConnections);
+
+    std::unique_lock<std::mutex> lock(GlobalSessionMapMutex);
+    GlobalSessionMap.erase(getId());
 }
 
 void ClientSession::handleIncomingMessage(SocketDisposition &disposition)
@@ -626,7 +639,7 @@ bool ClientSession::sendTile(const char * /*buffer*/, int /*length*/, const std:
     try
     {
         TileDesc tileDesc = TileDesc::parse(tokens);
-        tileDesc.setNormalizedViewId(getHash());
+        tileDesc.setNormalizedViewId(getCanonicalViewId());
         docBroker->handleTileRequest(tileDesc, shared_from_this());
     }
     catch (const std::exception& exc)
@@ -644,7 +657,7 @@ bool ClientSession::sendCombinedTiles(const char* /*buffer*/, int /*length*/, co
     try
     {
         TileCombined tileCombined = TileCombined::parse(tokens);
-        tileCombined.setNormalizedViewId(getHash());
+        tileCombined.setNormalizedViewId(getCanonicalViewId());
         docBroker->handleTileCombinedRequest(tileCombined, shared_from_this());
     }
     catch (const std::exception& exc)
@@ -1343,7 +1356,7 @@ void ClientSession::dumpState(std::ostream& os)
 void ClientSession::handleTileInvalidation(const std::string& message,
     const std::shared_ptr<DocumentBroker>& docBroker)
 {
-    docBroker->invalidateTiles(message, getHash());
+    docBroker->invalidateTiles(message, getCanonicalViewId());
 
     // Skip requesting new tiles if we don't have client visible area data yet.
     if(!_clientVisibleArea.hasSurface() ||
@@ -1368,7 +1381,7 @@ void ClientSession::handleTileInvalidation(const std::string& message,
     if( part == -1 ) // If no part is specifed we use the part used by the client
         part = _clientSelectedPart;
 
-    int normalizedViewId=getHash();
+    int normalizedViewId = getCanonicalViewId();
 
     std::vector<TileDesc> invalidTiles;
     if(part == _clientSelectedPart || _isTextDocument)
@@ -1401,7 +1414,7 @@ void ClientSession::handleTileInvalidation(const std::string& message,
     if(!invalidTiles.empty())
     {
         TileCombined tileCombined = TileCombined::create(invalidTiles);
-        tileCombined.setNormalizedViewId(getHash());
+        tileCombined.setNormalizedViewId(getCanonicalViewId());
         docBroker->handleTileCombinedRequest(tileCombined, shared_from_this());
     }
 }
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 5b3d98ace..2cd173f20 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -35,6 +35,8 @@ public:
                   const Poco::URI& uriPublic,
                   const bool isReadOnly = false);
 
+    void construct();
+
     virtual ~ClientSession();
 
     void handleIncomingMessage(SocketDisposition &) override;
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 6cebd7d25..75d9387ed 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -632,10 +632,7 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
     session->setUserName(username);
     session->setUserExtraInfo(userExtraInfo);
     session->setWatermarkText(watermarkText);
-    if(!watermarkText.empty())
-        session->setHash(watermarkText);
-    else
-        session->setHash(0);
+    session->recalcCanonicalViewId(_sessions);
 
     // Basic file information was stored by the above getWOPIFileInfo() or getLocalFileInfo() calls
     const StorageBase::FileInfo fileInfo = _storage->getFileInfo();
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 09a8ad5cc..439b33849 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -32,6 +32,7 @@
 #include "net/WebSocketHandler.hpp"
 
 #include "common/SigUtil.hpp"
+#include "common/Session.hpp"
 
 // Forwards.
 class PrisonerRequestDispatcher;
@@ -449,7 +450,7 @@ private:
     Poco::Timestamp _lastFileModifiedTime;
 
     /// All session of this DocBroker by ID.
-    std::map<std::string, std::shared_ptr<ClientSession> > _sessions;
+    SessionMap<ClientSession> _sessions;
 
     /// If we set the user-requested inital (on load) settings to be forced.
     std::set<std::string> _isInitialStateSet;
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 997e7d839..97b774cd0 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1760,7 +1760,10 @@ static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHand
         // In case of WOPI, if this session is not set as readonly, it might be set so
         // later after making a call to WOPI host which tells us the permission on files
         // (UserCanWrite param).
-        return std::make_shared<ClientSession>(id, docBroker, uriPublic, isReadOnly);
+        auto session = std::make_shared<ClientSession>(id, docBroker, uriPublic, isReadOnly);
+        session->construct();
+
+        return session;
     }
     catch (const std::exception& exc)
     {


More information about the Libreoffice-commits mailing list