[Libreoffice-commits] online.git: wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/PrisonerSession.cpp wsd/PrisonerSession.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Mon Jan 23 05:54:06 UTC 2017


 wsd/ClientSession.cpp   |   21 ++++++---------------
 wsd/ClientSession.hpp   |    7 +++----
 wsd/DocumentBroker.cpp  |   13 +------------
 wsd/PrisonerSession.cpp |   31 ++++++++++++-------------------
 wsd/PrisonerSession.hpp |   10 ++++------
 5 files changed, 26 insertions(+), 56 deletions(-)

New commits:
commit 1f3d9ee457148fcc9eff3fbd3da66002010eb8bb
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sat Jan 21 20:31:22 2017 -0500

    wsd: ClientSession now encapsulates PrisonerSession
    
    No need to expose PrisonerSession via ClientSession
    when the marshalling of messages can be done by
    ClientSession directly.
    
    PrisonerSession can now be removed altogether.
    
    Change-Id: I131e41f5d9ae50be7244fb92a6f391a757502111
    Reviewed-on: https://gerrit.libreoffice.org/33431
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 3bc76de..63ca129 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -43,6 +43,7 @@ ClientSession::ClientSession(const std::string& id,
 {
     LOG_INF("ClientSession ctor [" << getName() << "].");
 
+    _peer = std::make_shared<PrisonerSession>(*this, docBroker);
     _senderThread = std::thread([this]{ senderThread(); });
 }
 
@@ -62,21 +63,6 @@ bool ClientSession::isLoaded() const
     return _isLoadRequested && _peer && _peer->gotStatus();
 }
 
-void ClientSession::bridgePrisonerSession()
-{
-    auto docBroker = getDocumentBroker();
-    if (docBroker)
-    {
-        _peer = std::make_shared<PrisonerSession>(shared_from_this(), docBroker);
-    }
-    else
-    {
-        const std::string msg = "No valid DocBroker while bridging Prisoner Session for " + getName();
-        LOG_ERR(msg);
-        throw std::runtime_error(msg);
-    }
-}
-
 bool ClientSession::_handleInput(const char *buffer, int length)
 {
     LOG_TRC(getName() << ": handling [" << getAbbreviatedMessage(buffer, length) << "].");
@@ -495,4 +481,9 @@ void ClientSession::senderThread()
     LOG_DBG(getName() << " SenderThread finished");
 }
 
+bool ClientSession::handleKitToClientMessage(const char* data, const int size)
+{
+    return _peer->handleInput(data, size);
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 5d97d70..b6211a7 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -37,16 +37,15 @@ public:
 
     bool isLoaded() const;
 
-    /// Create and connect Prisoner Session between DocumentBroker and us.
-    void bridgePrisonerSession();
-    std::shared_ptr<PrisonerSession> getPeer() const { return _peer; }
-
     const std::string getUserId() const { return _userId; }
     void setUserId(const std::string& userId) { _userId = userId; }
     void setUserName(const std::string& userName) { _userName = userName; }
     void setDocumentOwner(const bool documentOwner) { _isDocumentOwner = documentOwner; }
     bool isDocumentOwner() const { return _isDocumentOwner; }
 
+    /// Handle kit-to-client message.
+    bool handleKitToClientMessage(const char* data, const int size);
+
     using Session::sendTextFrame;
 
     bool sendBinaryFrame(const char* buffer, int length) override
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 1d699a2..38d13c1 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -651,9 +651,6 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
     const std::string aMessage = "session " + id + ' ' + _docKey;
     _childProcess->sendTextFrame(aMessage);
 
-    // Now we are ready to bridge between the kit and client.
-    session->bridgePrisonerSession();
-
     // Tell the admin console about this new doc
     Admin::instance().addDoc(_docKey, getPid(), getFilename(), id);
 
@@ -1032,15 +1029,7 @@ bool DocumentBroker::forwardToClient(const std::string& prefix, const std::vecto
         const auto it = _sessions.find(sid);
         if (it != _sessions.end())
         {
-            const auto peer = it->second->getPeer();
-            if (peer)
-            {
-                return peer->handleInput(data, size);
-            }
-            else
-            {
-                LOG_WRN("Client session [" << sid << "] has no peer to forward message: " << message);
-            }
+            return it->second->handleKitToClientMessage(data, size);
         }
         else
         {
diff --git a/wsd/PrisonerSession.cpp b/wsd/PrisonerSession.cpp
index d05a9a6..9a2b0cf 100644
--- a/wsd/PrisonerSession.cpp
+++ b/wsd/PrisonerSession.cpp
@@ -32,9 +32,9 @@ using namespace LOOLProtocol;
 using Poco::Path;
 using Poco::StringTokenizer;
 
-PrisonerSession::PrisonerSession(std::shared_ptr<ClientSession> clientSession,
+PrisonerSession::PrisonerSession(ClientSession& clientSession,
                                  std::shared_ptr<DocumentBroker> docBroker) :
-    Session("ToPrisoner-" + clientSession->getId(), clientSession->getId(), nullptr),
+    Session("ToPrisoner-" + clientSession.getId(), clientSession.getId(), nullptr),
     _docBroker(std::move(docBroker)),
     _peer(clientSession),
     _curPart(0),
@@ -56,12 +56,6 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
 
     LOOLWSD::dumpOutgoingTrace(_docBroker->getJailId(), getId(), firstLine);
 
-    auto peer = _peer.lock();
-    if (!peer)
-    {
-        throw Poco::ProtocolException("The session has not been assigned a peer.");
-    }
-
     if (tokens[0] == "unocommandresult:")
     {
         const std::string stringMsg(buffer, length);
@@ -111,7 +105,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
                     const auto payload = std::make_shared<Message>(buffer, length,
                                                                    Message::Dir::Out,
                                                                    Message::Type::Text);
-                    forwardToPeer(peer, payload);
+                    forwardToPeer(payload);
                     LOG_WRN("Document load failed: " << errorKind);
                     return false;
                 }
@@ -151,7 +145,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
             }
         }
 
-        peer->setSaveAsUrl(url);
+        _peer.setSaveAsUrl(url);
         return true;
     }
     else if (tokens.count() == 2 && tokens[0] == "statechanged:")
@@ -181,7 +175,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
             const auto payload = std::make_shared<Message>(buffer, length,
                                                            Message::Dir::Out,
                                                            Message::Type::Text);
-            return forwardToPeer(peer, payload);
+            return forwardToPeer(payload);
         }
         else if (tokens[0] == "commandvalues:")
         {
@@ -251,7 +245,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
             const auto payload = std::make_shared<Message>(buffer, length,
                                                            Message::Dir::Out,
                                                            Message::Type::Binary);
-            return forwardToPeer(peer, payload);
+            return forwardToPeer(payload);
         }
     }
     else
