[Libreoffice-commits] online.git: loolwsd/ChildSession.cpp loolwsd/ClientSession.cpp loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/test loolwsd/TileCache.cpp loolwsd/TileCache.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Wed Sep 21 05:13:36 UTC 2016


 loolwsd/ChildSession.cpp        |    6 ------
 loolwsd/ClientSession.cpp       |    6 ++----
 loolwsd/DocumentBroker.cpp      |    7 +++++++
 loolwsd/DocumentBroker.hpp      |    2 ++
 loolwsd/TileCache.cpp           |   22 ++++++++++++++++++++++
 loolwsd/TileCache.hpp           |    3 +++
 loolwsd/test/TileCacheTests.cpp |   14 ++++++++++++++
 loolwsd/test/helpers.hpp        |    9 +++++++++
 8 files changed, 59 insertions(+), 10 deletions(-)

New commits:
commit 8f213bc170df4a4fc8112739ae5d678399035f66
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Aug 30 23:15:44 2016 -0400

    loolwsd: canceltiles re-designed using tilecache instead of queue
    
    Reviewed-on: https://gerrit.libreoffice.org/28526
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>
    (cherry picked from commit 571ff06906960c9840cd65a35914ca0607f72a11)
    
    Change-Id: Ie8f2c87a705aac14dd6f139c384f000df98db909
    Reviewed-on: https://gerrit.libreoffice.org/29117
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index a8a1a26..e054ce8 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -117,12 +117,6 @@ bool ChildSession::_handleInput(const char *buffer, int length)
         // Just to update the activity of a view-only client.
         return true;
     }
-    else if (tokens[0] == "canceltiles")
-    {
-        // This command makes sense only on the command queue level.
-        // Shouldn't get this here.
-        return true;
-    }
     else if (tokens[0] == "commandvalues")
     {
         return getCommandValues(buffer, length, tokens);
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index 6465519..43d712b 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -139,10 +139,8 @@ bool ClientSession::_handleInput(const char *buffer, int length)
     }
     else if (tokens[0] == "canceltiles")
     {
-        if (!_peer.expired())
-        {
-            return forwardToPeer(_peer, buffer, length, false);
-        }
+        _docBroker->cancelTileRequests(shared_from_this());
+        return true;
     }
     else if (tokens[0] == "commandvalues")
     {
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 6a1c037..a54483c 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -565,6 +565,13 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
     }
 }
 
+void DocumentBroker::cancelTileRequests(const std::shared_ptr<ClientSession>& session)
+{
+    std::unique_lock<std::mutex> lock(_mutex);
+
+    tileCache().cancelTiles(session);
+}
+
 void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
 {
     const std::string firstLine = getFirstLine(payload);
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index c8a46b7..7e7bc97 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -215,6 +215,8 @@ public:
     void handleTileCombinedRequest(TileCombined& tileCombined,
                                    const std::shared_ptr<ClientSession>& session);
 
+    void cancelTileRequests(const std::shared_ptr<ClientSession>& session);
+
     void handleTileResponse(const std::vector<char>& payload);
     void handleTileCombinedResponse(const std::vector<char>& payload);
 
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index ba20c8d..1ad4907 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -475,4 +475,26 @@ int TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared_
     }
 }
 
+void TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscriber)
+{
+    std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
+
+    const auto sub = subscriber.get();
+
+    Log::trace("Cancelling tiles for " + subscriber->getName());
+
+    for (auto it = _tilesBeingRendered.begin(); it != _tilesBeingRendered.end(); )
+    {
+        auto& subscribers = it->second->_subscribers;
+        Log::trace("Tile " + it->first + " has " + std::to_string(subscribers.size()) + " subscribers.");
+        subscribers.erase(std::remove_if(subscribers.begin(), subscribers.end(),
+                                         [sub](std::weak_ptr<ClientSession>& ptr){ return ptr.lock().get() == sub; }),
+                          subscribers.end());
+        Log::trace(" Tile " + it->first + " has " + std::to_string(subscribers.size()) + " subscribers.");
+
+        // Remove if there are no more subscribers on this tile.
+        it = (subscribers.empty() ? _tilesBeingRendered.erase(it) : ++it);
+    }
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/TileCache.hpp b/loolwsd/TileCache.hpp
index 08b14e0..2801e22 100644
--- a/loolwsd/TileCache.hpp
+++ b/loolwsd/TileCache.hpp
@@ -42,6 +42,9 @@ public:
     /// Otherwise returns 0 to signify a subscription exists.
     int subscribeToTileRendering(const TileDesc& tile, const std::shared_ptr<ClientSession> &subscriber);
 
+    /// Cancels all tile requests by the given subscriber.
+    void cancelTiles(const std::shared_ptr<ClientSession> &subscriber);
+
     std::unique_ptr<std::fstream> lookupTile(const TileDesc& tile);
 
     void saveTileAndNotify(const TileDesc& tile, const char *data, const size_t size);
diff --git a/loolwsd/test/TileCacheTests.cpp b/loolwsd/test/TileCacheTests.cpp
index f9ff937..2b172b0 100644
--- a/loolwsd/test/TileCacheTests.cpp
+++ b/loolwsd/test/TileCacheTests.cpp
@@ -52,6 +52,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testSimple);
     CPPUNIT_TEST(testSimpleCombine);
     CPPUNIT_TEST(testPerformance);
+    CPPUNIT_TEST(testCancelTiles);
     CPPUNIT_TEST(testUnresponsiveClient);
     CPPUNIT_TEST(testImpressTiles);
     CPPUNIT_TEST(testClientPartImpress);
@@ -70,6 +71,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
     void testSimple();
     void testSimpleCombine();
     void testPerformance();
+    void testCancelTiles();
     void testUnresponsiveClient();
     void testImpressTiles();
     void testClientPartImpress();
@@ -233,6 +235,18 @@ void TileCacheTests::testPerformance()
     socket.shutdown();
 }
 
+void TileCacheTests::testCancelTiles()
+{
+    const auto testName = "cancelTiles ";
+    auto socket = *loadDocAndGetSocket("load12.ods", _uri, testName);
+
+    // Request a huge tile, and cancel immediately.
+    sendTextFrame(socket, "tilecombine part=0 width=2560 height=2560 tileposx=0 tileposy=0 tilewidth=38400 tileheight=38400");
+    sendTextFrame(socket, "canceltiles");
+
+    assertNotInResponse(socket, "tile:", testName);
+}
+
 void TileCacheTests::testUnresponsiveClient()
 {
     const std::string docFilename = "hello.odt";
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 2af9070..410b6ea 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -335,6 +335,15 @@ std::string assertResponseLine(T& ws, const std::string& prefix, const std::stri
     return res;
 }
 
+/// Assert that we don't get a response with the given prefix.
+template <typename T>
+std::string assertNotInResponse(T& ws, const std::string& prefix, const std::string name = "")
+{
+    const auto res = getResponseLine(ws, prefix, name);
+    CPPUNIT_ASSERT_MESSAGE("Did not expect getting message [" + res + "].", res.empty());
+    return res;
+}
+
 inline
 void getResponseMessage(const std::shared_ptr<Poco::Net::WebSocket>& ws, const std::string& prefix, std::string& response, const bool isLine, const std::string& name = "")
 {


More information about the Libreoffice-commits mailing list