[Libreoffice-commits] online.git: common/Common.hpp common/IoUtil.cpp common/LOOLWebSocket.hpp kit/Kit.cpp net/WebSocketHandler.hpp test/helpers.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Mon Apr 3 05:18:30 UTC 2017


 common/Common.hpp        |    6 ------
 common/IoUtil.cpp        |   21 ---------------------
 common/LOOLWebSocket.hpp |   18 +-----------------
 kit/Kit.cpp              |    8 +++++---
 net/WebSocketHandler.hpp |   26 +++-----------------------
 test/helpers.hpp         |   18 +-----------------
 wsd/DocumentBroker.cpp   |    4 ++--
 wsd/DocumentBroker.hpp   |    3 ---
 8 files changed, 12 insertions(+), 92 deletions(-)

New commits:
commit 95d51493aa6f480948f07354f079e1b8d556108c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Mon Apr 3 00:45:40 2017 -0400

    wsd: remove nextmessage
    
    This was a workaround to Poco's limitation
    of requiring socket receiveFrame be given
    preallocated buffer, which couldn't be
    exceeded by a larger payload. This meant
    the receiver had to know the maximum
    payload in advance.
    
    Since only the Kit uses Poco sockets,
    and the Kit never receives large payloads,
    this preamble is now obsolete.
    
    100% (94/94) of old-style tests PASS.
    
    Change-Id: I76776f89497409e5755e335a3e25553e91cf0876
    Reviewed-on: https://gerrit.libreoffice.org/36037
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/common/Common.hpp b/common/Common.hpp
index ab4e1bdd..09bafad3 100644
--- a/common/Common.hpp
+++ b/common/Common.hpp
@@ -25,12 +25,6 @@ constexpr int WS_SEND_TIMEOUT_MS = 1000;
 /// which can be 1500 bytes long.
 constexpr long READ_BUFFER_SIZE = 64 * 1024;
 
-/// Size beyond which messages will be sent preceded with
-/// 'nextmessage' frame to let the receiver know in advance
-/// the size of the larger coming message. All messages up to,
-/// but not including, this size are considered small messages.
-constexpr int LARGE_MESSAGE_SIZE = READ_BUFFER_SIZE - 512;
-
 /// Message larger than this will be dropped as invalid
 /// or as intentionally flooding the server.
 constexpr int MAX_MESSAGE_SIZE = 2 * 1024 * READ_BUFFER_SIZE;
diff --git a/common/IoUtil.cpp b/common/IoUtil.cpp
index f94d0b3c..da9a4242 100644
--- a/common/IoUtil.cpp
+++ b/common/IoUtil.cpp
@@ -143,27 +143,6 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
                     }
                 }
             }
-            else
-            {
-                int size = 0;
-                Poco::StringTokenizer tokens(firstLine, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
-                // Check if it is a "nextmessage:" and in that case read the large
-                // follow-up message separately, and handle that only.
-                if (tokens.count() == 2 && tokens[0] == "nextmessage:" &&
-                    LOOLProtocol::getTokenInteger(tokens[1], "size", size) && size > 0)
-                {
-                    LOG_TRC("SocketProcessor [" << name << "]: Getting large message of " << size << " bytes.");
-                    if (size > MAX_MESSAGE_SIZE)
-                    {
-                        LOG_ERR("SocketProcessor [" << name << "]: Large-message size (" << size << ") over limit or invalid.");
-                    }
-                    else
-                    {
-                        payload.resize(size);
-                        continue;
-                    }
-                }
-            }
 
             LOG_CHECK(n > 0);
 
diff --git a/common/LOOLWebSocket.hpp b/common/LOOLWebSocket.hpp
index 24bc7a0e..784a0915 100644
--- a/common/LOOLWebSocket.hpp
+++ b/common/LOOLWebSocket.hpp
@@ -161,23 +161,7 @@ public:
         static const Poco::Timespan waitZero(0);
         std::unique_lock<std::mutex> lock(_mutexWrite);
 
-        if (length >= LARGE_MESSAGE_SIZE)
-        {
-            const std::string nextmessage = "nextmessage: size=" + std::to_string(length);
-            const int size = nextmessage.size();
-
-            if (Poco::Net::WebSocket::sendFrame(nextmessage.data(), size) == size)
-            {
-                LOG_TRC("Sent long message preample: " + nextmessage);
-            }
-            else
-            {
-                LOG_WRN("Failed to send long message preample.");
-                return -1;
-            }
-        }
-
-        int result = Poco::Net::WebSocket::sendFrame(buffer, length, flags);
+        const int result = Poco::Net::WebSocket::sendFrame(buffer, length, flags);
 
         lock.unlock();
 
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 35a48673..40a9a2df 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -659,7 +659,7 @@ public:
             return;
         }
 
-        LOG_TRC("Sending render-tile response (" + std::to_string(output.size()) + " bytes) for: " + response);
+        LOG_TRC("Sending render-tile response (" << output.size() << " bytes) for: " << response);
         ws->sendFrame(output.data(), output.size(), WebSocket::FRAME_BINARY);
     }
 