@@ -269,20 +263,19 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
                                                    Message::Dir::Out,
                                                    isBinary ? Message::Type::Binary
                                                             : Message::Type::Text);
-    return forwardToPeer(peer, payload);
+    return forwardToPeer(payload);
 }
 
-bool PrisonerSession::forwardToPeer(const std::shared_ptr<ClientSession>& clientSession,
-                                    const std::shared_ptr<Message>& payload)
+bool PrisonerSession::forwardToPeer(const std::shared_ptr<Message>& payload)
 {
     const auto& message = payload->abbr();
 
-    if (clientSession->isCloseFrame())
+    if (_peer.isCloseFrame())
     {
         LOG_TRC(getName() << ": peer began the closing handshake. Dropping forward message [" << message << "].");
         return true;
     }
-    else if (clientSession->isHeadless())
+    else if (_peer.isHeadless())
     {
         // Fail silently and return as there is no actual websocket
         // connection in this case.
@@ -290,8 +283,8 @@ bool PrisonerSession::forwardToPeer(const std::shared_ptr<ClientSession>& client
         return true;
     }
 
-    LOG_TRC(getName() << " -> " << clientSession->getName() << ": " << message);
-    clientSession->enqueueSendMessage(payload);
+    LOG_TRC(getName() << " -> " << _peer.getName() << ": " << message);
+    _peer.enqueueSendMessage(payload);
 
     return true;
 }
diff --git a/wsd/PrisonerSession.hpp b/wsd/PrisonerSession.hpp
index 12d607c..a79e51e 100644
--- a/wsd/PrisonerSession.hpp
+++ b/wsd/PrisonerSession.hpp
@@ -18,11 +18,10 @@ class ClientSession;
 /// Represents an internal session to a Kit process, in the WSD process.
 /// This doesn't really have a direct connection to any Kit process, rather
 /// all communication to said Kit process is really handled by DocumentBroker.
-class PrisonerSession final : public Session,
-                              public std::enable_shared_from_this<PrisonerSession>
+class PrisonerSession final : public Session
 {
 public:
-    PrisonerSession(std::shared_ptr<ClientSession> clientSession,
+    PrisonerSession(ClientSession& clientSession,
                     std::shared_ptr<DocumentBroker> docBroker);
 
     virtual ~PrisonerSession();
@@ -34,12 +33,11 @@ private:
     /// Handle messages from the Kit to the client.
     virtual bool _handleInput(const char* buffer, int length) override;
 
-    bool forwardToPeer(const std::shared_ptr<ClientSession>& clientSession,
-                       const std::shared_ptr<Message>& payload);
+    bool forwardToPeer(const std::shared_ptr<Message>& payload);
 
 private:
     std::shared_ptr<DocumentBroker> _docBroker;
-    std::weak_ptr<ClientSession> _peer;
+    ClientSession& _peer;
     int _curPart;
     bool _gotStatus;
 };


More information about the Libreoffice-commits mailing list