Ensure consistent canonical view id accounting between wsd and kit.

Confusion arose due to separate creation of session, and watermark
property fetch from CheckFileInfo which happens in DocumentBroker::load
which doesn't do a load. This happens in a subsequent 'load url='
message cf. global.js which can then race vs. the session creation.

This causes mis-ordering of another unhelpfully shared Session,
letting the view canonicalization list to get out of sync between
the two processes.

So instead - tell the view it's canonical id. An example of the
problems of trying to share some unclear subset of the Session
class between kit and wsd perhaps.

Change-Id: I63dc30f9a047e3f889fd339b6aaf392b9fef37b9
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>

common/Session.hpp

@@ -36,8 +36,8 @@ class SessionMap : public std::map<std::string, std::shared_ptr<T> >
     SessionMap() {
         static_assert(std::is_base_of<Session, T>::value, "sessions must have base of Session");
     }
-    /// Generate a unique key for this set of view properties
-    int getCanonicalId(const std::string &viewProps)
+    /// Generate a unique key for this set of view properties, only used by WSD
+    int createCanonicalId(const std::string &viewProps)
     {
         if (viewProps.empty())
             return 0;
@@ -49,6 +49,7 @@ class SessionMap : public std::map<std::string, std::shared_ptr<T> >
         _canonicalIds[viewProps] = id;
         return id;
     }
+    /// Lookup one session in the map that matches this canonical view id, only used by Kit
     std::shared_ptr<T> findByCanonicalId(int id)
     {
         for (const auto &it : *this) {
@@ -206,9 +207,12 @@ class Session : public MessageHandlerInterface
     const std::string& getJailedFilePathAnonym() const { return _jailedFilePathAnonym; }
 
     int  getCanonicalViewId() { return _canonicalViewId; }
-    template<class T> void recalcCanonicalViewId(SessionMap<T> &map)
+    // Only called by kit.
+    void setCanonicalViewId(int viewId) { _canonicalViewId = viewId; }
+    // Only called by wsd.
+    template<class T> void createCanonicalViewId(SessionMap<T> &map)
     {
-        _canonicalViewId = map.getCanonicalId(_watermarkText);
+        _canonicalViewId = map.createCanonicalId(_watermarkText);
     }
 
     const std::string& getDeviceFormFactor() const { return _deviceFormFactor; }

kit/Kit.cpp

@@ -517,7 +517,7 @@ class Document final : public DocumentManagerInterface
         return true;
     }
 
-    bool createSession(const std::string& sessionId)
+    bool createSession(const std::string& sessionId, int canonicalViewId)
     {
         std::unique_lock<std::mutex> lock(_mutex);
 
@@ -534,9 +534,10 @@ class Document final : public DocumentManagerInterface
                     sessionId << " on jailId: " << _jailId);
 
             auto session = std::make_shared<ChildSession>(
-                _websocketHandler,
-                sessionId, _jailId, JailRoot, *this);
+                _websocketHandler, sessionId,
+                _jailId, JailRoot, *this);
             _sessions.emplace(sessionId, session);
+            session->setCanonicalViewId(canonicalViewId);
 
             int viewId = session->getViewId();
             _lastUpdatedAt[viewId] = std::chrono::steady_clock::now();
@@ -1282,7 +1283,6 @@ class Document final : public DocumentManagerInterface
                 viewCount << " view" << (viewCount != 1 ? "s." : "."));
 
         session->initWatermark();
-        session->recalcCanonicalViewId(_sessions);
 
         return _loKitDocument;
     }
@@ -1882,12 +1882,13 @@ class KitWebSocketHandler final : public WebSocketHandler
             const std::string& sessionId = tokens[1];
             const std::string& docKey = tokens[2];
             const std::string& docId = tokens[3];
+            const int canonicalViewId = std::stoi(tokens[4]);
             const std::string fileId = Util::getFilenameFromURL(docKey);
             Util::mapAnonymized(fileId, fileId); // Identity mapping, since fileId is already obfuscated
 
             std::string url;
             URI::decode(docKey, url);
-            LOG_INF("New session [" << sessionId << "] request on url [" << url << "].");
+            LOG_INF("New session [" << sessionId << "] request on url [" << url << "] with viewId " << canonicalViewId);
 #ifndef IOS
             Util::setThreadName("kit" SHARED_DOC_THREADNAME_SUFFIX + docId);
 #endif
@@ -1901,7 +1902,7 @@ class KitWebSocketHandler final : public WebSocketHandler
             }
 
             // Validate and create session.
-            if (!(url == _document->getUrl() && _document->createSession(sessionId)))
+            if (!(url == _document->getUrl() && _document->createSession(sessionId, canonicalViewId)))
             {
                 LOG_DBG("CreateSession failed.");
             }

wsd/DocumentBroker.cpp

@@ -745,14 +745,15 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
         watermarkText = LOOLWSD::OverrideWatermark;
 #endif
 
-    LOG_DBG("Setting username [" << LOOLWSD::anonymizeUsername(username) << "] and userId [" <<
-            LOOLWSD::anonymizeUsername(userId) << "] for session [" << sessionId << ']');
-
     session->setUserId(userId);
     session->setUserName(username);
     session->setUserExtraInfo(userExtraInfo);
     session->setWatermarkText(watermarkText);
-    session->recalcCanonicalViewId(_sessions);
+    session->createCanonicalViewId(_sessions);
+
+    LOG_DBG("Setting username [" << LOOLWSD::anonymizeUsername(username) << "] and userId [" <<
+            LOOLWSD::anonymizeUsername(userId) << "] for session [" << sessionId <<
+            "] is canonical id " << session->getCanonicalViewId());
 
     // Basic file information was stored by the above getWOPIFileInfo() or getLocalFileInfo() calls
     const StorageBase::FileInfo fileInfo = _storage->getFileInfo();
@@ -1432,7 +1433,8 @@ size_t DocumentBroker::addSessionInternal(const std::shared_ptr<ClientSession>&
     const std::string id = session->getId();
 
     // Request a new session from the child kit.
-    const std::string aMessage = "session " + id + ' ' + _docKey + ' ' + _docId;
+    const std::string aMessage = "session " + id + ' ' + _docKey + ' ' +
+        _docId + ' ' + std::to_string(session->getCanonicalViewId());
     _childProcess->sendTextFrame(aMessage);
 
 #if !MOBILEAPP