[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-1-0' - 2 commits - loleaflet/src loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp loolwsd/MasterProcessSession.cpp loolwsd/MasterProcessSession.hpp

Pranav Kant pranavk at collabora.co.uk
Tue Oct 11 10:53:13 UTC 2016


 loleaflet/src/core/Socket.js     |    5 +++-
 loleaflet/src/map/Map.js         |    2 -
 loolwsd/DocumentBroker.cpp       |   45 +++++++++++++++++++++++++++++++--------
 loolwsd/DocumentBroker.hpp       |    8 +++++-
 loolwsd/LOOLWSD.cpp              |   26 +++++++++++++++-------
 loolwsd/MasterProcessSession.cpp |   10 +++++---
 loolwsd/MasterProcessSession.hpp |    7 +++++-
 7 files changed, 77 insertions(+), 26 deletions(-)

New commits:
commit c52e09d2677725725984e062cd1a45a28012c8f3
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Tue Oct 11 16:17:03 2016 +0530

    Revert "loleaflet: Fix incorrect reference to username"
    
    Structure of viewInfo object in master and this branch is
    different, so don't backport this here.
    
    This reverts commit 76e739bc0da70de966917267bbbf8fd411d81d4a.
    
    Change-Id: Ida30959a59cdf89183783ba9e4d84b7d32673489

diff --git a/loleaflet/src/map/Map.js b/loleaflet/src/map/Map.js
index 56ac23a..ba72f67 100644
--- a/loleaflet/src/map/Map.js
+++ b/loleaflet/src/map/Map.js
@@ -131,7 +131,7 @@ L.Map = L.Evented.extend({
 	},
 
 	removeView: function(viewid) {
-		var username = this._viewInfo[viewid].username;
+		var username = this._viewInfo[viewid];
 		delete this._viewInfo[viewid];
 		this.fire('removeview', {viewId: viewid, username: username});
 	},
commit 3ab3e3658429d63173f26cdff1157b5e87684b65
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Mon Jul 11 10:20:25 2016 +0530

    loolwsd: Introduce a readonly mode
    
    Specified when websocket is initialized. Documents once
    opened in readonly mode cannot edit throughout the life of the session.
    This is very much like present view mode except the ability to
    change to edit mode.
    
    Change-Id: I176e3bbf210c3383268d1a5b50dc17f0cbfb26b8
    (cherry picked from commit 62814d29cf7386343a85cc66a2fe2b039bcd2e83)
    (cherry picked from commit 3d306a0d7b0bf4030bb8b926f9e81a67d10f2b6b)

diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 688b9ca..2c28d3b 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -9,7 +9,10 @@ L.Socket = L.Class.extend({
 	initialize: function (map) {
 		this._map = map;
 		try {
-			this.socket = new WebSocket(map.options.server + '/lool/' + encodeURIComponent(map.options.doc) + '/ws');
+			var params = {
+				permission: map.options.permission
+			};
+			this.socket = new WebSocket(map.options.server + '/lool/' + encodeURIComponent(map.options.doc + '?' + $.param(params)) + '/ws');
 			this.socket.onerror = L.bind(this._onSocketError, this);
 			this.socket.onclose = L.bind(this._onSocketClose, this);
 			this.socket.onopen = L.bind(this._onSocketOpen, this);
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 818c32b..85e2830 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -96,6 +96,7 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
     _childProcess(childProcess),
     _lastSaveTime(std::chrono::steady_clock::now()),
     _markToDestroy(false),
+    _lastEditableSession(false),
     _isLoaded(false),
     _isModified(false)
 {
@@ -193,7 +194,7 @@ bool DocumentBroker::save(bool success, const std::string& result)
     // If we aren't destroying the last editable session just yet, and the file
     // timestamp hasn't changed, skip saving.
     const auto newFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified();
-    if (!isMarkedToDestroy() && newFileModifiedTime == _lastFileModifiedTime)
+    if (!isLastEditableSession() && newFileModifiedTime == _lastFileModifiedTime)
     {
         // Nothing to do.
         Log::debug() << "Skipping unnecessary saving to URI [" << uri
@@ -369,7 +370,13 @@ size_t DocumentBroker::addSession(std::shared_ptr<MasterProcessSession>& session
         Log::warn("DocumentBroker: Trying to add already existing session.");
     }
 
-    if (_sessions.size() == 1)
+    if (session->isReadOnly())
+    {
+        Log::debug("Adding a readonly session [" + id + "]");
+    }
+    // TODO: Below is not always true. What if readonly session is already opened
+    // In that case we still have to give edit lock to this *second* session.
+    else if (_sessions.size() == 1)
     {
         Log::debug("Giving editing lock to the first session [" + id + "].");
         _sessions.begin()->second->markEditLock(true);
@@ -413,11 +420,14 @@ size_t DocumentBroker::removeSession(const std::string& id)
 
         if (haveEditLock)
         {
-            // pass the edit lock to first session in map
-            it = _sessions.begin();
-            if (it != _sessions.end())
+            // pass the edit lock to first non-readonly session in map
+            for (auto& session: _sessions)
             {
-                it->second->setEditLock(true);
+                if (!session.second->isReadOnly())
+                {
+                    session.second->setEditLock(true);
+                    break;
+                }
             }
         }
     }
@@ -546,14 +556,31 @@ void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
     }
 }
 
-bool DocumentBroker::canDestroy()
+void DocumentBroker::startDestroy(const std::string& id)
 {
     std::unique_lock<std::mutex> lock(_mutex);
 
+    auto currentSession = _sessions.find(id);
+    assert(currentSession != _sessions.end());
+
+    // Check if session which is being destroyed is last non-readonly session
+    bool isLastEditableSession = !currentSession->second->isReadOnly();
+    for (auto& it: _sessions)
+    {
+        if (it.second->getId() == id)
+            continue;
+
+        if (!it.second->isReadOnly())
+        {
+            isLastEditableSession = false;
+        }
+    }
+
+    // Last editable session going away
+    _lastEditableSession = isLastEditableSession;
+
     // Last view going away, can destroy.
     _markToDestroy = (_sessions.size() <= 1);
-
-    return _markToDestroy;
 }
 
 void DocumentBroker::setModified(const bool value)
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 5fd2edd..c15c293 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -203,9 +203,12 @@ public:
 
     void handleTileResponse(const std::vector<char>& payload);
 
-    // Called when the last view is going out.
-    bool canDestroy();
+    // Called before destroying any session
+    // This method calculates and sets important states of
+    // session being destroyed.
+    void startDestroy(const std::string& id);
     bool isMarkedToDestroy() const { return _markToDestroy; }
+    bool isLastEditableSession() const { return _lastEditableSession; }
 
     bool handleInput(const std::vector<char>& payload);
 
@@ -232,6 +235,7 @@ private:
     std::unique_ptr<StorageBase> _storage;
     std::unique_ptr<TileCache> _tileCache;
     std::atomic<bool> _markToDestroy;
+    std::atomic<bool> _lastEditableSession;
     bool _isLoaded;
     bool _isModified;
     mutable std::mutex _mutex;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index be5a2c0..7d181fb 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -700,6 +700,15 @@ private:
             docBrokers.emplace(docKey, docBroker);
         }
 
+        // Check if readonly session is required
+        bool isReadOnly = false;
+        for (auto& param: uriPublic.getQueryParameters())
+        {
+            Log::debug("Query param: " + param.first + ", value: " + param.second);
+            if (param.first == "permission")
+                isReadOnly = param.second == "readonly";
+        }
+
         // Above this point exceptions are safe and will auto-cleanup.
         // Below this, we need to cleanup internal references.
         std::shared_ptr<MasterProcessSession> session;
@@ -708,7 +717,7 @@ private:
             // For ToClient sessions, we store incoming messages in a queue and have a separate
             // thread to pump them. This is to empty the queue when we get a "canceltiles" message.
             auto queue = std::make_shared<BasicTileQueue>();
-            session = std::make_shared<MasterProcessSession>(id, LOOLSession::Kind::ToClient, ws, docBroker, queue);
+            session = std::make_shared<MasterProcessSession>(id, LOOLSession::Kind::ToClient, ws, docBroker, queue, isReadOnly);
             if (!fileinfo._userName.empty())
             {
                 Log::debug(uriPublic.toString() + " requested with username [" + fileinfo._userName + "]");
@@ -749,29 +758,30 @@ private:
             {
                 std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
 
-                // We can destory if this is the last session.
-                // If not, we have to remove the session and check again.
+                // We can destroy it, before save, if this is the last session
                 // Otherwise, we may end up removing the one and only session.
                 bool removedSession = false;
-                auto canDestroy = docBroker->canDestroy();
+                docBroker->startDestroy(id);
+
+                // We issue a force-save when last editable (non-readonly) session is going away
+                bool forceSave = docBroker->isLastEditableSession();
                 sessionsCount = docBroker->getSessionsCount();
                 if (sessionsCount > 1)
                 {
                     sessionsCount = docBroker->removeSession(id);
                     removedSession = true;
                     Log::trace(docKey + ", ws_sessions--: " + std::to_string(sessionsCount));
-                    canDestroy = docBroker->canDestroy();
                 }
 
                 // If we are the last, we must wait for the save to complete.
-                if (canDestroy)
+                if (forceSave)
                 {
-                    Log::info("Shutdown of the last session, saving the document before tearing down.");
+                    Log::info("Shutdown of the last editable (non-readonly) session, saving the document before tearing down.");
                 }
 
                 // We need to wait until the save notification reaches us
                 // and Storage persists the document.
-                if (!docBroker->autoSave(canDestroy, COMMAND_TIMEOUT_MS))
+                if (!docBroker->autoSave(forceSave, COMMAND_TIMEOUT_MS))
                 {
                     Log::error("Auto-save before closing failed.");
                 }
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index 97e73a5..ebcf741 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -35,12 +35,14 @@ MasterProcessSession::MasterProcessSession(const std::string& id,
                                            const Kind kind,
                                            std::shared_ptr<Poco::Net::WebSocket> ws,
                                            std::shared_ptr<DocumentBroker> docBroker,
-                                           std::shared_ptr<BasicTileQueue> queue) :
+                                           std::shared_ptr<BasicTileQueue> queue,
+                                           const bool isReadOnly) :
     LOOLSession(id, kind, ws),
     _curPart(0),
     _loadPart(-1),
     _docBroker(docBroker),
-    _queue(queue)
+    _queue(queue),
+    _isReadOnly(isReadOnly)
 {
     Log::info("MasterProcessSession ctor [" + getName() + "].");
 }
@@ -280,7 +282,7 @@ bool MasterProcessSession::_handleInput(const char *buffer, int length)
         Log::error(getName() + ": Unexpected request [" + tokens[0] + "].");
         assert(false);
     }
-    else if (tokens[0] == "takeedit")
+    else if (!isReadOnly() && tokens[0] == "takeedit")
     {
         _docBroker->takeEditLock(getId());
         return true;
@@ -371,7 +373,7 @@ bool MasterProcessSession::_handleInput(const char *buffer, int length)
         }
 
         // Allow 'downloadas' for all kinds of views irrespective of editlock
-        if (_kind == Kind::ToClient && !isEditLocked() && tokens[0] != "downloadas" &&
+        if (_kind == Kind::ToClient && (isReadOnly() || !isEditLocked()) && tokens[0] != "downloadas" &&
             tokens[0] != "userinactive" && tokens[0] != "useractive")
         {
             std::string dummyFrame = "dummymsg";
diff --git a/loolwsd/MasterProcessSession.hpp b/loolwsd/MasterProcessSession.hpp
index eeed160..4156487 100644
--- a/loolwsd/MasterProcessSession.hpp
+++ b/loolwsd/MasterProcessSession.hpp
@@ -26,7 +26,8 @@ class MasterProcessSession final : public LOOLSession, public std::enable_shared
                          const Kind kind,
                          std::shared_ptr<Poco::Net::WebSocket> ws,
                          std::shared_ptr<DocumentBroker> docBroker,
-                         std::shared_ptr<BasicTileQueue> queue);
+                         std::shared_ptr<BasicTileQueue> queue,
+                         const bool isReadOnly = false);
     virtual ~MasterProcessSession();
 
     virtual bool getStatus(const char *buffer, int length) override;
@@ -50,6 +51,7 @@ class MasterProcessSession final : public LOOLSession, public std::enable_shared
     void setEditLock(const bool value);
     void markEditLock(const bool value) { _bEditLock = value; }
     bool isEditLocked() const { return _bEditLock; }
+    bool isReadOnly() const { return _isReadOnly; }
 
     bool shutdownPeer(Poco::UInt16 statusCode, const std::string& message);
 
@@ -97,6 +99,9 @@ public:
     // An edit lock will only allow the current session to make edits,
     // while other session opening the same document can only see
     bool _bEditLock = false;
+
+    // Whether session is opened as readonly
+    bool _isReadOnly;
 };
 
 #endif


More information about the Libreoffice-commits mailing list