[Libreoffice-commits] online.git: 3 commits - loolwsd/MasterProcessSession.cpp loolwsd/test loolwsd/TileCache.cpp

[Tor Lillqvist]

 loolwsd/MasterProcessSession.cpp |   19 +++++++++++-----
 loolwsd/TileCache.cpp            |    3 ++
 loolwsd/test/httpwstest.cpp      |   45 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 6 deletions(-)

New commits:
commit 232499f542341168a8f5b3efb62610ffc1ec8223
Author: Tor Lillqvist <tml at collabora.com>
Date:   Thu Apr 21 12:34:30 2016 +0300

    Add test that simultaneously requested tiles indeed were rendered just once
    
    This test requires the renderid parameter to be present in the 'tile:'
    response messages, and that is the case only when ENABLE_DEBUG, so we
    can run the test only in a debug build.

diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index 59dfda4..f915d79 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -68,6 +68,9 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testPasswordProtectedDocumentWithCorrectPassword);
     CPPUNIT_TEST(testPasswordProtectedDocumentWithCorrectPasswordAgain);
     CPPUNIT_TEST(testImpressPartCountChanged);
+#if ENABLE_DEBUG
+    CPPUNIT_TEST(testSimultaneousTilesRenderedJustOnce);
+#endif
 
     // This should be the last test:
     CPPUNIT_TEST(testNoExtraLoolKitsLeft);
@@ -92,6 +95,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     void testPasswordProtectedDocumentWithCorrectPassword();
     void testPasswordProtectedDocumentWithCorrectPasswordAgain();
     void testImpressPartCountChanged();
+    void testSimultaneousTilesRenderedJustOnce();
     void testNoExtraLoolKitsLeft();
 
     void loadDoc(const std::string& documentURL);
@@ -995,6 +999,47 @@ void HTTPWSTest::testImpressPartCountChanged()
     }
 }
 
+void HTTPWSTest::testSimultaneousTilesRenderedJustOnce()
+{
+        const std::string documentPath = Util::getTempFilePath(TDOC, "hello.odt");
+        const std::string documentURL = "file://" + Poco::Path(documentPath).makeAbsolute().toString();
+
+        Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, documentURL);
+        Poco::Net::WebSocket socket1 = *connectLOKit(request, _response);
+        sendTextFrame(socket1, "load url=" + documentURL);
+
+        Poco::Net::WebSocket socket2 = *connectLOKit(request, _response);
+        sendTextFrame(socket2, "load url=" + documentURL);
+
+        sendTextFrame(socket1, "tile part=42 width=400 height=400 tileposx=1000 tileposy=2000 tilewidth=3000 tileheight=3000");
+        sendTextFrame(socket2, "tile part=42 width=400 height=400 tileposx=1000 tileposy=2000 tilewidth=3000 tileheight=3000");
+
+        std::string response1;
+        getResponseMessage(socket1, "tile:", response1, true);
+        CPPUNIT_ASSERT_MESSAGE("did not receive a tile: message as expected", !response1.empty());
+
+        std::string response2;
+        getResponseMessage(socket2, "tile:", response2, true);
+        CPPUNIT_ASSERT_MESSAGE("did not receive a tile: message as expected", !response2.empty());
+
+        if (!response1.empty() && !response2.empty())
+        {
+            Poco::StringTokenizer tokens1(response1, " ");
+            std::string renderId1;
+            LOOLProtocol::getTokenString(tokens1, "renderid", renderId1);
+            Poco::StringTokenizer tokens2(response2, " ");
+            std::string renderId2;
+            LOOLProtocol::getTokenString(tokens2, "renderid", renderId2);
+
+            CPPUNIT_ASSERT(renderId1 == renderId2 ||
+                           (renderId1 == "cached" && renderId2 != "cached") ||
+                           (renderId1 != "cached" && renderId2 == "cached"));
+        }
+
+        socket1.shutdown();
+        socket2.shutdown();
+}
+
 void HTTPWSTest::testNoExtraLoolKitsLeft()
 {
     int countNow = countLoolKitProcesses();
commit 95278b8643031e3c826a0bc712166d6dd3e741fc
Author: Tor Lillqvist <tml at collabora.com>
Date:   Thu Apr 21 12:23:57 2016 +0300

    Don't just re-use the 'tile:' response as a 'tile' request
    
    In a debug build it contains also the parameter renderid. (And in the
    future, might be extended in other ways, too.) Construct a new request
    message that has exactly and only the parameters we want.

diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index b7ae049..1fa0a03 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -196,14 +196,21 @@ bool MasterProcessSession::_handleInput(const char *buffer, int length)
                             Log::debug("Sending tile message also to subscriber " + subscriber->getName() + " line: '" + firstLine + "'");
                             std::shared_ptr<BasicTileQueue> queue;
                             queue = subscriber->getQueue();
-                            // re-emit the tile command in the other thread
-                            // to re-check and hit the cache. NB. it needs to be
-                            // 'tile' and not 'tile:'
+                            // Re-emit the tile command in the other thread(s) to re-check and hit
+                            // the cache. Construct the message from scratch to contain only the
+                            // mandatory parts of the message.
                             if (queue)
                             {
-                                std::string noColon = firstLine + "\n";
-                                noColon.erase(4,1);
-                                queue->put(noColon);
+                                const std::string message("tile "
+                                                          " part=" + std::to_string(part) +
+                                                          " width=" + std::to_string(width) +
+                                                          " height=" + std::to_string(height) +
+                                                          " tileposx=" + std::to_string(tilePosX) +
+                                                          " tileposy=" + std::to_string(tilePosY) +
+                                                          " tilewidth=" + std::to_string(tileWidth) +
+                                                          " tileheight=" + std::to_string(tileHeight) +
+                                                          "\n");
+                                queue->put(message);
                             }
                         }
                     }
commit d446822fb38524997b3e14ea4befb818913c2a5d
Author: Tor Lillqvist <tml at collabora.com>
Date:   Thu Apr 21 08:19:58 2016 +0300

    Add some more logging for tile cache management

diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index 534cc38..996d5f3 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -235,6 +235,7 @@ void TileCache::documentSaved()
     // first remove the invalidated tiles from the Persistent cache
     for (const auto& it : _toBeRemoved)
     {
+        Log::debug("Removing tile: " + _persCacheDir + "/" + it);
         Util::removeFile(_persCacheDir + "/" + it);
     }
 
@@ -246,6 +247,7 @@ void TileCache::documentSaved()
         std::unique_lock<std::mutex> lock(_cacheMutex);
         for (auto tileIterator = DirectoryIterator(_editCacheDir); tileIterator != DirectoryIterator(); ++tileIterator)
         {
+            Log::debug("Moving tile: " + tileIterator.path().toString() + " to " + _persCacheDir);
             tileIterator->moveTo(_persCacheDir);
         }
 
@@ -331,6 +333,7 @@ void TileCache::invalidateTiles(int part, int x, int y, int width, int height)
             const std::string fileName = tileIterator.path().getFileName();
             if (intersectsTile(fileName, part, x, y, width, height))
             {
+                Log::debug("Removing tile: " + tileIterator.path().toString());
                 Util::removeFile(tileIterator.path());
             }
         }