@@ -741,7 +741,8 @@ public:
             if (hash != 0 && tiles[tileIndex].getOldHash() == hash)
             {
                 // The tile content is identical to what the client already has, so skip it
-                LOG_TRC("Match for tile #" << tileIndex << " at (" << positionX << "," << positionY << ") oldhash==hash (" << hash << "), skipping");
+                LOG_TRC("Match for tile #" << tileIndex << " at (" << positionX << "," <<
+                        positionY << ") oldhash==hash (" << hash << "), skipping");
                 tiles.erase(tiles.begin() + tileIndex);
                 continue;
             }
@@ -756,7 +757,8 @@ public:
             }
 
             const auto imgSize = output.size() - oldSize;
-            LOG_TRC("Encoded tile #" << tileIndex << " at (" << positionX << "," << positionY << ") with oldhash=" << tiles[tileIndex].getOldHash() << ", hash=" << hash << " in " << imgSize << " bytes.");
+            LOG_TRC("Encoded tile #" << tileIndex << " at (" << positionX << "," << positionY << ") with oldhash=" <<
+                    tiles[tileIndex].getOldHash() << ", hash=" << hash << " in " << imgSize << " bytes.");
             tiles[tileIndex].setHash(hash);
             tiles[tileIndex].setImgSize(imgSize);
             tileIndex++;
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 1c6caf46..0cf63a36 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -311,31 +311,11 @@ public:
     /// 0 for closed/invalid socket, and -1 for other errors.
     int sendMessage(const char* data, const size_t len, const WSOpCode code, const bool flush = true) const
     {
-        if (data == nullptr || len == 0)
-            return -1;
-
-        auto socket = _socket.lock();
-        if (socket == nullptr)
-            return -1; // no socket == error.
-
-        assert(socket->isCorrectThread(true));
-        std::vector<char>& out = socket->_outBuffer;
-
         //TODO: Support fragmented messages.
-        static const unsigned char fin = static_cast<unsigned char>(WSFrameMask::Fin);
-
-        // FIXME: need to support fragmented mesages, but for now send prefix message with size.
-        if (len >= LARGE_MESSAGE_SIZE)
-        {
-            const std::string nextmessage = "nextmessage: size=" + std::to_string(len);
-            const unsigned char size = (nextmessage.size() & 0xff);
-            out.push_back(static_cast<char>(fin | WSOpCode::Text));
-            out.push_back(size);
-            out.insert(out.end(), nextmessage.data(), nextmessage.data() + size);
-            socket->writeOutgoingData();
-        }
+        static const unsigned char Fin = static_cast<unsigned char>(WSFrameMask::Fin);
 
-        return sendFrame(socket, data, len, static_cast<unsigned char>(fin | code), flush);
+        auto socket = _socket.lock();
+        return sendFrame(socket, data, len, static_cast<unsigned char>(Fin | code), flush);
     }
 
 protected:
diff --git a/test/helpers.hpp b/test/helpers.hpp
index a950de9d..ce6aef98 100644
--- a/test/helpers.hpp
+++ b/test/helpers.hpp
@@ -222,7 +222,7 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
                     timedout = false;
                 }
 
-                response.resize(READ_BUFFER_SIZE);
+                response.resize(READ_BUFFER_SIZE * 8);
                 int bytes = ws.receiveFrame(response.data(), response.size(), flags);
                 response.resize(std::max(bytes, 0));
                 std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
@@ -233,22 +233,6 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
                     {
                         return response;
                     }
-                    else if (LOOLProtocol::matchPrefix("nextmessage", message))
-                    {
-                        int size = 0;
-                        if (LOOLProtocol::getTokenIntegerFromMessage(message, "size", size) && size > 0)
-                        {
-                            response.resize(size);
-                            bytes = ws.receiveFrame(response.data(), response.size(), flags);
-                            response.resize(std::max(bytes, 0));
-                            std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
-                            if (bytes > 0 &&
-                                LOOLProtocol::matchPrefix(prefix, LOOLProtocol::getFirstLine(response)))
-                            {
-                                return response;
-                            }
-                        }
-                    }
                 }
                 else
                 {
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a89e3862..910f328f 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1077,7 +1077,7 @@ void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
         }
         else
         {
-            LOG_DBG("Render request declined for " << firstLine);
+            LOG_WRN("Dropping empty tile response: " << firstLine);
             // They will get re-issued if we don't forget them.
         }
     }
@@ -1111,7 +1111,7 @@ void DocumentBroker::handleTileCombinedResponse(const std::vector<char>& payload
         }
         else
         {
-            LOG_ERR("Render request declined for " << firstLine);
+            LOG_WRN("Dropping empty tilecombine response: " << firstLine);
             // They will get re-issued if we don't forget them.
         }
     }
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 60bf536d..91dfb63a 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -185,9 +185,6 @@ public:
     }
 
 private:
-    void socketProcessor();
-
-private:
     Poco::Process::PID _pid;
     std::shared_ptr<WebSocketHandler> _ws;
     std::shared_ptr<Socket> _socket;


More information about the Libreoffice-commits mailing list