[Libreoffice-commits] online.git: 3 commits - loleaflet/src loolwsd/ChildSession.cpp loolwsd/ChildSession.hpp loolwsd/LibreOfficeKit.hpp loolwsd/LOOLKit.cpp loolwsd/protocol.txt loolwsd/test

Pranav Kant pranavk at collabora.co.uk
Wed Sep 21 06:33:11 UTC 2016


 loleaflet/src/core/Socket.js          |    1 
 loleaflet/src/layer/tile/TileLayer.js |   49 ++++++++++--------
 loolwsd/ChildSession.cpp              |   11 +---
 loolwsd/ChildSession.hpp              |    8 ---
 loolwsd/LOOLKit.cpp                   |   88 ++++++++++++++++++----------------
 loolwsd/LibreOfficeKit.hpp            |   15 +++++
 loolwsd/protocol.txt                  |   18 +-----
 loolwsd/test/httpwstest.cpp           |    6 --
 8 files changed, 105 insertions(+), 91 deletions(-)

New commits:
commit 0991924a597861b93fde902acad49f64d2b55ecc
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

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 626eab255a23b985507c08c74558de7a67ce40c8
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

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 46107dd0c8973f48117d50111fe7a397320412c8
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

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:");


More information about the Libreoffice-commits mailing list