[Libreoffice-commits] online.git: loolwsd/ChildSession.cpp loolwsd/ChildSession.hpp loolwsd/LOOLKit.cpp loolwsd/LOOLWSD.cpp loolwsd/test

[Ashod Nakashian]

 loolwsd/ChildSession.cpp       |   30 +++++++++++++++++++++++++-----
 loolwsd/ChildSession.hpp       |    2 +-
 loolwsd/LOOLKit.cpp            |   22 ++++++++++------------
 loolwsd/LOOLWSD.cpp            |    2 ++
 loolwsd/test/WhiteBoxTests.cpp |    2 +-
 5 files changed, 39 insertions(+), 19 deletions(-)

New commits:
commit c542fde2446b7fda6a8d058728ee53c1355fb60c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Wed Oct 5 22:06:02 2016 -0400

    loolwsd: factor out getViewIds and broadcasting
    
    This minimizes unlocking and relocking
    on the LOK Document.
    
    Change-Id: Ibe5045c82008df907dc329c1530cb50138b0c211
    Reviewed-on: https://gerrit.libreoffice.org/29545
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index 62747e5..5e085b9 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -92,13 +92,22 @@ bool ChildSession::_handleInput(const char *buffer, int length)
         // Client is getting active again.
         // Send invalidation and other sync-up messages.
         std::unique_lock<std::recursive_mutex> lock(Mutex); //TODO: Move to top of function?
+        auto lockLokDoc(_loKitDocument->getLock());
 
         _loKitDocument->setView(_viewId);
 
-        // Notify all views about updated view info
-        _docManager.notifyViewInfo();
+        // Get the list of view ids from the core
+        const int viewCount = _loKitDocument->getViewsCount();
+        std::vector<int> viewIds(viewCount);
+        _loKitDocument->getViewIds(viewIds.data(), viewCount);
 
         const int curPart = _loKitDocument->getPart();
+
+        lockLokDoc.unlock();
+
+        // Notify all views about updated view info
+        _docManager.notifyViewInfo(viewIds);
+
         sendTextFrame("curpart: part=" + std::to_string(curPart));
         sendTextFrame("setpart: part=" + std::to_string(curPart));
 
@@ -296,13 +305,18 @@ bool ChildSession::loadDocument(const char * /*buffer*/, int /*length*/, StringT
     std::unique_lock<std::recursive_mutex> lock(Mutex);
 
     _loKitDocument = _docManager.onLoad(getId(), _jailedFilePath, _userName, _docPassword, renderOpts, _haveDocPassword);
-    if (!_loKitDocument)
+    if (!_loKitDocument || _viewId < 0)
     {
         Log::error("Failed to get LoKitDocument instance.");
         return false;
     }
 
     Log::info("Created new view with viewid: [" + std::to_string(_viewId) + "] for username: [" + _userName + "].");
+
+    auto lockLokDoc(_loKitDocument->getLock());
+
+    _loKitDocument->setView(_viewId);
+
     _docType = LOKitHelper::getDocumentTypeAsString(_loKitDocument->get());
     if (_docType != "text" && part != -1)
     {
@@ -311,7 +325,6 @@ bool ChildSession::loadDocument(const char * /*buffer*/, int /*length*/, StringT
 
     // Respond by the document status
     Log::debug("Sending status after loading view " + std::to_string(_viewId) + ".");
-    _loKitDocument->setView(_viewId);
     const auto status = LOKitHelper::documentStatus(_loKitDocument->get());
     if (status.empty() || !sendTextFrame("status: " + status))
     {
@@ -319,8 +332,15 @@ bool ChildSession::loadDocument(const char * /*buffer*/, int /*length*/, StringT
         return false;
     }
 
+    // Get the list of view ids from the core
+    const int viewCount = _loKitDocument->getViewsCount();
+    std::vector<int> viewIds(viewCount);
+    _loKitDocument->getViewIds(viewIds.data(), viewCount);
+
+    lockLokDoc.unlock();
+
     // Inform everyone (including this one) about updated view info
-    _docManager.notifyViewInfo();
+    _docManager.notifyViewInfo(viewIds);
 
     Log::info("Loaded session " + getId());
     return true;
diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp
index 1ee6ecd..e8e23ac 100644
--- a/loolwsd/ChildSession.hpp
+++ b/loolwsd/ChildSession.hpp
@@ -42,7 +42,7 @@ public:
 
     /// Send updated view info to all active sessions
     virtual
-    void notifyViewInfo() = 0;
+    void notifyViewInfo(const std::vector<int>& viewIds) = 0;
     /// Get a view ID <-> user name map.
     virtual
     std::map<int, std::string> getViewInfo() = 0;
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 33a4921..d2cfdb5 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -977,7 +977,7 @@ private:
                     << " view" << (_clientViews != 1 ? "s" : "")
                     << Log::end;
 
-        std::unique_lock<std::mutex> lock(_loKitDocument->getLock());
+        std::unique_lock<std::mutex> lockLokDoc(_loKitDocument->getLock());
 
         const auto viewId = session.getViewId();
         _loKitDocument->setView(viewId);
@@ -985,10 +985,16 @@ private:
         _loKitDocument->destroyView(viewId);
         _viewIdToCallbackDescr.erase(viewId);
         Log::debug("Destroyed view " + std::to_string(viewId));
-        lock.unlock();
+
+        // Get the list of view ids from the core
+        const int viewCount = _loKitDocument->getViewsCount();
+        std::vector<int> viewIds(viewCount);
+        _loKitDocument->getViewIds(viewIds.data(), viewCount);
+
+        lockLokDoc.unlock();
 
         // Broadcast updated view info
-        notifyViewInfo();
+        notifyViewInfo(viewIds);
     }
 
     std::map<int, std::string> getViewInfo() override
@@ -1020,16 +1026,8 @@ private:
     }
 
     /// Notify all views of viewId and their associated usernames
-    void notifyViewInfo() override
+    void notifyViewInfo(const std::vector<int>& viewIds) override
     {
-        std::unique_lock<std::mutex> lockLokDoc(_loKitDocument->getLock());
-
-        // 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();
-
         // Store the list of viewid, username mapping in a map
         std::map<int, std::string> viewInfoMap = getViewInfo();
         std::unique_lock<std::mutex> lock(_mutex);
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 23d73e6..3542df9 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -234,9 +234,11 @@ static void forkChildren(const int number)
 static void preForkChildren()
 {
     std::unique_lock<std::mutex> lock(newChildrenMutex);
+
     int numPreSpawn = LOOLWSD::NumPreSpawnedChildren;
     UnitWSD::get().preSpawnCount(numPreSpawn);
     --numPreSpawn; // ForKit always spawns one child at startup.
+
     forkChildren(numPreSpawn);
 }
 
diff --git a/loolwsd/test/WhiteBoxTests.cpp b/loolwsd/test/WhiteBoxTests.cpp
index af4e34f..d17cc22 100644
--- a/loolwsd/test/WhiteBoxTests.cpp
+++ b/loolwsd/test/WhiteBoxTests.cpp
@@ -174,7 +174,7 @@ public:
     {
     }
 
-    void notifyViewInfo() override
+    void notifyViewInfo(const std::vector<int>& /*viewIds*/) override
     {
     }