[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-1-9' - 91 commits - loleaflet/build loleaflet/dist loleaflet/main.js loleaflet/Makefile loleaflet/src loolwsd/AdminModel.cpp loolwsd/Auth.cpp loolwsd/bundled loolwsd/ChildSession.cpp loolwsd/ChildSession.hpp loolwsd/.clang-tidy loolwsd/ClientSession.cpp loolwsd/ClientSession.hpp loolwsd/configure.ac loolwsd/Connect.cpp loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/.gitignore loolwsd/IoUtil.cpp loolwsd/LibreOfficeKit.hpp loolwsd/Log.cpp loolwsd/LOOLForKit.cpp loolwsd/LOOLKit.cpp loolwsd/LOOLSession.cpp loolwsd/LOOLSession.hpp loolwsd/LOOLStress.cpp loolwsd/LOOLWSD.cpp loolwsd/loolwsd.xml.in loolwsd/Makefile.am loolwsd/MessageQueue.cpp loolwsd/MessageQueue.hpp loolwsd/PrisonerSession.cpp loolwsd/PrisonerSession.hpp loolwsd/protocol.txt loolwsd/Storage.cpp loolwsd/sysconfig.loolwsd loolwsd/test loolwsd/TileCache.cpp loolwsd/TileCache.hpp loolwsd/TileDesc.hpp loolwsd/TraceFile.hpp
Pranav Kant
pranavk at collabora.co.uk
Wed Sep 21 06:52:53 UTC 2016
loleaflet/Makefile | 1
loleaflet/build/deps.js | 3
loleaflet/dist/toolbar/toolbar.js | 60 --
loleaflet/main.js | 2
loleaflet/src/control/Control.DocumentRepair.js | 2
loleaflet/src/control/Permission.js | 1
loleaflet/src/core/Socket.js | 7
loleaflet/src/layer/Popup.js | 5
loleaflet/src/layer/tile/CalcTileLayer.js | 3
loleaflet/src/layer/tile/ImpressTileLayer.js | 3
loleaflet/src/layer/tile/TileLayer.js | 109 ++--
loleaflet/src/layer/tile/WriterTileLayer.js | 3
loleaflet/src/layer/vector/Path.Popup.js | 72 ++
loleaflet/src/layer/vector/SVG.js | 21
loleaflet/src/map/Map.js | 2
loolwsd/.clang-tidy | 3
loolwsd/.gitignore | 6
loolwsd/AdminModel.cpp | 10
loolwsd/Auth.cpp | 8
loolwsd/ChildSession.cpp | 116 +---
loolwsd/ChildSession.hpp | 13
loolwsd/ClientSession.cpp | 30 -
loolwsd/ClientSession.hpp | 19
loolwsd/Connect.cpp | 4
loolwsd/DocumentBroker.cpp | 206 +++----
loolwsd/DocumentBroker.hpp | 11
loolwsd/IoUtil.cpp | 4
loolwsd/LOOLForKit.cpp | 3
loolwsd/LOOLKit.cpp | 310 ++++++-----
loolwsd/LOOLSession.cpp | 7
loolwsd/LOOLSession.hpp | 6
loolwsd/LOOLStress.cpp | 100 ++-
loolwsd/LOOLWSD.cpp | 96 +--
loolwsd/LibreOfficeKit.hpp | 30 -
loolwsd/Log.cpp | 7
loolwsd/Makefile.am | 1
loolwsd/MessageQueue.cpp | 117 ++++
loolwsd/MessageQueue.hpp | 4
loolwsd/PrisonerSession.cpp | 12
loolwsd/PrisonerSession.hpp | 3
loolwsd/Storage.cpp | 12
loolwsd/TileCache.cpp | 125 +++-
loolwsd/TileCache.hpp | 5
loolwsd/TileDesc.hpp | 51 +
loolwsd/TraceFile.hpp | 7
loolwsd/bundled/include/LibreOfficeKit/LibreOfficeKit.h | 11
loolwsd/configure.ac | 18
loolwsd/loolwsd.xml.in | 12
loolwsd/protocol.txt | 36 -
loolwsd/sysconfig.loolwsd | 1
loolwsd/test/Makefile.am | 3
loolwsd/test/TileCacheTests.cpp | 221 +++++++-
loolwsd/test/countloolkits.hpp | 6
loolwsd/test/data/graphicviewselection.odp |binary
loolwsd/test/data/graphicviewselection.ods |binary
loolwsd/test/data/graphicviewselection.odt |binary
loolwsd/test/helpers.hpp | 125 ++--
loolwsd/test/httpwstest.cpp | 418 ++++------------
loolwsd/test/test.cpp | 11
59 files changed, 1322 insertions(+), 1160 deletions(-)
New commits:
commit debeb841235afbb7504666ccd111d3680bbd1dbd
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed Sep 21 11:43:09 2016 +0530
loleaflet: Cleanup internal view list after socket close
Change-Id: Ic18bc0f3efcd7cf68d5291305e4f0bcff9d48fdb
(cherry picked from commit 0991924a597861b93fde902acad49f64d2b55ecc)
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 58e1de8..edeac8d 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -309,6 +309,7 @@ L.Socket = L.Class.extend({
this._map._active = false;
}
+ this._docLayer.removeAllViews();
if (this.fail) {
this.fire('error', {msg: _('Well, this is embarrassing, we cannot connect to your document. Please try again.'), cmd: 'socket', kind: 'closed', id: 4});
}
diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index 44fc25d..5f099b6 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -728,6 +728,12 @@ L.TileLayer = L.GridLayer.extend({
this._map.removeView(viewId);
},
+ removeAllViews: function() {
+ for (var viewInfoIdx in this._map._viewInfo) {
+ this._removeView(parseInt(viewInfoIdx));
+ }
+ },
+
_onViewInfoMsg: function(textMsg) {
textMsg = textMsg.substring('viewinfo: '.length);
var viewInfo = JSON.parse(textMsg);
commit d4682a5fa9cf37f79f4cdc8ca30edc1882d5b6b5
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Tue Sep 20 22:08:14 2016 +0530
loleaflet: Handle new message, 'viewinfo:'
Change-Id: I82d886e3450439bbfd2e4b381cc8f9336bcdd57e
(cherry picked from commit 626eab255a23b985507c08c74558de7a67ce40c8)
diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index b9e0afb..44fc25d 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -378,14 +378,8 @@ L.TileLayer = L.GridLayer.extend({
else if (textMsg.startsWith('cellviewcursor:')) {
this._onCellViewCursorMsg(textMsg);
}
- else if (textMsg.startsWith('addview:')) {
- this._onAddViewMsg(textMsg);
- }
- else if (textMsg.startsWith('remview:')) {
- this._onRemViewMsg(textMsg);
- }
- else if (textMsg.startsWith('remallviews:')) {
- this._onRemAllViewMsg(textMsg);
+ else if (textMsg.startsWith('viewinfo:')) {
+ this._onViewInfoMsg(textMsg);
}
else if (textMsg.startsWith('textviewselection:')) {
this._onTextViewSelectionMsg(textMsg);
@@ -697,12 +691,7 @@ L.TileLayer = L.GridLayer.extend({
this._onUpdateViewCursor(viewId);
},
- _onAddViewMsg: function(textMsg) {
- textMsg = textMsg.substring('addview:'.length + 1);
- var obj = JSON.parse(textMsg);
- var viewId = parseInt(obj.id);
- var username = obj.username;
-
+ _addView: function(viewId, username) {
// Ignore if viewid is same as ours
if (viewId === this._viewId) {
return;
@@ -718,10 +707,7 @@ L.TileLayer = L.GridLayer.extend({
this._onUpdateViewCursor(viewId);
},
- _onRemViewMsg: function(textMsg) {
- textMsg = textMsg.substring('remview:'.length + 1);
- var viewId = parseInt(textMsg);
-
+ _removeView: function(viewId) {
// Couldn't be ours, now could it?!
if (viewId === this._viewId) {
return;
@@ -742,9 +728,24 @@ L.TileLayer = L.GridLayer.extend({
this._map.removeView(viewId);
},
- _onRemAllViewMsg: function(textMsg) {
- for (var viewId in this._map._viewInfo) {
- this._onRemViewMsg('remview: ' + viewId);
+ _onViewInfoMsg: function(textMsg) {
+ textMsg = textMsg.substring('viewinfo: '.length);
+ var viewInfo = JSON.parse(textMsg);
+
+ // A new view
+ var viewIds = [];
+ for (var viewInfoIdx in viewInfo) {
+ if (!(parseInt(viewInfo[viewInfoIdx].id) in this._map._viewInfo)) {
+ this._addView(viewInfo[viewInfoIdx].id, viewInfo[viewInfoIdx].username);
+ }
+ viewIds.push(viewInfo[viewInfoIdx].id);
+ }
+
+ // Check if any view is deleted
+ for (viewInfoIdx in this._map._viewInfo) {
+ if (viewIds.indexOf(parseInt(viewInfoIdx)) === -1) {
+ this._removeView(parseInt(viewInfoIdx));
+ }
}
},
commit fab971fde8298c026b3013b81f94e2b16b49ad07
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Tue Sep 20 15:59:53 2016 +0530
loolwsd: Always send the updated view info to clients
This replaces addview/remview/remallview messages in the protocol
with 'viewinfo' message which is sent whenever there is any
change in the view information.
Let the client deal with what information is redundant to it.
Change-Id: Ic470ea88a94ff281a0ae021014a9fba1b876f648
(cherry picked from commit 46107dd0c8973f48117d50111fe7a397320412c8)
diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index e054ce8..a915e0a 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -95,9 +95,8 @@ bool ChildSession::_handleInput(const char *buffer, int length)
_loKitDocument->setView(_viewId);
- // Refresh the viewIds.
- sendTextFrame("remallviews:");
- _docManager.notifyCurrentViewOfOtherViews(getId());
+ // Notify all views about updated view info
+ _docManager.notifyViewInfo();
const int curPart = _loKitDocument->getPart();
sendTextFrame("curpart: part=" + std::to_string(curPart));
@@ -313,8 +312,6 @@ bool ChildSession::loadDocument(const char * /*buffer*/, int /*length*/, StringT
viewInfoObj->stringify(ossViewInfo);
Log::info("Created new view with viewid: [" + viewId + "] for username: [" + _userName + "].");
- _docManager.notifyOtherSessions(getId(), "addview: " + ossViewInfo.str());
-
_docType = LOKitHelper::getDocumentTypeAsString(_loKitDocument->get());
if (_docType != "text" && part != -1)
{
@@ -330,8 +327,8 @@ bool ChildSession::loadDocument(const char * /*buffer*/, int /*length*/, StringT
return false;
}
- // Inform this view of other views
- _docManager.notifyCurrentViewOfOtherViews(getId());
+ // Inform everyone (including this one) about updated view info
+ _docManager.notifyViewInfo();
Log::info("Loaded session " + getId());
return true;
diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp
index 2528ef2..0ada61e 100644
--- a/loolwsd/ChildSession.hpp
+++ b/loolwsd/ChildSession.hpp
@@ -40,13 +40,9 @@ public:
virtual
void onUnload(const ChildSession& session) = 0;
- /// Send message to all other sessions except 'sessionId'
+ /// Send updated view info to all active sessions
virtual
- void notifyOtherSessions(const std::string& sessionId, const std::string& message) const = 0;
-
- /// Send other view's information to current view (one with sessionId)
- virtual
- void notifyCurrentViewOfOtherViews(const std::string& sessionId) const = 0;
+ void notifyViewInfo() = 0;
};
/// Represents the session to the WSD process, in a Kit process.
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 5eb7d42..0d7b1ff 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -71,6 +71,7 @@ typedef int (LokHookPreInit) (const char *install_path, const char *user_profil
using Poco::AutoPtr;
using Poco::Exception;
using Poco::File;
+using Poco::JSON::Array;
using Poco::JSON::Object;
using Poco::JSON::Parser;
using Poco::Net::HTTPClientSession;
@@ -981,8 +982,6 @@ private:
const auto& sessionId = session.getId();
Log::info("Unloading [" + sessionId + "].");
- // Broadcast the demise and removal of session.
- notifyOtherSessions(sessionId, "remview: " + std::to_string(session.getViewId()));
_tileQueue->removeCursorPosition(session.getViewId());
if (_loKitDocument == nullptr)
@@ -1005,60 +1004,71 @@ private:
_loKitDocument->destroyView(viewId);
_viewIdToCallbackDescr.erase(viewId);
Log::debug("Destroyed view " + std::to_string(viewId));
+ lock.unlock();
+
+ // Broadcast updated view info
+ notifyViewInfo();
}
- /// Notify all currently active sessions about session with given 'sessionId'
- void notifyOtherSessions(const std::string& sessionId, const std::string& message) const override
+ /// Notify all views of viewId and their associated usernames
+ void notifyViewInfo() override
{
- std::unique_lock<std::mutex> lock(_mutex);
+ std::unique_lock<std::mutex> lockLokDoc(_loKitDocument->getLock());
- for (auto& it: _connections)
+ // Get the list of view ids from the core
+ int viewCount = _loKitDocument->getViewsCount();
+ std::vector<int> viewIds(viewCount);
+ _loKitDocument->getViewIds(viewIds.data(), viewCount);
+ lockLokDoc.unlock();
+
+ std::unique_lock<std::mutex> lock(_mutex);
+ // Store the list of viewid, username mapping in a map
+ std::map<int, std::string> viewInfoMap;
+ for (auto& connectionIt : _connections)
{
- if (it.second->isRunning() && it.second->getSessionId() != sessionId)
+ if (connectionIt.second->isRunning())
{
- auto session = it.second->getSession();
- if (session && session->isActive())
- {
- session->sendTextFrame(message);
- }
+ const auto session = connectionIt.second->getSession();
+ const auto viewId = session->getViewId();
+ viewInfoMap[viewId] = session->getViewUserName();
}
}
- }
-
- /// Notify session (with given 'sessionId'), if active, of other existing views
- void notifyCurrentViewOfOtherViews(const std::string& sessionId) const override
- {
- std::unique_lock<std::mutex> lock(_mutex);
- const auto& it = _connections.find(Util::decodeId(sessionId));
- if (it == _connections.end() || !it->second)
+ // Double check if list of viewids from core and our list matches,
+ // and create an array of JSON objects containing id and username
+ Array::Ptr viewInfoArray = new Array();
+ int arrayIndex = 0;
+ for (auto& viewId: viewIds)
{
- Log::error("Cannot find current session [" + sessionId + "].");
- return;
- }
+ Object::Ptr viewInfoObj = new Object();
+ viewInfoObj->set("id", viewId);
- auto currentSession = it->second->getSession();
- if (!currentSession->isActive())
- {
- return;
+ if (viewInfoMap.find(viewId) == viewInfoMap.end())
+ {
+ Log::error("No username found for viewId [" + std::to_string(viewId) + "].");
+ viewInfoObj->set("username", "Unknown");
+ }
+ else
+ {
+ viewInfoObj->set("username", viewInfoMap[viewId]);
+ }
+
+ viewInfoArray->set(arrayIndex++, viewInfoObj);
}
+ std::ostringstream ossViewInfo;
+ viewInfoArray->stringify(ossViewInfo);
+
+ // Broadcast updated viewinfo to all _active_ connections
for (auto& connectionIt: _connections)
{
- if (connectionIt.second->isRunning() && connectionIt.second->getSessionId() != sessionId)
+ if (connectionIt.second->isRunning())
{
auto session = connectionIt.second->getSession();
- const auto viewId = session->getViewId();
- const auto viewUserName = session->getViewUserName();
-
- // Create a message object
- Object::Ptr viewInfoObj = new Object();
- viewInfoObj->set("id", viewId);
- viewInfoObj->set("username", viewUserName);
- std::ostringstream ossViewInfo;
- viewInfoObj->stringify(ossViewInfo);
-
- currentSession->sendTextFrame("addview: " + ossViewInfo.str());
+ if (session->isActive())
+ {
+ session->sendTextFrame("viewinfo: " + ossViewInfo.str());
+ }
}
}
}
diff --git a/loolwsd/LibreOfficeKit.hpp b/loolwsd/LibreOfficeKit.hpp
index 3abdf7f..c7319e7 100644
--- a/loolwsd/LibreOfficeKit.hpp
+++ b/loolwsd/LibreOfficeKit.hpp
@@ -433,6 +433,21 @@ public:
}
/**
+ * Returns the viewID for each existing view. Since viewIDs are not reused,
+ * viewIDs are not the same as the index of the view in the view array over
+ * time. Use getViewsCount() to know the minimal nSize that's large enough.
+ *
+ * @param pArray the array to write the viewIDs into
+ * @param nSize the size of pArray
+ * @returns true if pArray was large enough and result is written, false
+ * otherwise.
+ */
+ inline int getViewIds(int* pArray, size_t nSize)
+ {
+ return _pDoc->pClass->getViewIds(_pDoc, pArray, nSize);
+ }
+
+ /**
* Paints a font name to be displayed in the font list
* @param pFontName the font to be painted
*/
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index bf1a8ea..64bdf73 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -316,21 +316,11 @@ viewlock:
Per-view lock rectangle. JSON payload.
-addview: <JSON string>
+viewinfo: <payload>
- Eg: {"id": "<viewid>",
- "username": "<name of the user>"}
-
- New view with the given view information is created.
-
-remview: <viewId>
-
- The view with the given viewId has been destroyed.
-
-remallviews:
-
- Removes all views to send only current ones.
- The UI should still maintain its own view and cursor.
+ Message is sent everytime there is any change in view information.
+ <payload> consists of an array of JSON objects. Structure of JSON
+ objects is like : {"id": <viewid>, "username": <Full Name of the user>}
redlinetablechanged:
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index e1096da..e222e13 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -1010,22 +1010,20 @@ void HTTPWSTest::testInactiveClient()
{
const auto token = LOOLProtocol::getFirstToken(msg);
CPPUNIT_ASSERT_MESSAGE("unexpected message: " + msg,
- token == "addview:" ||
token == "cursorvisible:" ||
token == "graphicselection:" ||
token == "graphicviewselection:" ||
token == "invalidatecursor:" ||
token == "invalidatetiles:" ||
token == "invalidateviewcursor:" ||
- token == "remallviews:" ||
- token == "remview:" ||
token == "setpart:" ||
token == "statechanged:" ||
token == "textselection:" ||
token == "textselectionend:" ||
token == "textselectionstart:" ||
token == "textviewselection:" ||
- token == "viewcursorvisible:");
+ token == "viewcursorvisible:" ||
+ token == "viewinfo:");
// End when we get state changed.
return (token != "statechanged:");
commit 45611e1bcaef8306b973abfb5f125da3300e5cd3
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 22:29:32 2016 -0400
loolwsd: TileCacheTests cleanup
Change-Id: I88b84e9eb8d8b4e38354132c5875a26cacde9dca
Reviewed-on: https://gerrit.libreoffice.org/29128
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 5cbc1f10a428bd0a2f337ec7f22345290694a4c3)
diff --git a/loolwsd/test/TileCacheTests.cpp b/loolwsd/test/TileCacheTests.cpp
index 926da87..f2d7f3d 100644
--- a/loolwsd/test/TileCacheTests.cpp
+++ b/loolwsd/test/TileCacheTests.cpp
@@ -185,8 +185,11 @@ void TileCacheTests::testSimple()
void TileCacheTests::testSimpleCombine()
{
- const std::string docFilename = "hello.odt";
- auto socket1 = *loadDocAndGetSocket(docFilename, _uri, "simpleCombine-1 ");
+ std::string documentPath, documentURL;
+ getDocumentPathAndURL("hello.odt", documentPath, documentURL);
+
+ // First.
+ auto socket1 = loadDocAndGetSocket(_uri, documentURL, "simpleCombine-1 ");
sendTextFrame(socket1, "tilecombine part=0 width=256 height=256 tileposx=0,3840 tileposy=0,0 tilewidth=3840 tileheight=3840");
@@ -201,17 +204,16 @@ void TileCacheTests::testSimpleCombine()
tile1b = getResponseMessage(socket1, "tile:");
CPPUNIT_ASSERT_MESSAGE("did not receive a tile: message as expected", !tile1b.empty());
+ // Second.
std::cerr << "Connecting second client." << std::endl;
- auto socket2 = *loadDocAndGetSocket(docFilename, _uri, "simpleCombine-2 ", true);
+ auto socket2 = loadDocAndGetSocket(_uri, documentURL, "simpleCombine-2 ", true);
+
sendTextFrame(socket2, "tilecombine part=0 width=256 height=256 tileposx=0,3840 tileposy=0,0 tilewidth=3840 tileheight=3840");
auto tile2a = getResponseMessage(socket2, "tile:");
CPPUNIT_ASSERT_MESSAGE("did not receive a tile: message as expected", !tile2a.empty());
auto tile2b = getResponseMessage(socket2, "tile:");
CPPUNIT_ASSERT_MESSAGE("did not receive a tile: message as expected", !tile2b.empty());
-
- socket1.shutdown();
- socket2.shutdown();
}
void TileCacheTests::testPerformance()
@@ -273,12 +275,14 @@ void TileCacheTests::testCancelTilesMultiView()
void TileCacheTests::testUnresponsiveClient()
{
- const std::string docFilename = "hello.odt";
+ std::string documentPath, documentURL;
+ getDocumentPathAndURL("hello.odt", documentPath, documentURL);
+
std::cerr << "Connecting first client." << std::endl;
- auto socket1 = *loadDocAndGetSocket(docFilename, _uri, "unresponsiveClient-1 ");
+ auto socket1 = loadDocAndGetSocket(_uri, documentURL, "unresponsiveClient-1 ");
std::cerr << "Connecting second client." << std::endl;
- auto socket2 = *loadDocAndGetSocket(docFilename, _uri, "unresponsiveClient-2 ", true);
+ auto socket2 = loadDocAndGetSocket(_uri, documentURL, "unresponsiveClient-2 ");
// Pathologically request tiles and fail to read (say slow connection).
// Meanwhile, verify that others can get all tiles fine.
@@ -311,9 +315,6 @@ void TileCacheTests::testUnresponsiveClient()
CPPUNIT_ASSERT_MESSAGE("Did not receive tile #" + std::to_string(i+1) + " of 8: message as expected", !tile.empty());
}
}
-
- socket1.shutdown();
- socket2.shutdown();
}
void TileCacheTests::testImpressTiles()
@@ -371,12 +372,10 @@ void TileCacheTests::testSimultaneousTilesRenderedJustOnce()
std::string documentPath, documentURL;
getDocumentPathAndURL("hello.odt", documentPath, documentURL);
- Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, documentURL);
- Poco::Net::WebSocket socket1 = *connectLOKit(_uri, request, _response);
- sendTextFrame(socket1, "load url=" + documentURL);
-
- Poco::Net::WebSocket socket2 = *connectLOKit(_uri, request, _response);
- sendTextFrame(socket2, "load url=" + documentURL);
+ std::cerr << "Connecting first client." << std::endl;
+ auto socket1 = loadDocAndGetSocket(_uri, documentURL, "simultaneousTilesRenderdJustOnce-1 ");
+ std::cerr << "Connecting second client." << std::endl;
+ auto socket2 = loadDocAndGetSocket(_uri, documentURL, "simultaneousTilesRenderdJustOnce-2 ");
// Wait for the invalidatetile events to pass, otherwise they
// remove our tile subscription.
@@ -407,9 +406,6 @@ void TileCacheTests::testSimultaneousTilesRenderedJustOnce()
(renderId1 == "cached" && renderId2 != "cached") ||
(renderId1 != "cached" && renderId2 == "cached"));
}
-
- socket1.shutdown();
- socket2.shutdown();
}
void TileCacheTests::testLoad12ods()
@@ -749,24 +745,19 @@ void TileCacheTests::checkTiles(Poco::Net::WebSocket& socket, const std::string&
int docHeight = 0;
int docWidth = 0;
- std::string response;
- std::string text;
-
// check total slides 10
- sendTextFrame(socket, "status");
- getResponseMessage(socket, "status:", response, false);
- CPPUNIT_ASSERT_MESSAGE("did not receive a status: message as expected", !response.empty());
+ sendTextFrame(socket, "status", name);
+ const auto response = assertResponseLine(socket, "status:", name);
{
std::string line;
- std::istringstream istr(response);
+ std::istringstream istr(response.substr(8));
std::getline(istr, line);
- std::cout << "status: " << response << std::endl;
Poco::StringTokenizer tokens(line, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(6), tokens.count());
// Expected format is something like 'type= parts= current= width= height='.
- text = tokens[0].substr(type.size());
+ const auto text = tokens[0].substr(type.size());
totalParts = std::stoi(tokens[1].substr(parts.size()));
currentPart = std::stoi(tokens[2].substr(current.size()));
docWidth = std::stoi(tokens[3].substr(width.size()));
@@ -781,6 +772,7 @@ void TileCacheTests::checkTiles(Poco::Net::WebSocket& socket, const std::string&
if (docType == "presentation")
{
// request tiles
+ std::cerr << "Requesting Impress tiles." << std::endl;
requestTiles(socket, currentPart, docWidth, docHeight, name);
}
@@ -793,13 +785,12 @@ void TileCacheTests::checkTiles(Poco::Net::WebSocket& socket, const std::string&
if (currentPart != it)
{
// change part
- text = Poco::format("setclientpart part=%d", it);
- std::cout << text << std::endl;
- sendTextFrame(socket, text);
+ const auto text = Poco::format("setclientpart part=%d", it);
+ sendTextFrame(socket, text, name);
// Wait for the change to take effect otherwise we get invalidatetile
// which removes our next tile request subscription (expecting us to
// issue a new tile request as a response, which a real client would do).
- assertResponseLine(socket, "setpart:", "checkTiles");
+ assertResponseLine(socket, "setpart:", name);
requestTiles(socket, it, docWidth, docHeight, name);
}
@@ -848,9 +839,9 @@ void TileCacheTests::requestTiles(Poco::Net::WebSocket& socket, const int part,
tileX = tileSize * itCol;
tileY = tileSize * itRow;
text = Poco::format("tile part=%d width=%d height=%d tileposx=%d tileposy=%d tilewidth=%d tileheight=%d",
- part, pixTileSize, pixTileSize, tileX, tileY, tileWidth, tileHeight);
+ part, pixTileSize, pixTileSize, tileX, tileY, tileWidth, tileHeight);
- sendTextFrame(socket, text);
+ sendTextFrame(socket, text, name);
tile = assertResponseLine(socket, "tile:", name);
// expected tile: part= width= height= tileposx= tileposy= tilewidth= tileheight=
Poco::StringTokenizer tokens(tile, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
commit 20fc3dee833e41408424f1211826350822f05a85
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 22:28:35 2016 -0400
loolwsd: assume loading new view in unittests by default
...and prepare to removing view/non-view flags.
Change-Id: I464a71fa0e73abc577644170cb17309e5c8c252f
Reviewed-on: https://gerrit.libreoffice.org/29127
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 40f23677eec2ee5b12d3299412244df0091727c7)
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 415e098..ddb571f 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -104,7 +104,7 @@ void sendTextFrame(const std::shared_ptr<Poco::Net::WebSocket>& socket, const st
}
inline
-bool isDocumentLoaded(Poco::Net::WebSocket& ws, const std::string& name = "", bool isView = false)
+bool isDocumentLoaded(Poco::Net::WebSocket& ws, const std::string& name = "", bool isView = true)
{
bool isLoaded = false;
try
@@ -156,7 +156,7 @@ bool isDocumentLoaded(Poco::Net::WebSocket& ws, const std::string& name = "", bo
}
inline
-bool isDocumentLoaded(std::shared_ptr<Poco::Net::WebSocket>& ws, const std::string& name = "", bool isView = false)
+bool isDocumentLoaded(std::shared_ptr<Poco::Net::WebSocket>& ws, const std::string& name = "", bool isView = true)
{
return isDocumentLoaded(*ws, name, isView);
}
@@ -356,7 +356,7 @@ std::string assertResponseLine(T& ws, const std::string& prefix, const std::stri
template <typename T>
std::string assertNotInResponse(T& ws, const std::string& prefix, const std::string name = "")
{
- const auto res = getResponseLine(ws, prefix, name, 3000);
+ const auto res = getResponseLine(ws, prefix, name, 2000);
CPPUNIT_ASSERT_MESSAGE("Did not expect getting message [" + res + "].", res.empty());
return res;
}
@@ -395,7 +395,7 @@ connectLOKit(const Poco::URI& uri,
}
inline
-std::shared_ptr<Poco::Net::WebSocket> loadDocAndGetSocket(const Poco::URI& uri, const std::string& documentURL, const std::string& name = "", bool isView = false)
+std::shared_ptr<Poco::Net::WebSocket> loadDocAndGetSocket(const Poco::URI& uri, const std::string& documentURL, const std::string& name = "", bool isView = true)
{
try
{
@@ -420,7 +420,7 @@ std::shared_ptr<Poco::Net::WebSocket> loadDocAndGetSocket(const Poco::URI& uri,
}
inline
-std::shared_ptr<Poco::Net::WebSocket> loadDocAndGetSocket(const std::string& docFilename, const Poco::URI& uri, const std::string& name = "", bool isView = false)
+std::shared_ptr<Poco::Net::WebSocket> loadDocAndGetSocket(const std::string& docFilename, const Poco::URI& uri, const std::string& name = "", bool isView = true)
{
try
{
commit 8b96c028cb36f098409bb0d084108556eda8f40a
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 22:26:19 2016 -0400
loolwsd: always request newer tile version
And accept any version newer than expected.
Since we have proper de-duplication of tiles
requesting newer versions reduces changes of
races between client and the renderer.
Change-Id: I30bb53f98ef6f1461b53c1cf527d315dc35f7f26
Reviewed-on: https://gerrit.libreoffice.org/29125
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit ae4a9f7110f6d6677b3f3ef5d545f8021222d5da)
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 55ecbfa..289842c 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -510,14 +510,12 @@ void DocumentBroker::handleTileRequest(TileDesc& tile,
return;
}
- if (tileCache().subscribeToTileRendering(tile, session) > 0)
- {
- Log::debug() << "Sending render request for tile (" << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() << ")." << Log::end;
+ tileCache().subscribeToTileRendering(tile, session);
- // Forward to child to render.
- const std::string request = "tile " + tile.serialize();
- _childProcess->getWebSocket()->sendFrame(request.data(), request.size());
- }
+ // Forward to child to render.
+ Log::debug() << "Sending render request for tile (" << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() << ")." << Log::end;
+ const std::string request = "tile " + tile.serialize();
+ _childProcess->getWebSocket()->sendFrame(request.data(), request.size());
}
void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index eb8e8d8..416543e 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -195,7 +195,7 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
}
// Remove subscriptions.
- if (tileBeingRendered->getVersion() == tile.getVersion())
+ if (tileBeingRendered->getVersion() <= tile.getVersion())
{
Log::debug() << "STATISTICS: tile " << tile.getVersion() << " internal roundtrip "
<< tileBeingRendered->getElapsedTimeMs() << " ms." << Log::end;
commit 59170c3464ba28972aa57a42710d8333e5fbc1d4
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 22:19:52 2016 -0400
loolwsd: handle invalidatetiles in DocumentBroker
Change-Id: I05e70f82af9b5c8bdb590a64688ffa70c6ba2034
Reviewed-on: https://gerrit.libreoffice.org/29124
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 9640fd1e78475b00554b8f8972f8433128d95a1c)
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index a30b371..55ecbfa 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -463,6 +463,17 @@ bool DocumentBroker::handleInput(const std::vector<char>& payload)
return true;
}
+void DocumentBroker::invalidateTiles(const std::string& tiles)
+{
+ std::unique_lock<std::mutex> lock(_mutex);
+
+ // Remove from cache.
+ _tileCache->invalidateTiles(tiles);
+
+ //TODO: Re-issue the tiles again to avoid races.
+
+}
+
void DocumentBroker::handleTileRequest(TileDesc& tile,
const std::shared_ptr<ClientSession>& session)
{
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 7e7bc97..a547a32 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -210,6 +210,7 @@ public:
_cursorHeight = h;
}
+ void invalidateTiles(const std::string& tiles);
void handleTileRequest(TileDesc& tile,
const std::shared_ptr<ClientSession>& session);
void handleTileCombinedRequest(TileCombined& tileCombined,
diff --git a/loolwsd/PrisonerSession.cpp b/loolwsd/PrisonerSession.cpp
index 3844974..c2ea47f 100644
--- a/loolwsd/PrisonerSession.cpp
+++ b/loolwsd/PrisonerSession.cpp
@@ -216,7 +216,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
else if (tokens[0] == "invalidatetiles:")
{
assert(firstLine.size() == static_cast<std::string::size_type>(length));
- _docBroker->tileCache().invalidateTiles(firstLine);
+ _docBroker->invalidateTiles(firstLine);
}
else if (tokens[0] == "invalidatecursor:")
{
commit 4b5bf10c0eec6b41c0505ef03f30232fe2dc3cdf
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 21:40:28 2016 -0400
loolwsd: don't remove subscriptions on invalidation
Tile invalidation is a hint to the clients to request
fresh tiles and replace the existing ones. However
any outstanding tile request will be rendered anew.
So no need to remove those.
Nonetheless, we should issue new versions to avoid
race between old tile and invalidate. This can
happen when a tile rendered just before invalidate
reaches the client after the invalidate. The client
will think the tile is a new one when it was rendered
just before the invalidate.
Change-Id: Ieb2ffab1214dd904da8e532e7d9d20e6ad783b78
Reviewed-on: https://gerrit.libreoffice.org/29123
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 87a9f6e166e1cf487eb7497b4abcb65b30821b7a)
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index a439a63..eb8e8d8 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -202,6 +202,10 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
_tilesBeingRendered.erase(cachedName);
}
}
+ else
+ {
+ Log::debug("No subscribers for: " + cachedName);
+ }
}
std::string TileCache::getTextFile(const std::string& fileName)
@@ -312,21 +316,6 @@ void TileCache::invalidateTiles(int part, int x, int y, int width, int height)
}
}
}
-
- // Forget this tile as it will have to be rendered again.
- for (auto it = _tilesBeingRendered.begin(); it != _tilesBeingRendered.end(); )
- {
- const std::string cachedName = it->first;
- if (intersectsTile(cachedName, part, x, y, width, height))
- {
- Log::debug("Removing subscriptions for: " + cachedName);
- it = _tilesBeingRendered.erase(it);
- }
- else
- {
- ++it;
- }
- }
}
void TileCache::invalidateTiles(const std::string& tiles)
commit 438515e1d2572814e66e0fb67f65fd613e249bd3
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 20:44:08 2016 -0400
loolwsd: new multi-view canceltiles unittest
Change-Id: Ia7fdc2c64c96e3edeb82ef48d3621b70ca958b5d
Reviewed-on: https://gerrit.libreoffice.org/29122
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 14f11460b62ea5d05aad5bda05e11ae71dd47063)
diff --git a/loolwsd/test/TileCacheTests.cpp b/loolwsd/test/TileCacheTests.cpp
index 2b172b0..926da87 100644
--- a/loolwsd/test/TileCacheTests.cpp
+++ b/loolwsd/test/TileCacheTests.cpp
@@ -53,6 +53,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testSimpleCombine);
CPPUNIT_TEST(testPerformance);
CPPUNIT_TEST(testCancelTiles);
+ CPPUNIT_TEST(testCancelTilesMultiView);
CPPUNIT_TEST(testUnresponsiveClient);
CPPUNIT_TEST(testImpressTiles);
CPPUNIT_TEST(testClientPartImpress);
@@ -72,6 +73,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testSimpleCombine();
void testPerformance();
void testCancelTiles();
+ void testCancelTilesMultiView();
void testUnresponsiveClient();
void testImpressTiles();
void testClientPartImpress();
@@ -238,7 +240,7 @@ void TileCacheTests::testPerformance()
void TileCacheTests::testCancelTiles()
{
const auto testName = "cancelTiles ";
- auto socket = *loadDocAndGetSocket("load12.ods", _uri, testName);
+ auto socket = *loadDocAndGetSocket("setclientpart.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");
@@ -247,6 +249,28 @@ void TileCacheTests::testCancelTiles()
assertNotInResponse(socket, "tile:", testName);
}
+void TileCacheTests::testCancelTilesMultiView()
+{
+ std::string documentPath, documentURL;
+ getDocumentPathAndURL("setclientpart.ods", documentPath, documentURL);
+
+ auto socket1 = loadDocAndGetSocket(_uri, documentURL, "cancelTilesMultiView-1 ");
+ auto socket2 = loadDocAndGetSocket(_uri, documentURL, "cancelTilesMultiView-2 ", true);
+
+ sendTextFrame(socket1, "tilecombine part=0 width=256 height=256 tileposx=0,3840,7680,11520,0,3840,7680,11520 tileposy=0,0,0,0,3840,3840,3840,3840 tilewidth=3840 tileheight=3840", "cancelTilesMultiView-1 ");
+ sendTextFrame(socket2, "tilecombine part=0 width=256 height=256 tileposx=0,3840,7680,0 tileposy=0,0,0,22520 tilewidth=3840 tileheight=3840", "cancelTilesMultiView-2 ");
+
+ sendTextFrame(socket1, "canceltiles");
+
+ for (auto i = 0; i < 4; ++i)
+ {
+ getTileMessage(*socket2, "cancelTilesMultiView-2 ");
+ }
+
+ assertNotInResponse(socket1, "tile:", "cancelTilesMultiView-1 ");
+ assertNotInResponse(socket2, "tile:", "cancelTilesMultiView-2 ");
+}
+
void TileCacheTests::testUnresponsiveClient()
{
const std::string docFilename = "hello.odt";
commit e41379dbea4d0d74e44b706af3b42e5021e36b83
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 20:41:30 2016 -0400
loolwsd: better timeout handling in unittests
Change-Id: Ib9a002cb25eda12335727bef56f7e2d48d682438
Reviewed-on: https://gerrit.libreoffice.org/29121
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit d4a8177803d9693b6e4782d9cf20792a0b7e4ffa)
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 410b6ea..415e098 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -238,21 +238,28 @@ void getResponseMessage(Poco::Net::WebSocket& ws, const std::string& prefix, std
}
inline
-std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string& prefix, std::string name = "")
+std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string& prefix, std::string name = "", const size_t timeoutMs = 10000)
{
name = name + '[' + prefix + "] ";
try
{
int flags = 0;
- int retries = 20;
- static const Poco::Timespan waitTime(2000000);
+ int retries = timeoutMs / 500;
+ const Poco::Timespan waitTime(retries ? timeoutMs * 1000 / retries : timeoutMs * 1000);
std::vector<char> response;
+ bool timedout = false;
ws.setReceiveTimeout(0);
do
{
if (ws.poll(waitTime, Poco::Net::Socket::SELECT_READ))
{
+ if (timedout)
+ {
+ std::cerr << std::endl;
+ timedout = false;
+ }
+
response.resize(READ_BUFFER_SIZE);
int bytes = ws.receiveFrame(response.data(), response.size(), flags);
response.resize(bytes >= 0 ? bytes : 0);
@@ -290,19 +297,29 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string
std::cerr << name << "Got " << bytes << " bytes, flags: " << std::hex << flags << std::dec << std::endl;
}
- retries = 10;
if (bytes <= 0)
{
break;
}
if ((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
+ {
std::cerr << name << "Ignored: " << message << std::endl;
+ }
}
else
{
+ if (!timedout)
+ {
+ std::cerr << name << "Timeout " ;
+ }
+ else
+ {
+ std::cerr << retries << ' ';
+ }
+
--retries;
- std::cerr << name << "Timeout " << retries << std::endl;
+ timedout = true;
}
}
while (retries > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
@@ -316,15 +333,15 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string
}
inline
-std::vector<char> getResponseMessage(const std::shared_ptr<Poco::Net::WebSocket>& ws, const std::string& prefix, const std::string& name = "")
+std::vector<char> getResponseMessage(const std::shared_ptr<Poco::Net::WebSocket>& ws, const std::string& prefix, const std::string& name = "", const size_t timeoutMs = 10000)
{
- return getResponseMessage(*ws, prefix, name);
+ return getResponseMessage(*ws, prefix, name, timeoutMs);
}
template <typename T>
-std::string getResponseLine(T& ws, const std::string& prefix, const std::string name = "")
+std::string getResponseLine(T& ws, const std::string& prefix, const std::string name = "", const size_t timeoutMs = 10000)
{
- return LOOLProtocol::getFirstLine(getResponseMessage(ws, prefix, name));
+ return LOOLProtocol::getFirstLine(getResponseMessage(ws, prefix, name, timeoutMs));
}
template <typename T>
@@ -339,7 +356,7 @@ std::string assertResponseLine(T& ws, const std::string& prefix, const std::stri
template <typename T>
std::string assertNotInResponse(T& ws, const std::string& prefix, const std::string name = "")
{
- const auto res = getResponseLine(ws, prefix, name);
+ const auto res = getResponseLine(ws, prefix, name, 3000);
CPPUNIT_ASSERT_MESSAGE("Did not expect getting message [" + res + "].", res.empty());
return res;
}
commit d23ca06427cfaef9b77db6a23f70e471e2d99e0b
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 19:59:06 2016 -0400
loolwsd: cancel individual tiles
Change-Id: I18faee319fc12de2151460afbb054b8509578579
Reviewed-on: https://gerrit.libreoffice.org/29120
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 0326aa4304e866bf346aaa90697a49b43f0d9d74)
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 3b4e39b..5eb7d42 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -1547,7 +1547,7 @@ void lokit_main(const std::string& childRoot,
Log::debug("CreateSession failed.");
}
}
- else if (tokens[0] == "tile")
+ else if (tokens[0] == "tile" || tokens[0] == "tilecombine" || tokens[0] == "canceltiles")
{
if (document)
{
@@ -1555,18 +1555,7 @@ void lokit_main(const std::string& childRoot,
}
else
{
- Log::warn("No document while processing tile request.");
- }
- }
- else if (tokens[0] == "tilecombine")
- {
- if (document)
- {
- queue->put(message);
- }
- else
- {
- Log::warn("No document while processing tilecombine request.");
+ Log::warn("No document while processing " + tokens[0] + " request.");
}
}
else if (document && document->canDiscard())
@@ -1576,7 +1565,7 @@ void lokit_main(const std::string& childRoot,
}
else
{
- Log::info("Bad or unknown token [" + tokens[0] + "]");
+ Log::error("Bad or unknown token [" + tokens[0] + "]");
}
return true;
diff --git a/loolwsd/MessageQueue.cpp b/loolwsd/MessageQueue.cpp
index 7e81608..0a4c96d 100644
--- a/loolwsd/MessageQueue.cpp
+++ b/loolwsd/MessageQueue.cpp
@@ -11,9 +11,13 @@
#include <algorithm>
+#include <Poco/StringTokenizer.h>
+
#include <TileDesc.hpp>
#include <Log.hpp>
+using Poco::StringTokenizer;
+
MessageQueue::~MessageQueue()
{
clear();
@@ -102,6 +106,35 @@ void TileQueue::put_impl(const Payload& value)
{
const auto msg = std::string(value.data(), value.size());
Log::trace() << "Putting [" << msg << "]" << Log::end;
+
+ if (msg.compare(0, 11, "canceltiles") == 0)
+ {
+ Log::trace("Processing " + msg);
+ Log::trace() << "Before canceltiles have " << _queue.size() << " in queue." << Log::end;
+ const auto seqs = msg.substr(12);
+ StringTokenizer tokens(seqs, ",", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
+ _queue.erase(std::remove_if(_queue.begin(), _queue.end(),
+ [&tokens](const Payload& v)
+ {
+ const std::string s(v.data(), v.size());
+ for (size_t i = 0; i < tokens.count(); ++i)
+ {
+ if (s.find("ver=" + tokens[i]) != std::string::npos)
+ {
+ Log::trace("Matched " + tokens[i] + ", Removing [" + s + "]");
+ return true;
+ }
+ }
+
+ return false;
+
+ }), _queue.end());
+
+ // Don't push canceltiles into the queue.
+ Log::trace() << "After canceltiles have " << _queue.size() << " in queue." << Log::end;
+ return;
+ }
+
if (!_queue.empty())
{
if (msg.compare(0, 4, "tile") == 0 || msg.compare(0, 10, "tilecombine") == 0)
commit ebbe959b4027d2e75edab509ce4309a0beda35b9
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 19:20:23 2016 -0400
loolwsd: make sure to combine all contiguous tiles
Change-Id: If65e2ff53f593015ba7df23b6943a711c36550de
Reviewed-on: https://gerrit.libreoffice.org/29119
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 9e5df31cbb2b2aeeb5c4bdbf260307d0a027703e)
diff --git a/loolwsd/MessageQueue.cpp b/loolwsd/MessageQueue.cpp
index 16946e2..7e81608 100644
--- a/loolwsd/MessageQueue.cpp
+++ b/loolwsd/MessageQueue.cpp
@@ -214,35 +214,45 @@ MessageQueue::Payload TileQueue::get_impl()
tiles.emplace_back(TileDesc::parse(msg));
// Combine as many tiles as possible with the top one.
- for (size_t i = 0; i < _queue.size(); )
+ bool added;
+ do
{
- auto& it = _queue[i];
- msg = std::string(it.data(), it.size());
- if (msg.compare(0, 5, "tile ") != 0 ||
- msg.find("id=") != std::string::npos)
+ added = false;
+ for (size_t i = 0; i < _queue.size(); )
{
- // Don't combine non-tiles or tiles with id.
- continue;
- }
+ auto& it = _queue[i];
+ msg = std::string(it.data(), it.size());
+ if (msg.compare(0, 5, "tile ") != 0 ||
+ msg.find("id=") != std::string::npos)
+ {
+ // Don't combine non-tiles or tiles with id.
+ ++i;
+ continue;
+ }
- auto tile2 = TileDesc::parse(msg);
- bool found = false;
- Log::trace() << "combining?: " << msg << Log::end;
+ auto tile2 = TileDesc::parse(msg);
+ Log::trace() << "combining?: " << msg << Log::end;
- // Check if adjacent tiles.
- for (auto& tile : tiles)
- {
- if (tile.isAdjacent(tile2))
+ // Check if adjacent tiles.
+ bool found = false;
+ for (auto& tile : tiles)
{
- tiles.emplace_back(tile2);
- _queue.erase(_queue.begin() + i);
- found = true;
- break;
+ if (tile.isAdjacent(tile2))
+ {
+ tiles.emplace_back(tile2);
+ _queue.erase(_queue.begin() + i);
+ found = true;
+ added = true;
+ break;
+ }
}
- }
- i += !found;
+ i += !found;
+ }
}
+ while (added);
+
+ Log::trace() << "Combined " << tiles.size() << " tiles, leaving " << _queue.size() << " in queue." << Log::end;
if (tiles.size() == 1)
{
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index a5f7c0f..a439a63 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -477,11 +477,13 @@ int TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared_
std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscriber)
{
+ assert(subscriber && "cancelTiles expects valid subscriber");
+ Log::trace("Cancelling tiles for " + subscriber->getName());
+
std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
const auto sub = subscriber.get();
- Log::trace("Cancelling tiles for " + subscriber->getName());
std::ostringstream oss;
for (auto it = _tilesBeingRendered.begin(); it != _tilesBeingRendered.end(); )
commit 638419561fea1b07115daedb4183e057539b6071
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Sep 19 22:16:45 2016 -0400
loolwsd: cancel subscriber tiles only
Change-Id: I207f2332520f29308a2994769aa3a12ea5178477
Reviewed-on: https://gerrit.libreoffice.org/29118
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 6bf643183e4a93dcb85546f146c364b9f0621a83)
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index a54483c..a30b371 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -569,7 +569,12 @@ void DocumentBroker::cancelTileRequests(const std::shared_ptr<ClientSession>& se
{
std::unique_lock<std::mutex> lock(_mutex);
- tileCache().cancelTiles(session);
+ const auto canceltiles = tileCache().cancelTiles(session);
+ if (!canceltiles.empty())
+ {
+ Log::debug() << "Forwarding canceltiles request: " << canceltiles << Log::end;
+ _childProcess->getWebSocket()->sendFrame(canceltiles.data(), canceltiles.size());
+ }
}
void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index 1ad4907..a5f7c0f 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -475,26 +475,42 @@ int TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared_
}
}
-void TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscriber)
+std::string 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());
+ std::ostringstream oss;
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);
+ const auto itRem = std::find_if(subscribers.begin(), subscribers.end(),
+ [sub](std::weak_ptr<ClientSession>& ptr){ return ptr.lock().get() == sub; });
+ if (itRem != subscribers.end())
+ {
+ Log::trace("Tile " + it->first + " has " + std::to_string(subscribers.size()) + " subscribers. Removing one.");
+ subscribers.erase(itRem, itRem + 1);
+ if (subscribers.empty())
+ {
+ // No other subscriber, remove it from the render queue.
+ oss << it->second->getVersion() << ',';
+ it = _tilesBeingRendered.erase(it);
+ }
+ }
+
+ if (!subscribers.empty())
+ {
+ ++it;
+ }
}
+
+ const auto canceltiles = oss.str();
+ return (canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles);
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/TileCache.hpp b/loolwsd/TileCache.hpp
index 2801e22..6d206fc 100644
--- a/loolwsd/TileCache.hpp
+++ b/loolwsd/TileCache.hpp
@@ -43,7 +43,7 @@ public:
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::string cancelTiles(const std::shared_ptr<ClientSession> &subscriber);
std::unique_ptr<std::fstream> lookupTile(const TileDesc& tile);
commit 84cbadb4ac0862a6ef669e30ad8be79b87b280b0
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>
(cherry picked from commit 8f213bc170df4a4fc8112739ae5d678399035f66)
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 = "")
{
commit 98ad0dadc99e50724ca92723855e06fa78aa4765
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Thu Sep 15 08:43:30 2016 -0400
loolwsd: unittests for combined rendering
Change-Id: I164942c1b14727c2f1707341d625c3b4bfc3f672
Reviewed-on: https://gerrit.libreoffice.org/29116
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 836219d269ab95dd4a7335461a1d86b658fc7761)
diff --git a/loolwsd/test/TileCacheTests.cpp b/loolwsd/test/TileCacheTests.cpp
index 71469d7..f9ff937 100644
--- a/loolwsd/test/TileCacheTests.cpp
+++ b/loolwsd/test/TileCacheTests.cpp
@@ -63,6 +63,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testTileInvalidateWriter);
//CPPUNIT_TEST(testTileInvalidateCalc);
CPPUNIT_TEST(testTileQueuePriority);
+ CPPUNIT_TEST(testTileCombinedRendering);
CPPUNIT_TEST_SUITE_END();
@@ -79,6 +80,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testWriterAnyKey();
void testTileInvalidateCalc();
void testTileQueuePriority();
+ void testTileCombinedRendering();
void checkTiles(Poco::Net::WebSocket& socket,
const std::string& type,
@@ -620,9 +622,11 @@ void TileCacheTests::testTileInvalidateCalc()
void TileCacheTests::testTileQueuePriority()
{
const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840";
- const TileQueue::Payload payloadHigh(reqHigh.data(), reqHigh.data() + reqHigh.size());
+ const std::string resHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 ver=-1";
+ const TileQueue::Payload payloadHigh(resHigh.data(), resHigh.data() + resHigh.size());
const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840";
- const TileQueue::Payload payloadLow(reqLow.data(), reqLow.data() + reqLow.size());
+ const std::string resLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 ver=-1";
+ const TileQueue::Payload payloadLow(resLow.data(), resLow.data() + resLow.size());
TileQueue queue;
@@ -661,6 +665,39 @@ void TileCacheTests::testTileQueuePriority()
CPPUNIT_ASSERT_EQUAL(payloadHigh, queue.get());
}
+void TileCacheTests::testTileCombinedRendering()
+{
+ const std::string req1 = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840";
+ const std::string req2 = "tile part=0 width=256 height=256 tileposx=3840 tileposy=0 tilewidth=3840 tileheight=3840";
+ const std::string req3 = "tile part=0 width=256 height=256 tileposx=0 tileposy=3840 tilewidth=3840 tileheight=3840";
+ const std::string req4 = "tile part=0 width=256 height=256 tileposx=3840 tileposy=3840 tilewidth=3840 tileheight=3840";
+
+ const std::string resHor = "tilecombine part=0 width=256 height=256 tileposx=0,3840 tileposy=0,0 imgsize=0,0 tilewidth=3840 tileheight=3840";
+ const TileQueue::Payload payloadHor(resHor.data(), resHor.data() + resHor.size());
+ const std::string resVer = "tilecombine part=0 width=256 height=256 tileposx=0,0 tileposy=0,3840 imgsize=0,0 tilewidth=3840 tileheight=3840";
+ const TileQueue::Payload payloadVer(resVer.data(), resVer.data() + resVer.size());
+ const std::string resFull = "tilecombine part=0 width=256 height=256 tileposx=0,3840,0 tileposy=0,0,3840 imgsize=0,0,0 tilewidth=3840 tileheight=3840";
+ const TileQueue::Payload payloadFull(resFull.data(), resFull.data() + resFull.size());
+
+ TileQueue queue;
+
+ // Horizontal.
+ queue.put(req1);
+ queue.put(req2);
+ CPPUNIT_ASSERT_EQUAL(payloadHor, queue.get());
+
+ // Vertical.
+ queue.put(req1);
+ queue.put(req3);
+ CPPUNIT_ASSERT_EQUAL(payloadVer, queue.get());
+
+ // Vertical.
+ queue.put(req1);
+ queue.put(req2);
+ queue.put(req3);
+ CPPUNIT_ASSERT_EQUAL(payloadFull, queue.get());
+}
+
void TileCacheTests::checkTiles(Poco::Net::WebSocket& socket, const std::string& docType, const std::string& name)
{
const std::string current = "current=";
commit 87382195ced293d3e3a0e30b0185b12af12c39bc
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Thu Sep 15 08:40:26 2016 -0400
loolwsd: combine tiles before rendering to reduce latency
Change-Id: I5af2d2a9ddf3b5a3db5bc5f0835687d7cae5b17c
Reviewed-on: https://gerrit.libreoffice.org/29115
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit e9f37433d7aafb0ac91e240cc4adab758036508f)
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index a04945c..3b4e39b 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -1279,6 +1279,10 @@ private:
{
pThis->renderCombinedTiles(tokens, pThis->_ws);
}
+ else
+ {
+ Log::error("Unexpected tile request: [" + message + "].");
+ }
}
}
catch (const std::exception& exc)
diff --git a/loolwsd/MessageQueue.cpp b/loolwsd/MessageQueue.cpp
index 0660d2b..16946e2 100644
--- a/loolwsd/MessageQueue.cpp
+++ b/loolwsd/MessageQueue.cpp
@@ -48,6 +48,8 @@ void MessageQueue::remove_if(const std::function<bool(const Payload&)>& pred)
void MessageQueue::put_impl(const Payload& value)
{
+ const auto msg = std::string(value.data(), value.size());
+ Log::trace() << "Pushing into MQ [" << msg << "]" << Log::end;
_queue.push_back(value);
}
@@ -73,6 +75,8 @@ void BasicTileQueue::put_impl(const Payload& value)
const auto msg = std::string(&value[0], value.size());
if (msg == "canceltiles")
{
+ Log::error("Unexpected canceltiles!");
+
// remove all the existing tiles from the queue
_queue.erase(std::remove_if(_queue.begin(), _queue.end(),
[](const Payload& v)
@@ -104,16 +108,6 @@ void TileQueue::put_impl(const Payload& value)
{
const auto newMsg = msg.substr(0, msg.find(" ver"));
- // TODO: implement a real re-ordering here, so that the tiles closest to
- // the cursor are returned first.
- // * we will want to put just a general "tile" message to the queue
- // * add a std::set that handles the tiles
- // * change the get_impl() to decide which tile is the correct one to
- // be returned
- // * we will also need to be informed about the position of the cursor
- // so that get_impl() returns optimal results
- //
- // For now: just don't put duplicates into the queue
for (size_t i = 0; i < _queue.size(); ++i)
{
auto& it = _queue[i];
@@ -201,4 +195,64 @@ bool TileQueue::priority(const std::string& tileMsg)
return false;
}
+MessageQueue::Payload TileQueue::get_impl()
+{
+ std::vector<TileDesc> tiles;
+ const auto front = _queue.front();
+ _queue.pop_front();
+
+ auto msg = std::string(front.data(), front.size());
+ Log::trace() << "MessageQueue Get, Size: " << _queue.size() << ", Front: " << msg << Log::end;
+
+ if (msg.compare(0, 5, "tile ") != 0 || msg.find("id=") != std::string::npos)
+ {
+ // Don't combine non-tiles or tiles with id.
+ Log::trace() << "MessageQueue res: " << msg << Log::end;
+ return front;
+ }
+
+ tiles.emplace_back(TileDesc::parse(msg));
+
+ // Combine as many tiles as possible with the top one.
+ for (size_t i = 0; i < _queue.size(); )
+ {
+ auto& it = _queue[i];
+ msg = std::string(it.data(), it.size());
+ if (msg.compare(0, 5, "tile ") != 0 ||
+ msg.find("id=") != std::string::npos)
+ {
+ // Don't combine non-tiles or tiles with id.
+ continue;
+ }
+
+ auto tile2 = TileDesc::parse(msg);
+ bool found = false;
+ Log::trace() << "combining?: " << msg << Log::end;
+
+ // Check if adjacent tiles.
+ for (auto& tile : tiles)
+ {
+ if (tile.isAdjacent(tile2))
+ {
+ tiles.emplace_back(tile2);
+ _queue.erase(_queue.begin() + i);
+ found = true;
+ break;
+ }
+ }
+
+ i += !found;
+ }
+
+ if (tiles.size() == 1)
+ {
+ msg = tiles[0].serialize("tile");
+ return Payload(msg.data(), msg.data() + msg.size());
+ }
+
+ auto tileCombined = TileCombined::create(tiles).serialize("tilecombine");
+ Log::trace() << "MessageQueue res: " << tileCombined << Log::end;
+ return Payload(tileCombined.data(), tileCombined.data() + tileCombined.size());
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/MessageQueue.hpp b/loolwsd/MessageQueue.hpp
index f1b14b2..4d1efba 100644
--- a/loolwsd/MessageQueue.hpp
+++ b/loolwsd/MessageQueue.hpp
@@ -58,7 +58,7 @@ protected:
bool wait_impl() const;
- Payload get_impl();
+ virtual Payload get_impl();
void clear_impl();
@@ -124,6 +124,8 @@ public:
protected:
virtual void put_impl(const Payload& value) override;
+ virtual Payload get_impl() override;
+
private:
/// Bring the underlying tile (if any) to the top.
diff --git a/loolwsd/TileDesc.hpp b/loolwsd/TileDesc.hpp
index 0258645..20ac257 100644
--- a/loolwsd/TileDesc.hpp
+++ b/loolwsd/TileDesc.hpp
@@ -10,6 +10,7 @@
#ifndef INCLUDED_TILEDESC_HPP
#define INCLUDED_TILEDESC_HPP
+#include <cassert>
#include <map>
#include <sstream>
#include <string>
@@ -60,6 +61,7 @@ public:
void setVersion(const int ver) { _ver = ver; }
int getImgSize() const { return _imgSize; }
void setImgSize(const int imgSize) { _imgSize = imgSize; }
+
bool intersectsWithRect(int x, int y, int w, int h) const
{
return x + w >= getTilePosX() &&
@@ -68,6 +70,26 @@ public:
y <= getTilePosY() + getTileHeight();
}
+ bool intersects(const TileDesc& other) const
+ {
+ return intersectsWithRect(other.getTilePosX(), other.getTilePosY(),
+ other.getTileWidth(), other.getTileHeight());
+ }
+
+ bool isAdjacent(const TileDesc& other) const
+ {
+ if (other.getPart() != getPart() ||
+ other.getWidth() != getWidth() ||
+ other.getHeight() != getHeight() ||
+ other.getTileWidth() != getTileWidth() ||
+ other.getTileHeight() != getTileHeight())
+ {
+ return false;
+ }
+
+ return intersects(other);
+ }
+
/// Serialize this instance into a string.
/// Optionally prepend a prefix.
std::string serialize(const std::string& prefix = "") const
@@ -334,6 +356,26 @@ public:
return parse(tokens);
}
+ static
+ TileCombined create(const std::vector<TileDesc>& tiles)
+ {
+ assert(!tiles.empty());
+
+ std::ostringstream xs;
+ std::ostringstream ys;
+ int ver = -1;
+
+ for (auto& tile : tiles)
+ {
+ xs << tile.getTilePosX() << ',';
+ ys << tile.getTilePosY() << ',';
+ ver = std::max(tile.getVersion(), ver);
+ }
+
+ return TileCombined(tiles[0].getPart(), tiles[0].getWidth(), tiles[0].getHeight(),
+ xs.str(), ys.str(), tiles[0].getTileWidth(), tiles[0].getTileHeight(), ver, "", -1);
+ }
+
private:
std::vector<TileDesc> _tiles;
int _part;
commit df1ab76d5bfe868fd516bec81768e1c9c2790989
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 17:51:05 2016 -0400
loolwsd: added impress tiles unittest
Change-Id: I25036f6a9ad77ebd06991866245e89c1045fbea3
Reviewed-on: https://gerrit.libreoffice.org/29114
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit cdafb34b534535086b769264a9fdb926f9beea4a)
diff --git a/loolwsd/test/TileCacheTests.cpp b/loolwsd/test/TileCacheTests.cpp
index b9f8900..71469d7 100644
--- a/loolwsd/test/TileCacheTests.cpp
+++ b/loolwsd/test/TileCacheTests.cpp
@@ -53,6 +53,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testSimpleCombine);
CPPUNIT_TEST(testPerformance);
CPPUNIT_TEST(testUnresponsiveClient);
+ CPPUNIT_TEST(testImpressTiles);
CPPUNIT_TEST(testClientPartImpress);
CPPUNIT_TEST(testClientPartCalc);
#if ENABLE_DEBUG
@@ -69,6 +70,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testSimpleCombine();
void testPerformance();
void testUnresponsiveClient();
+ void testImpressTiles();
void testClientPartImpress();
void testClientPartCalc();
void testSimultaneousTilesRenderedJustOnce();
@@ -274,6 +276,22 @@ void TileCacheTests::testUnresponsiveClient()
socket2.shutdown();
}
+void TileCacheTests::testImpressTiles()
+{
+ try
+ {
+ const std::string testName = "impressTiles ";
+ auto socket = *loadDocAndGetSocket("setclientpart.odp", _uri, testName);
+
+ sendTextFrame(socket, "tile part=0 width=180 height=135 tileposx=0 tileposy=0 tilewidth=15875 tileheight=11906 id=0", testName);
+ getTileMessage(socket, testName);
+ }
+ catch (const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText());
+ }
+}
+
void TileCacheTests::testClientPartImpress()
{
try
commit 1477141b09b615b47bbc9b6e4108c0fea73e7487
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Sep 20 20:32:30 2016 +0300
Add configure options to use a self-built cppunit
I wanted to try loolwsd built with _GLIBCXX_DEBUG defined and thus
needed cppunit built like that, too. (And Poco, but we already had
configury to point to a self-built Poco.)
(cherry picked from commit b7aa4791203d8600d502e584969c663b7bbb8cf4)
diff --git a/loolwsd/configure.ac b/loolwsd/configure.ac
index 06b511c..5adeb89 100644
--- a/loolwsd/configure.ac
+++ b/loolwsd/configure.ac
@@ -76,6 +76,14 @@ AC_ARG_WITH([libpng-libs],
AS_HELP_STRING([--with-libpng-libs=<path>],
[Path to the "lib" directory with the libpng libraries]))
+AC_ARG_WITH([cppunit-includes],
+ AS_HELP_STRING([--with-cppunit-includes=<path>],
+ [Path to the "include" directory with the Cppunit headers]))
+
+AC_ARG_WITH([cppunit-libs],
+ AS_HELP_STRING([--with-cppunit-libs=<path>],
+ [Path to the "lib" directory with the Cppunit libraries]))
+
AC_ARG_ENABLE([ssl],
AS_HELP_STRING([--disable-ssl],
[Compile without SSL support]))
@@ -168,10 +176,20 @@ AS_IF([test -n "$with_libpng_includes"],
AS_IF([test -n "$with_libpng_libs"],
[LDFLAGS="$LDFLAGS -L${with_libpng_libs}"])
+AS_IF([test -n "$with_cppunit_includes"],
+ [CPPFLAGS="$CPPFLAGS -isystem ${with_cppunit_includes}"])
+
+AS_IF([test -n "$with_cppunit_libs"],
+ [LDFLAGS="$LDFLAGS -L${with_cppunit_libs}"])
+
AS_IF([test `uname -s` = Linux],
[AS_IF([test -n "$with_poco_libs"],
[LDFLAGS="$LDFLAGS -Wl,-rpath,${with_poco_libs}"])])
+AS_IF([test `uname -s` = Linux],
+ [AS_IF([test -n "$with_cppunit_libs"],
+ [LDFLAGS="$LDFLAGS -Wl,-rpath,${with_cppunit_libs}"])])
+
AS_IF([test `uname -s` != Darwin],
[AC_SEARCH_LIBS([dlopen],
[dl dld],
commit 169a6e7f5d1a16a49e8d54465f22c0a2c4ee655e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Sep 20 07:56:06 2016 -0400
loolwsd: remove the callback descriptor after destroyView
On destroyView Core must flush the events queue, otherwise
by the next idle the descriptor will be gone and we'll
be using freed memory.
Change-Id: I6d3d8f9461bc156383a7294e9c65c535d79f2e7a
Reviewed-on: https://gerrit.libreoffice.org/29088
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 3b2055a0a880e79f9a4c3dec0fa2c91cbc9c07ce)
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 4f1f448..a04945c 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -1002,8 +1002,8 @@ private:
const auto viewId = session.getViewId();
_loKitDocument->setView(viewId);
_loKitDocument->registerCallback(nullptr, nullptr);
- _viewIdToCallbackDescr.erase(viewId);
_loKitDocument->destroyView(viewId);
+ _viewIdToCallbackDescr.erase(viewId);
Log::debug("Destroyed view " + std::to_string(viewId));
}
commit faa965a996567f665c1e048cc998b6c7b4e1cc8a
Author: László Németh <laszlo.nemeth at collabora.com>
Date: Tue Sep 20 13:16:02 2016 +0200
add client-side tile debugging mode
Extending the document URL with debug=1 option will switch
- visible tile boundaries
- update numbers over tiles
and invalidated tiles will be blue during waiting for an update.
(cherry picked from commit fb351f2c1af04e784b66aefb2d1fd2bd696df6c1)
diff --git a/loleaflet/src/layer/tile/CalcTileLayer.js b/loleaflet/src/layer/tile/CalcTileLayer.js
index b3ac37f..1d51cf8 100644
--- a/loleaflet/src/layer/tile/CalcTileLayer.js
+++ b/loleaflet/src/layer/tile/CalcTileLayer.js
@@ -64,6 +64,9 @@ L.CalcTileLayer = L.TileLayer.extend({
}
tilePositionsY += tileTopLeft.y;
needsNewTiles = true;
+ if (this._debug && this._tiles[key]._debugTile) {
+ this._tiles[key]._debugTile.setStyle({fillOpacity: 0.5});
+ }
}
else {
// tile outside of the visible area, just remove it
diff --git a/loleaflet/src/layer/tile/ImpressTileLayer.js b/loleaflet/src/layer/tile/ImpressTileLayer.js
index 8f2358a..96934d6 100644
--- a/loleaflet/src/layer/tile/ImpressTileLayer.js
+++ b/loleaflet/src/layer/tile/ImpressTileLayer.js
@@ -48,6 +48,9 @@ L.ImpressTileLayer = L.TileLayer.extend({
}
tilePositionsY += tileTopLeft.y;
needsNewTiles = true;
+ if (this._debug && this._tiles[key]._debugTile) {
+ this._tiles[key]._debugTile.setStyle({fillOpacity: 0.5});
+ }
}
else {
// tile outside of the visible area, just remove it
diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index 4317e17..b9e0afb 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -12,6 +12,12 @@ if (typeof String.prototype.startsWith !== 'function') {
};
}
+function getParameterByName(name) {
+ name = name.replace(/[\[]/, '\\[').replace(/[\]]/, '\\]');
+ var regex = new RegExp('[\\?&]' + name + '=([^&#]*)'), results = regex.exec(location.search);
+ return results === null ? '' : results[1].replace(/\+/g, ' ');
+}
+
L.Compatibility = {
clipboardGet: function (event) {
var text = null;
@@ -146,6 +152,12 @@ L.TileLayer = L.GridLayer.extend({
map.addLayer(this._viewSelectionsGroup);
this._viewSelections = {};
+ this._debug = (getParameterByName('debug') == '1');
+ if (this._debug) {
+ this._debugInfo = new L.LayerGroup();
+ map.addLayer(this._debugInfo);
+ }
+
this._searchResultsLayer = new L.LayerGroup();
map.addLayer(this._searchResultsLayer);
@@ -982,6 +994,25 @@ L.TileLayer = L.GridLayer.extend({
coords.part = command.part;
var key = this._tileCoordsToKey(coords);
var tile = this._tiles[key];
+ if (this._debug && tile) {
+ var tileBound = this._keyToBounds(key);
+ if (tile._debugLoadCount) {
+ tile._debugLoadCount += 1;
+ } else {
+ tile._debugLoadCount = 1;
+ }
+ if (!tile._debugPopup) {
+ tile._debugPopup = L.popup({offset: new L.Point(0, 0), autoPan: false, closeButton: false, closeOnClick: false})
+ .setLatLng(new L.LatLng(tileBound.getSouth(), tileBound.getCenter().lng)).setContent('-');
+ this._debugInfo.addLayer(tile._debugPopup);
+ tile._debugTile = L.rectangle(tileBound, {color: 'blue', weight: 1, fillOpacity: 0, pointerEvents: 'none'});
+ this._debugInfo.addLayer(tile._debugTile);
+ }
+ tile._debugPopup.setContent('' + this._tiles[key]._debugLoadCount);
+ if (tile._debugTile) {
+ tile._debugTile.setStyle({fillOpacity: 0});
+ }
+ }
if (command.id !== undefined) {
this._map.fire('tilepreview', {
tile: img,
@@ -1701,6 +1732,13 @@ L.TileLayer = L.GridLayer.extend({
},
_invalidateClientVisibleArea: function() {
+ if (this._debug) {
+ this._debugInfo.clearLayers();
+ for (var key in this._tiles) {
+ this._tiles[key]._debugPopup = null;
+ this._tiles[key]._debugTile = null;
+ }
+ }
this._clientVisibleArea = true;
}
});
diff --git a/loleaflet/src/layer/tile/WriterTileLayer.js b/loleaflet/src/layer/tile/WriterTileLayer.js
index ab604aa..d29c3d5 100644
--- a/loleaflet/src/layer/tile/WriterTileLayer.js
+++ b/loleaflet/src/layer/tile/WriterTileLayer.js
@@ -49,6 +49,9 @@ L.WriterTileLayer = L.TileLayer.extend({
}
tilePositionsY += tileTopLeft.y;
needsNewTiles = true;
+ if (this._debug && this._tiles[key]._debugTile) {
+ this._tiles[key]._debugTile.setStyle({fillOpacity: 0.5});
+ }
}
else {
// tile outside of the visible area, just remove it
commit 83fa35a797c9bcdcd81d632192bac717064e8c7a
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Sep 20 13:07:29 2016 +0300
Don't erase the CallbackDescriptor record before turning off callbacks
Seems to fix the deadlock problem, or at least makes it very much
rarer. (I am not entirely certain that a callback might not be in
progress already (in another thread) when we turn off callbacks, and
in that case the callback might still then access freed memory?)
(cherry picked from commit 90c7b553edacc2425279d5bc21c5cb970361cdb4)
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 02919e6..4f1f448 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -1000,9 +1000,9 @@ private:
std::unique_lock<std::mutex> lock(_loKitDocument->getLock());
const auto viewId = session.getViewId();
- _viewIdToCallbackDescr.erase(viewId);
_loKitDocument->setView(viewId);
_loKitDocument->registerCallback(nullptr, nullptr);
+ _viewIdToCallbackDescr.erase(viewId);
_loKitDocument->destroyView(viewId);
Log::debug("Destroyed view " + std::to_string(viewId));
}
commit fab03cba2f1411caa0f7d161ec13a93b9ff89740
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Sep 20 12:58:50 2016 +0300
Poco::Thread::current() is not reliable for logging
It can return null for no obvious reason, leading to misleading
logging where the same thread is identified as numer zero at one place
and non-zero at another. So use the actual Linux thread id in logging.
Sure, thread ids are somewhat less convenient, as they are larger
numbers, from the same number space as process ids.
(cherry picked from commit 8207412c020284b458eddeeb3501f8bb20ccb09e)
diff --git a/loolwsd/Log.cpp b/loolwsd/Log.cpp
index 0c327df..c146af5 100644
--- a/loolwsd/Log.cpp
+++ b/loolwsd/Log.cpp
@@ -15,6 +15,9 @@
#include <sstream>
#include <string>
+#include <sys/syscall.h>
+#include <unistd.h>
+
#include <Poco/ConsoleChannel.h>
#include <Poco/FileChannel.h>
#include <Poco/FormattingChannel.h>
@@ -88,8 +91,8 @@ namespace Log
const char *appName = (Source.inited ? Source.id.c_str() : "<shutdown>");
assert(strlen(appName) + 32 + 28 < 1024 - 1);
- snprintf(buffer, 4095, "%s-%.2d %d:%.2d:%.2d.%.6d [ %s ] %s ", appName,
- (Poco::Thread::current() ? Poco::Thread::current()->id() : 0),
+ snprintf(buffer, 4095, "%s-%.04lu %d:%.2d:%.2d.%.6d [ %s ] %s ", appName,
+ syscall(SYS_gettid),
(int)hours, (int)minutes, (int)seconds, (int)usec,
procName, level);
}
commit e6a6f84411a32ed1ddeb16780b2ad94660c61ad0
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Tue Sep 20 15:15:56 2016 +0530
loleaflet: Fix some cases of leaked connecting spinner
Change-Id: If694e8f699a2a087dad1e36fc3b67960ac294560
(cherry picked from commit f84cad1e8d8ac18cc39e68a156ed06961875e349)
diff --git a/loleaflet/src/control/Permission.js b/loleaflet/src/control/Permission.js
index 5be2554..8df2297 100644
--- a/loleaflet/src/control/Permission.js
+++ b/loleaflet/src/control/Permission.js
@@ -3,6 +3,7 @@
*/
L.Map.include({
setPermission: function (perm) {
+ this._oldPermission = this._permission;
this._permission = perm;
if (perm === 'edit') {
this._socket.sendMessage('requestloksession');
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 29e7059..58e1de8 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -74,9 +74,11 @@ L.Socket = L.Class.extend({
// TODO: Move the version number somewhere sensible.
this._doSend('loolclient ' + this.ProtocolVersionNumber);
+ var reconnecting = false;
var msg = 'load url=' + this._map.options.doc;
if (this._map._docLayer) {
// we are reconnecting after a lost connection
+ reconnecting = true;
msg += ' part=' + this._map.getCurrentPartNumber();
this._map.fire('statusindicator', {statusType : 'reconnected'});
}
@@ -100,6 +102,10 @@ L.Socket = L.Class.extend({
}
this._msgQueue = [];
+ if (reconnecting) {
+ this._map.setPermission(this._map._oldPermission);
+ }
+
this._map._activate();
},
diff --git a/loleaflet/src/map/Map.js b/loleaflet/src/map/Map.js
index e2204b4..7baeaae 100644
--- a/loleaflet/src/map/Map.js
+++ b/loleaflet/src/map/Map.js
@@ -835,7 +835,7 @@ L.Map = L.Evented.extend({
else if (e.statusType === 'setvalue') {
this._progressBar.setValue(e.value);
}
- else if (e.statusType === 'finish' || e.statusType === 'loleafletloaded') {
+ else if (e.statusType === 'finish' || e.statusType === 'loleafletloaded' || e.statusType === 'reconnected') {
this.hideBusy();
}
},
commit fdbda089de40a8691dc5d99892c7c547fa3eeb76
Author: Andras Timar <andras.timar at collabora.com>
Date: Tue Sep 20 09:52:24 2016 +0200
l10n: add src/control/Control.DocumentRepair.js for string extraction to pot file
(cherry picked from commit 7465853d34863cbf148e4524c00c353c89d5d187)
diff --git a/loleaflet/Makefile b/loleaflet/Makefile
index 1e1d7f1..ae2fa2f 100644
--- a/loleaflet/Makefile
+++ b/loleaflet/Makefile
@@ -48,6 +48,7 @@ pot:
src/admin/AdminStrings.js \
src/admin/Util.js \
src/control/Control.ColumnHeader.js \
+ src/control/Control.DocumentRepair.js \
src/control/Control.Menubar.js \
src/control/Control.RowHeader.js \
src/control/Control.Tabs.js \
commit 991fbb4b9f841c7876f931cb3e95362d8a9268be
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date: Tue Sep 20 09:46:39 2016 +0200
loolwsd: remove takeedit and editlock commands
As a follow-up to commit 77e219ceff24dd4a566dfdf4f82a6929fe9a563e
(loleaflet: Kill editlock code, completely, 2016-09-20).
Change-Id: I48a58bb738c0939f99d220eca7a8fd3f4c3debe4
(cherry picked from commit ef4ca0507a81a58786f15841d11830e90d4d7644)
diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index 17cf397..a8a1a26 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -199,8 +199,7 @@ bool ChildSession::_handleInput(const char *buffer, int length)
tokens[0] == "resetselection" ||
tokens[0] == "saveas" ||
tokens[0] == "useractive" ||
- tokens[0] == "userinactive" ||
- tokens[0] == "editlock:");
+ tokens[0] == "userinactive");
if (tokens[0] == "clientzoom")
{
@@ -266,17 +265,6 @@ bool ChildSession::_handleInput(const char *buffer, int length)
{
setIsActive(false);
}
- else if (tokens[0] == "editlock:")
- {
- // Nothing for us to do but to let the
- // client know about the edit lock state.
- // Yes, this is echoed back because it's better
- // to do this on each child's queue and thread
- // than for WSD to potentially stall while notifying
- // each client with the edit lock state.
- Log::trace("Echoing back [" + firstLine + "].");
- return sendTextFrame(firstLine);
- }
else
{
assert(false && "Unknown command token.");
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index b493c58..6465519 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -93,12 +93,7 @@ bool ClientSession::_handleInput(const char *buffer, int length)
return true;
}
- if (!isReadOnly() && tokens[0] == "takeedit")
- {
- _docBroker->takeEditLock(getId());
- return true;
- }
- else if (tokens[0] == "load")
+ if (tokens[0] == "load")
{
if (_docURL != "")
{
@@ -257,14 +252,6 @@ bool ClientSession::getStatus(const char *buffer, int length)
return forwardToPeer(_peer, buffer, length, false);
}
-bool ClientSession::setEditLock()
-{
- // Update the sate and forward to child.
- const std::string msg = "editlock: 1";
- Log::debug("Forwarding [" + msg + "] to set editlock to 1.");
- return forwardToPeer(_peer, msg.data(), msg.size(), false);
-}
-
bool ClientSession::getCommandValues(const char *buffer, int length, StringTokenizer& tokens)
{
std::string command;
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index a285efe..237fe13 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -27,7 +27,6 @@ public:
virtual ~ClientSession();
- bool setEditLock();
bool isReadOnly() const { return _isReadOnly; }
void setPeer(const std::shared_ptr<PrisonerSession>& peer) { _peer = peer; }
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 5ef3f60..6a1c037 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -112,7 +112,6 @@ DocumentBroker::DocumentBroker() :
_cursorHeight(0),
_isLoaded(false),
_isModified(false),
- _isEditLockHeld(false),
_tileVersion(0)
{
Log::info("Empty DocumentBroker (marked to destroy) created.");
@@ -136,7 +135,6 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
_cursorHeight(0),
_isLoaded(false),
_isModified(false),
- _isEditLockHeld(false),
_tileVersion(0)
{
assert(!_docKey.empty());
@@ -381,18 +379,6 @@ std::string DocumentBroker::getJailRoot() const
return Poco::Path(_childRoot, _jailId).toString();
}
-void DocumentBroker::takeEditLock(const std::string& id)
-{
- Log::debug("Session " + id + " taking the editing lock.");
- std::lock_guard<std::mutex> lock(_mutex);
-
- // Forward to all children.
- for (auto& it: _sessions)
- {
- it.second->setEditLock();
- }
-}
-
size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
{
const auto id = session->getId();
@@ -414,11 +400,6 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
{
Log::debug("Adding a readonly session [" + id + "]");
}
- else if (!_isEditLockHeld)
- {
- Log::debug("Giving editing lock to the first editable session [" + id + "].");
- _isEditLockHeld = true;
- }
// Below values are recalculated when startDestroy() is called (before destroying the
// document). It is safe to reset their values to their defaults whenever a new session is added
@@ -453,20 +434,6 @@ size_t DocumentBroker::removeSession(const std::string& id)
if (it != _sessions.end())
{
_sessions.erase(it);
-
- // pass the edit lock to first non-readonly session in map
- bool editLockGiven = false;
- for (auto& session: _sessions)
- {
- if (!session.second->isReadOnly())
- {
- session.second->setEditLock();
- editLockGiven = true;
- break;
- }
- }
-
- _isEditLockHeld = editLockGiven;
}
return _sessions.size();
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 301f896..c8a46b7 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -194,10 +194,6 @@ public:
std::string getJailRoot() const;
- /// Ignore input events from all web socket sessions
- /// except this one
- void takeEditLock(const std::string& id);
-
/// Add a new session. Returns the new number of sessions.
size_t addSession(std::shared_ptr<ClientSession>& session);
/// Connect a prison session to its client peer.
@@ -264,7 +260,6 @@ private:
mutable std::mutex _mutex;
std::condition_variable _saveCV;
std::mutex _saveMutex;
- std::atomic<bool> _isEditLockHeld;
/// Versioning is used to prevent races between
/// painting and invalidation.
diff --git a/loolwsd/PrisonerSession.cpp b/loolwsd/PrisonerSession.cpp
index 5a924bc..3844974 100644
--- a/loolwsd/PrisonerSession.cpp
+++ b/loolwsd/PrisonerSession.cpp
@@ -185,12 +185,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
_docBroker->setLoaded();
// Forward the status response to the client.
- forwardToPeer(_peer, buffer, length, isBinary);
-
- // And let clients know if they hold the edit lock.
- std::string message = "editlock: 1";
- Log::debug("Forwarding [" + message + "] in response to status.");
- return forwardToPeer(_peer, message.c_str(), message.size(), isBinary);
+ return forwardToPeer(_peer, buffer, length, isBinary);
}
else if (tokens[0] == "commandvalues:")
{
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index 3639484..bf1a8ea 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -143,13 +143,6 @@ clientvisiblearea x=<x> y=<y> width=<width> height=<height>
Invokes lok::Document::setClientVisibleArea().
-takeedit
-
- Request for an edit lock. If successful, client will receive an 'editlock: 1'
- message meaning editlock is granted.
-
- See 'editlock:' message in server -> client.
-
useractive
Sent when the user regains focus or clicks within the active area to
@@ -210,17 +203,6 @@ downloadas: jail=<jail directory> dir=<a tmp dir> name=<name> port=<port>
The client should then request http://server:port/jail/dir/name in order to download
the document
-editlock: <1 or 0>
-
- Informs the client of any change in ownership of edit lock. A value of '1'
- means client can edit the document, and '0' means that client can only view
- the document. This message always follows the 'status:' message after a
- document is loaded, so that client has this information as soon as it loads
- the document.
-
- Note that only one client can have the editlock at a time and
- others can only view.
-
error: cmd=<command> kind=<kind> [code=<error_code>]
<freeErrorText>
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index bf4eee1..e1096da 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -361,12 +361,6 @@ void HTTPWSTest::loadDoc(const std::string& documentURL)
// Might be too strict, consider something flexible instread.
CPPUNIT_ASSERT_EQUAL(std::string("type=text parts=1 current=0 width=12808 height=16408 viewid=0"), status);
}
- else if (msg.find("editlock") == 0)
- {
- // First session always gets the lock.
- CPPUNIT_ASSERT_EQUAL(std::string("editlock: 1"), msg);
- return false;
- }
return true;
});
@@ -439,8 +433,7 @@ void HTTPWSTest::testBadLoad()
// For some reason the server claims a client has the 'edit lock' even if no
// document has been successfully loaded
- if (LOOLProtocol::getFirstToken(buffer, n) == "editlock:" ||
- LOOLProtocol::getFirstToken(buffer, n) == "statusindicator:")
+ if (LOOLProtocol::getFirstToken(buffer, n) == "statusindicator:")
continue;
CPPUNIT_ASSERT_EQUAL(std::string("error: cmd=status kind=nodocloaded"), line);
@@ -1019,7 +1012,6 @@ void HTTPWSTest::testInactiveClient()
CPPUNIT_ASSERT_MESSAGE("unexpected message: " + msg,
token == "addview:" ||
token == "cursorvisible:" ||
- token == "editlock:" ||
token == "graphicselection:" ||
token == "graphicviewselection:" ||
token == "invalidatecursor:" ||
commit 81ca47d363c794fb1a778f00abf500c9b3fbdfa7
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Tue Sep 20 10:57:39 2016 +0530
loleaflet: Kill editlock code, completely
Editlock buttons have already been removed; most of this code is
unreachable/useless anyways.
Don't listen to editlock messages anymore, and always set the map
permission to edit unless specified.
Change-Id: I2ee672e72beaa48a7c6cd0bbd1c548ff10a251d1
(cherry picked from commit 77e219ceff24dd4a566dfdf4f82a6929fe9a563e)
diff --git a/loleaflet/dist/toolbar/toolbar.js b/loleaflet/dist/toolbar/toolbar.js
index 218bb46..401936f 100644
--- a/loleaflet/dist/toolbar/toolbar.js
+++ b/loleaflet/dist/toolbar/toolbar.js
@@ -123,13 +123,6 @@ function onClick(id, item, subItem) {
map.setPart(id);
}
}
- else if (id === 'takeedit') {
- if (!item.checked) {
- map._socket.sendMessage('takeedit');
- // And advertise which page we're on.
- map._socket.sendMessage('setclientpart part=' + map._docLayer._selectedPart);
- }
- }
else if (id === 'searchprev') {
map.search(L.DomUtil.get('search-input').value, true);
}
@@ -480,8 +473,6 @@ var formatButtons = {
'incrementindent': true, 'decrementindent': true, 'insertgraphic': true
};
-var takeEditPopupMessage = '<div>' + _('You are viewing now.') + '<br/>' + _('Click here to take edit.') + '</div>';
-var takeEditPopupTimeout = null;
var userJoinedPopupMessage = '<div>' + _('%user has joined') + '</div>';
var userLeftPopupMessage = '<div>' + _('%user has left') + '</div>';
var userPopupTimeout = null;
@@ -988,7 +979,9 @@ map.on('commandstatechanged', function (e) {
}
// only store the state for now;
// buttons with stored state === enabled will
- // be enabled when we get the editlock
+ // be enabled later (if we are in editmode)
+ // If we are in viewmode, these store states will be used
+ // when we get the edit access
else if (state === 'enabled') {
formatButtons[id] = true;
}
@@ -996,8 +989,7 @@ map.on('commandstatechanged', function (e) {
formatButtons[id] = false;
}
... etc. - the rest is truncated
More information about the Libreoffice-commits
mailing list