[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/MasterProcessSession.cpp

[Ashod Nakashian]

 loolwsd/DocumentBroker.cpp       |    9 +++++++--
 loolwsd/DocumentBroker.hpp       |    2 +-
 loolwsd/MasterProcessSession.cpp |   29 +++++++++++++++++++++++------
 3 files changed, 31 insertions(+), 9 deletions(-)

New commits:
commit 909c996a6d2b052a395621be6de06ecc4cbdfba9
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Wed May 4 22:18:39 2016 -0400

    bccu#1774: poor API never fails to frustrate
    
    The tile and tilecombine messages apparently have optional
    appendages at their rear ends. Not one, but two (at least).
    
    While the first (timestamp) seems to be truely optional
    (in the sense that leaving it out doesn't break anything,)
    the same can't be said of the second (id).
    
    For Impress slides this id is used to identify the slide
    to which the tile belongs. Or rather the slide being
    rendered, as it seems meaningful only for the slide
    thumbnails.
    
    Previously the complete arguments of tile were copied
    verbatim from the input to the output (i.e. back to the
    client) and so any extra payload was also echoed back.
    
    But when id is missing (when expected) loleaflet not
    only fails to show these tiles (understandably,) but
    it also fails to show the scrollbar for said slide
    thumbnails altogether!
    
    With the new logic to move the tile communication to
    the child socket instead of the clients, the arguments
    are parsed and then serialized back in the response.
    So all fields must be explicitly known in advance.
    
    This change is necessary because tilecombine is broken
    to tile commands and so both share common code. This
    means that echoing back the request verbatim will
    break loleaflet since tilecombine arguments (which
    is a list) is not a valid response (which has the
    format of tile). So the internal representation
    has to be something neutral/common.
    
    The fix is to parse the timestamp and id only when
    provided and to echo back the id only in that case.
    
    Change-Id: Ic97cf0de4083d412a4b3543e8b9a8713ac27a27c
    Reviewed-on: https://gerrit.libreoffice.org/24669
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 040e7b0..612becd 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -390,12 +390,12 @@ bool DocumentBroker::handleInput(const std::vector<char>& payload)
 }
 
 void DocumentBroker::handleTileRequest(int part, int width, int height, int tilePosX,
-                                       int tilePosY, int tileWidth, int tileHeight,
+                                       int tilePosY, int tileWidth, int tileHeight, int id,
                                        const std::shared_ptr<MasterProcessSession>& session)
 {
     Log::trace() << "Tile request for part: " << part << ", width: " << width << ", height: " << height
                  << ", tilePosX: " << tilePosX << ", tilePosY: " << tilePosY << ", tileWidth: " << tileWidth
-                 << ", tileHeight: " << tileHeight << Log::end;
+                 << ", tileHeight: " << tileHeight << ", id: " << id << Log::end;
 
     std::ostringstream oss;
     oss << " part=" << part
@@ -405,6 +405,11 @@ void DocumentBroker::handleTileRequest(int part, int width, int height, int tile
         << " tileposy=" << tilePosY
         << " tilewidth=" << tileWidth
         << " tileheight=" << tileHeight;
+    if (id >= 0)
+    {
+        oss << " id=" << id;
+    }
+
     std::string tileMsg = oss.str();
 
     std::unique_lock<std::mutex> lock(_mutex);
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index bf8df26..ca22419 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -198,7 +198,7 @@ public:
     size_t removeSession(const std::string& id);
 
     void handleTileRequest(int part, int width, int height, int tilePosX,
-                           int tilePosY, int tileWidth, int tileHeight,
+                           int tilePosY, int tileWidth, int tileHeight, int id,
                            const std::shared_ptr<MasterProcessSession>& session);
 
     void handleTileResponse(const std::vector<char>& payload);
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index 28c7ada..733b43b 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -491,7 +491,7 @@ void MasterProcessSession::sendFontRendering(const char *buffer, int length, Str
 
 void MasterProcessSession::sendTile(const char * /*buffer*/, int /*length*/, StringTokenizer& tokens)
 {
-    int part, width, height, tilePosX, tilePosY, tileWidth, tileHeight;
+    int part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, id = -1;
     if (tokens.count() < 8 ||
         !getTokenInteger(tokens[1], "part", part) ||
         !getTokenInteger(tokens[2], "width", width) ||
@@ -517,13 +517,20 @@ void MasterProcessSession::sendTile(const char * /*buffer*/, int /*length*/, Str
         return;
     }
 
+    size_t index = 8;
+    if (tokens.count() > index && tokens[index].find("id") == 0)
+    {
+        getTokenInteger(tokens[index], "id", id);
+        ++index;
+    }
+
     _docBroker->handleTileRequest(part, width, height, tilePosX, tilePosY,
-                                  tileWidth, tileHeight, shared_from_this());
+                                  tileWidth, tileHeight, id, shared_from_this());
 }
 
 void MasterProcessSession::sendCombinedTiles(const char* /*buffer*/, int /*length*/, StringTokenizer& tokens)
 {
-    int part, pixelWidth, pixelHeight, tileWidth, tileHeight;
+    int part, pixelWidth, pixelHeight, tileWidth, tileHeight, id = -1;
     std::string tilePositionsX, tilePositionsY;
     if (tokens.count() < 8 ||
         !getTokenInteger(tokens[1], "part", part) ||
@@ -547,8 +554,18 @@ void MasterProcessSession::sendCombinedTiles(const char* /*buffer*/, int /*lengt
     }
 
     std::string reqTimestamp;
-    if (tokens.count() > 8)
-        getTokenString(tokens[8], "timestamp", reqTimestamp);
+    size_t index = 8;
+    if (tokens.count() > index && tokens[index].find("timestamp") == 0)
+    {
+        getTokenString(tokens[index], "timestamp", reqTimestamp);
+        ++index;
+    }
+
+    if (tokens.count() > index && tokens[index].find("id") == 0)
+    {
+        getTokenInteger(tokens[index], "id", id);
+        ++index;
+    }
 
     StringTokenizer positionXtokens(tilePositionsX, ",", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
     StringTokenizer positionYtokens(tilePositionsY, ",", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
@@ -579,7 +596,7 @@ void MasterProcessSession::sendCombinedTiles(const char* /*buffer*/, int /*lengt
         }
 
         _docBroker->handleTileRequest(part, pixelWidth, pixelHeight, x, y,
-                                      tileWidth, tileHeight, shared_from_this());
+                                      tileWidth, tileHeight, id, shared_from_this());
     }
 }