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