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

[Ashod Nakashian]

 loolwsd/DocumentBroker.cpp |   29 ++++++++++-------------------
 loolwsd/TileCache.cpp      |    8 ++++----
 2 files changed, 14 insertions(+), 23 deletions(-)

New commits:
commit c8eb2ba6127f9d3da56811c25860c89ce085f588
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Oct 30 15:24:27 2016 -0400

    loolwsd: cleanup tile response handling
    
    Change-Id: I34b76b463c05e3479d1fd1516906c9a75282c753
    Reviewed-on: https://gerrit.libreoffice.org/30419
    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 185d610..7516cc5 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -740,29 +740,27 @@ void DocumentBroker::cancelTileRequests(const std::shared_ptr<ClientSession>& se
 void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
 {
     const std::string firstLine = getFirstLine(payload);
+    Log::debug("Handling tile combined: " + firstLine);
+
     try
     {
-        auto tile = TileDesc::parse(firstLine);
-        const auto buffer = payload.data();
         const auto length = payload.size();
-
         if (firstLine.size() < static_cast<std::string::size_type>(length) - 1)
         {
+            const auto tile = TileDesc::parse(firstLine);
+            const auto buffer = payload.data();
             const auto offset = firstLine.size() + 1;
             tileCache().saveTileAndNotify(tile, buffer + offset, length - offset);
         }
         else
         {
             Log::debug() << "Render request declined for " << firstLine << Log::end;
-            std::unique_lock<std::mutex> tileBeingRenderedLock(tileCache().getTilesBeingRenderedLock());
-            tileCache().forgetTileBeingRendered(tile);
+            // They will get re-issued if we don't forget them.
         }
     }
     catch (const std::exception& exc)
     {
         Log::error("Failed to process tile response [" + firstLine + "]: " + exc.what() + ".");
-        //FIXME: Return error.
-        //sendTextFrame("error: cmd=tile kind=syntax");
     }
 }
 
@@ -773,13 +771,12 @@ void DocumentBroker::handleTileCombinedResponse(const std::vector<char>& payload
 
     try
     {
-        auto tileCombined = TileCombined::parse(firstLine);
-        const auto buffer = payload.data();
         const auto length = payload.size();
-        auto offset = firstLine.size() + 1;
-
         if (firstLine.size() < static_cast<std::string::size_type>(length) - 1)
         {
+            const auto tileCombined = TileCombined::parse(firstLine);
+            const auto buffer = payload.data();
+            auto offset = firstLine.size() + 1;
             for (const auto& tile : tileCombined.getTiles())
             {
                 tileCache().saveTileAndNotify(tile, buffer + offset, tile.getImgSize());
@@ -788,19 +785,13 @@ void DocumentBroker::handleTileCombinedResponse(const std::vector<char>& payload
         }
         else
         {
-            Log::error() << "Render request failed for " << firstLine << Log::end;
-            std::unique_lock<std::mutex> tileBeingRenderedLock(tileCache().getTilesBeingRenderedLock());
-            for (const auto& tile : tileCombined.getTiles())
-            {
-                tileCache().forgetTileBeingRendered(tile);
-            }
+            Log::error() << "Render request declined for " << firstLine << Log::end;
+            // They will get re-issued if we don't forget them.
         }
     }
     catch (const std::exception& exc)
     {
         Log::error("Failed to process tile response [" + firstLine + "]: " + exc.what() + ".");
-        //FIXME: Return error.
-        //sendTextFrame("error: cmd=tile kind=syntax");
     }
 }
 
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index e8ebdd1..d26db3b 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -444,14 +444,14 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
 {
     assert(subscriber->getKind() == LOOLSession::Kind::ToClient);
 
-    std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
-
-    std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
-
     std::ostringstream oss;
     oss << '(' << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() << ')';
     const auto name = oss.str();
 
+    std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
+
+    std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
+
     if (tileBeingRendered)
     {
         for (const auto &s : tileBeingRendered->_subscribers)