test/httpwstest.cpp    |   70 -------------------------------------------------
 wsd/ClientSession.cpp  |    4 --
 wsd/ClientSession.hpp  |   10 -------
 wsd/DocumentBroker.cpp |   20 +++++++-------
 4 files changed, 12 insertions(+), 92 deletions(-)

New commits:
commit db3416c32f881e3edcffc670074874f1e074725a
Author: Jan Holesovsky <ke...@collabora.com>
Date:   Thu Apr 20 17:32:24 2017 +0200

    Revert "wsd: unittest for correctly saving in presence of passive clients"
    
    This reverts commit b0a21ae1b349a4eb52d93f1b63a4f8f10463b0f2.

diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 917a41dd..78d717fb 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -70,7 +70,6 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testReload);
     CPPUNIT_TEST(testGetTextSelection);
     CPPUNIT_TEST(testSaveOnDisconnect);
-    CPPUNIT_TEST(testSavePassiveOnDisconnect);
     CPPUNIT_TEST(testReloadWhileDisconnecting);
     CPPUNIT_TEST(testExcelLoad);
     CPPUNIT_TEST(testPaste);
@@ -124,7 +123,6 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     void testReload();
     void testGetTextSelection();
     void testSaveOnDisconnect();
-    void testSavePassiveOnDisconnect();
     void testReloadWhileDisconnecting();
     void testExcelLoad();
     void testPaste();
@@ -713,74 +711,6 @@ void HTTPWSTest::testSaveOnDisconnect()
     }
 }
 
