wsd: make sure browsersetting.json is sent early as possible

- async browsersetting might not be ready by the time we download the document - now we do sync request if async has not yet finished the execution to make sure we get browsersetting as early as possible Signed-off-by: Rashesh <rashesh.padia@collabora.com> Change-Id: I4b01b8bcbd0fb2397888495b982aba366b99c3b8
CollaboraOnline · Dec 27, 2024 · 21f90d0 · 21f90d0
1 parent 79b8f17
commit 21f90d0
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 29 deletions.
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
@@ -93,7 +93,8 @@ ClientSession::ClientSession(
     _isTextDocument(false),
     _thumbnailSession(false),
     _canonicalViewId(0),
-    _sentAudit(false)
+    _sentAudit(false),
+    _sentBrowserSetting(false)
 {
     const std::size_t curConnections = ++COOLWSD::NumConnections;
     LOG_INF("ClientSession ctor [" << getName() << "] for URI: [" << _uriPublic.toString()

diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
@@ -272,6 +272,13 @@ class ClientSession final : public Session
 
     int  getCanonicalViewId() const { return _canonicalViewId; }
 
+    bool getSentBrowserSetting() const { return _sentBrowserSetting; }
+
+    void setSentBrowserSetting(const bool sentBrowserSetting)
+    {
+        _sentBrowserSetting = sentBrowserSetting;
+    }
+
 private:
     std::shared_ptr<ClientSession> client_from_this()
     {
@@ -429,6 +436,9 @@ class ClientSession final : public Session
 
     /// If server audit was already sent
     bool _sentAudit;
+
+    /// If browser setting was already sent
+    bool _sentBrowserSetting;
 };
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
@@ -1019,6 +1019,8 @@ bool DocumentBroker::download(
     // Call the storage specific fileinfo functions
     std::string templateSource;
 
+    std::string browserSettingUri;
+
 #if !MOBILEAPP
     std::chrono::milliseconds checkFileInfoCallDurationMs = std::chrono::milliseconds::zero();
     WopiStorage* wopiStorage = dynamic_cast<WopiStorage*>(_storage.get());
@@ -1049,6 +1051,7 @@ bool DocumentBroker::download(
 
         if (session)
         {
+            browserSettingUri = wopiFileInfo->getBrowserSettingsUri();
             templateSource =
                 updateSessionWithWopiInfo(session, wopiStorage, std::move(wopiFileInfo));
         }
@@ -1170,6 +1173,27 @@ bool DocumentBroker::download(
             session->sendTextFrame(msg);
         }
     }
+
+    // if async browsersetting json request is not downlaoded even after document download is complete
+    // we do sync request to make sure the browser setting json sent before document starts to load
+    if (session && !browserSettingUri.empty())
+    {
+        LOG_DBG("BrowserSetting for docKey ["
+                << _docKey << "] for session #" << session->getId()
+                << (session->getSentBrowserSetting() ? " already exists" : " is missing"));
+        if (!session->getSentBrowserSetting())
+        {
+            sendBrowserSetting(session, browserSettingUri, /** async **/ false);
+            if (!session->getSentBrowserSetting())
+            {
+                const std::string uriAnonym = COOLWSD::anonymizeUrl(browserSettingUri);
+                LOG_ERR("Request to uri["
+                        << uriAnonym
+                        << "] failed or timedout while adding session #" + session->getId());
+            }
+        }
+    }
+
 #endif
     return true;
 }
@@ -1452,7 +1476,7 @@ DocumentBroker::updateSessionWithWopiInfo(const std::shared_ptr<ClientSession>&
     std::string browserSettingUri = wopiFileInfo->getBrowserSettingsUri();
     if (_sessions.empty() && !browserSettingUri.empty())
     {
-        asyncSendBrowserSetting(session, browserSettingUri);
+        sendBrowserSetting(session, browserSettingUri);
     }
 
     // Pass the ownership to the client session.
@@ -1635,8 +1659,8 @@ void DocumentBroker::asyncInstallPresets(const std::shared_ptr<ClientSession> se
     _asyncInstallTask->appendCallback([this](bool){ _asyncInstallTask.reset(); });
 }
 
-void DocumentBroker::asyncSendBrowserSetting(const std::shared_ptr<ClientSession>& session,
-                                             const std::string& browserSettingUri)
+void DocumentBroker::sendBrowserSetting(const std::shared_ptr<ClientSession>& session,
+                                        const std::string& browserSettingUri, bool async)
 {
     // Download the json for browser settings
     const Poco::URI settingsUri{ browserSettingUri };
@@ -1646,30 +1670,14 @@ void DocumentBroker::asyncSendBrowserSetting(const std::shared_ptr<ClientSession
     request.set("User-Agent", http::getAgentString());
 
     const std::string uriAnonym = COOLWSD::anonymizeUrl(browserSettingUri);
-    LOG_DBG("Getting settings from [" << uriAnonym << ']');
+    const std::string requestType = async ? "asyncRequest" : "syncRequest";
+    LOG_DBG("Getting settings from [" << uriAnonym << "] using " << requestType);
 
-    http::Session::FinishedCallback finishedCallback =
-        [uriAnonym, session](const std::shared_ptr<http::Session>& configSession)
+    auto parseResponse =
+        [session, uriAnonym](const std::shared_ptr<const http::Response>& httpResponse)
     {
-        if (SigUtil::getShutdownRequestFlag())
-        {
-            LOG_DBG("Shutdown flagged, giving up on in-flight requests");
+        if (session->getSentBrowserSetting())
             return;
-        }
-
-        const std::shared_ptr<const http::Response> httpResponse = configSession->response();
-        LOG_TRC("DocumentBroker::asyncSendBrowserSetting returned "
-                << httpResponse->statusLine().statusCode());
-
-        const bool failed = (httpResponse->statusLine().statusCode() != http::StatusCode::OK);
-        if (failed)
-        {
-            if (httpResponse->statusLine().statusCode() == http::StatusCode::Forbidden)
-                LOG_ERR("Access denied to [" << uriAnonym << ']');
-            else
-                LOG_ERR("Invalid URI or access denied to [" << uriAnonym << ']');
-            return;
-        }
 
         const std::string& body = httpResponse->getBody();
         Poco::JSON::Object::Ptr settings;
@@ -1683,10 +1691,61 @@ void DocumentBroker::asyncSendBrowserSetting(const std::shared_ptr<ClientSession
         {
             LOG_ERR("Parse of browserSetting json: " << uriAnonym << " failed");
         }
+        session->setSentBrowserSetting(true);
     };
 
-    httpSession->setFinishedHandler(std::move(finishedCallback));
-    httpSession->asyncRequest(request, *_poll);
+    if (async)
+    {
+        http::Session::FinishedCallback finishedCallback =
+            [uriAnonym, session, requestType,
+             parseResponse](const std::shared_ptr<http::Session>& configSession)
+        {
+            if (SigUtil::getShutdownRequestFlag())
+            {
+                LOG_DBG("Shutdown flagged, giving up on in-flight requests");
+                return;
+            }
+
+            const std::shared_ptr<const http::Response> httpResponse = configSession->response();
+            LOG_TRC("DocumentBroker::sendBrowserSetting with "
+                    << requestType << " returned " << httpResponse->statusLine().statusCode());
+
+            const bool failed = (httpResponse->statusLine().statusCode() != http::StatusCode::OK);
+            if (failed)
+            {
+                if (httpResponse->statusLine().statusCode() == http::StatusCode::Forbidden)
+                    LOG_ERR("Access denied to [" << uriAnonym << ']');
+                else
+                    LOG_ERR("Invalid URI or access denied to [" << uriAnonym << ']');
+                return;
+            }
+
+            parseResponse(httpResponse);
+        };
+
+        httpSession->setFinishedHandler(std::move(finishedCallback));
+        httpSession->asyncRequest(request, *_poll);
+    }
+    else
+    {
+        const std::shared_ptr<const http::Response> httpResponse =
+            httpSession->syncRequest(request, *_poll);
+
+        LOG_TRC("DocumentBroker::sendBrowserSetting with "
+                << requestType << " returned " << httpResponse->statusLine().statusCode());
+
+        const bool failed = (httpResponse->statusLine().statusCode() != http::StatusCode::OK);
+        if (failed)
+        {
+            if (httpResponse->statusLine().statusCode() == http::StatusCode::Forbidden)
+                LOG_ERR("Access denied to [" << uriAnonym << ']');
+            else
+                LOG_ERR("Invalid URI or access denied to [" << uriAnonym << ']');
+            return;
+        }
+
+        parseResponse(httpResponse);
+    }
 }
 
 struct PresetRequest

diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
@@ -548,8 +548,8 @@ class DocumentBroker : public std::enable_shared_from_this<DocumentBroker>
                              const std::string& userSettingsUri,
                              const std::string& presetsPath);
 
-    void asyncSendBrowserSetting(const std::shared_ptr<ClientSession>& session,
-                                 const std::string& browserSettingUri);
+    void sendBrowserSetting(const std::shared_ptr<ClientSession>& session,
+                            const std::string& settingUri, bool async = true);
 
     /// Start an asynchronous Installation of the user presets, e.g. autotexts etc, as
     /// described at userSettingsUri for installation into presetsPath