-void HTTPWSTest::testSavePassiveOnDisconnect()
-{
-    const auto testname = "saveOnPassiveDisconnect ";
-
-    const auto text = helpers::genRandomString(40);
-    std::cerr << "Test string: [" << text << "]." << std::endl;
-
-    std::string documentPath, documentURL;
-    getDocumentPathAndURL("hello.odt", documentPath, documentURL, testname);
-
-    int kitcount = -1;
-    try
-    {
-        auto socket = loadDocAndGetSocket(_uri, documentURL, testname);
-
-        Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, 
documentURL);
-        auto socket2 = connectLOKit(_uri, request, _response);
-
-        sendTextFrame(socket, "uno .uno:SelectAll", testname);
-        sendTextFrame(socket, "uno .uno:Delete", testname);
-        sendTextFrame(socket, "paste mimetype=text/plain;charset=utf-8\n" + 
text, testname);
-
-        // Check if the document contains the pasted text.
-        sendTextFrame(socket, "uno .uno:SelectAll", testname);
-        sendTextFrame(socket, "gettextselection 
mimetype=text/plain;charset=utf-8", testname);
-        const auto selection = assertResponseString(socket, 
"textselectioncontent:", testname);
-        CPPUNIT_ASSERT_EQUAL("textselectioncontent: " + text, selection);
-
-        // Closing connection too fast might not flush buffers.
-        // Often nothing more than the SelectAll reaches the server before
-        // the socket is closed, when the doc is not even modified yet.
-        getResponseMessage(socket, "statechanged", testname);
-
-        kitcount = getLoolKitProcessCount();
-
-        // Shutdown abruptly.
-        std::cerr << "Closing connection after pasting." << std::endl;
-        socket->shutdown(); // Should trigger saving.
-        socket2->shutdown();
-    }
-    catch (const Poco::Exception& exc)
-    {
-        CPPUNIT_FAIL(exc.displayText());
-    }
-
-    // Allow time to save and destroy before we connect again.
-    testNoExtraLoolKitsLeft();
-    std::cerr << "Loading again." << std::endl;
-    try
-    {
-        // Load the same document and check that the last changes (pasted 
text) is saved.
-        auto socket = loadDocAndGetSocket(_uri, documentURL, testname);
-
-        // Should have no new instances.
-        CPPUNIT_ASSERT_EQUAL(kitcount, countLoolKitProcesses(kitcount));
-
-        // Check if the document contains the pasted text.
-        sendTextFrame(socket, "uno .uno:SelectAll", testname);
-        sendTextFrame(socket, "gettextselection 
mimetype=text/plain;charset=utf-8", testname);
-        const auto selection = assertResponseString(socket, 
"textselectioncontent:", testname);
-        CPPUNIT_ASSERT_EQUAL("textselectioncontent: " + text, selection);
-    }
-    catch (const Poco::Exception& exc)
-    {
-        CPPUNIT_FAIL(exc.displayText());
-    }
-}
-
 void HTTPWSTest::testReloadWhileDisconnecting()
 {
     const auto testname = "reloadWhileDisconnecting ";
commit 63398891b273b78042ed49c41d8439c8384cbcd4
Author: Jan Holesovsky <ke...@collabora.com>
Date:   Thu Apr 20 17:32:18 2017 +0200

    Revert "wsd: rely only on loaded sessions for saving"
    
    This reverts commit c1dbc3a117ef77557db10c98a9d5ca43bb2065f6.

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 8be60603..414f8b94 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -37,8 +37,7 @@ ClientSession::ClientSession(const std::string& id,
     _docBroker(docBroker),
     _uriPublic(uriPublic),
     _isDocumentOwner(false),
-    _isAttached(false),
-    _isLoaded(false)
+    _isAttached(false)
 {
     const size_t curConnections = ++LOOLWSD::NumConnections;
     LOG_INF("ClientSession ctor [" << getName() << "], current number of 
connections: " << curConnections);
@@ -632,7 +631,6 @@ bool ClientSession::handleKitToClientMessage(const char* 
buffer, const int lengt
         }
         else if (tokens[0] == "status:")
         {
-            setLoaded();
             docBroker->setLoaded();
 
             // Forward the status response to the client.
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 58ca0bb0..9cd67533 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -34,14 +34,10 @@ public:
 
     void setReadOnly() override;
 
-    /// Returns true if this session is added to a DocBroker.
+    /// Returns true if a document is loaded (i.e. we got status message).
     bool isAttached() const { return _isAttached; }
     void setAttached() { _isAttached = true; }
 
-    /// Returns true if this session has loaded a view (i.e. we got status 
message).
-    bool isLoaded() const { return _isLoaded; }
-    void setLoaded() { _isLoaded = true; }
-
     const std::string getUserId() const { return _userId; }
     void setUserId(const std::string& userId) { _userId = userId; }
     void setUserName(const std::string& userName) { _userName = userName; }
@@ -143,12 +139,8 @@ private:
     /// The socket to which the converted (saveas) doc is sent.
     std::shared_ptr<StreamSocket> _saveAsSocket;
 
-    /// If we are added to a DocBroker.
     bool _isAttached;
 
-    /// If we have loaded a view.
-    bool _isLoaded;
-
     /// Wopi FileInfo object
     std::unique_ptr<WopiStorage::WOPIFileInfo> _wopiFileInfo;
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 2af8f008..94a49a1d 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1218,7 +1218,6 @@ void DocumentBroker::destroyIfLastEditor(const 
std::string& id)
         for (const auto& it : _sessions)
         {
             if (it.second->getId() != id &&
-                it.second->isLoaded() &&
                 !it.second->isReadOnly())
             {
                 // Found another editable.
commit 44627abb5adfdc9b61e241791a0199d6dac1e223
Author: Jan Holesovsky <ke...@collabora.com>
Date:   Thu Apr 20 17:12:58 2017 +0200

    Revert "wsd: stop the DocBroker when saving session is last"
    
    This breaks explicit saving - the session gets locked.
    
    This reverts commit 2a0253b76be3e329af55d0ab80157544eea2253e.

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 0c60326d..2af8f008 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -562,13 +562,16 @@ bool DocumentBroker::saveToStorage(const std::string& 
sessionId,
 
     const bool res = saveToStorageInternal(sessionId, success, result);
 
-    // We've saved and can safely destroy this session.
-    removeSessionInternal(sessionId);
-
     // If marked to destroy, then this was the last session.
-    // Otherwise, check that we are (which we might be by now).
-    if (_markToDestroy || _sessions.empty())
+    // FIXME: If during that last save another client connects
+    // to this doc, the _markToDestroy will be reset and we
+    // will leak the last session. Need to mark the session as
+    // dead and cleanup somehow.
+    if (_markToDestroy)
     {
+        // We've saved and can safely destroy.
+        removeSessionInternal(sessionId);
+
         // Stop so we get cleaned up and removed.
         _stop = true;
     }
@@ -863,8 +866,7 @@ size_t DocumentBroker::removeSession(const std::string& id, 
bool destroyIfLast)
     try
     {
         LOG_INF("Removing session [" << id << "] on docKey [" << _docKey <<
-                "]. Have " << _sessions.size() << " sessions. markToDestroy: " 
<< _markToDestroy <<
-                ", LastEditableSession: " << _lastEditableSession);
+                "]. Have " << _sessions.size() << " sessions.");
 
         if (!_lastEditableSession || !autoSave(true))
             return removeSessionInternal(id);
@@ -1229,8 +1231,7 @@ void DocumentBroker::destroyIfLastEditor(const 
std::string& id)
     // Last view going away, can destroy.
     _markToDestroy = (_sessions.size() <= 1);
     LOG_DBG("startDestroy on session [" << id << "] on docKey [" << _docKey <<
-            "], sessions: " << _sessions.size() << " markToDestroy: " << 
_markToDestroy <<
-            ", lastEditableSession: " << _lastEditableSession);
+            "], markToDestroy: " << _markToDestroy << ", lastEditableSession: 
" << _lastEditableSession);
 }
 
 void DocumentBroker::setModified(const bool value)
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to