sfx2: lok: refactor notifications and const correctness

This reduces the stringification and reuses the notificaiton
helpers to reduce code duplication.

Change-Id: Icf9f9c50f3eeee61a0ded741d39fed37cfcc8da1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99972
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ash@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106221
Tested-by: Jenkins
Reviewed-by: Tor Lillqvist <tml@collabora.com>

4 files changed, 56 insertions, 36 deletions

diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index 8f82d577f3cc..6fe7347e9b70 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -2195,7 +2195,7 @@ static LibreOfficeKitDocument* lo_documentLoadWithOptions(LibreOfficeKit* pThis,
     LibLibreOffice_Impl* pLib = static_cast<LibLibreOffice_Impl*>(pThis);
     pLib->maLastExceptionMsg.clear();
 
-    OUString aURL(getAbsoluteURL(pURL));
+    const OUString aURL(getAbsoluteURL(pURL));
     if (aURL.isEmpty())
     {
         pLib->maLastExceptionMsg = "Filename to load was not provided.";
@@ -2291,7 +2291,7 @@ static LibreOfficeKitDocument* lo_documentLoadWithOptions(LibreOfficeKit* pThis,
 
         LibLODocument_Impl* pDocument = new LibLODocument_Impl(xComponent, nDocumentIdCounter++);
 
-        // Do we know that after loading the document, its initial view is the "current" view?
+        // After loading the document, its initial view is the "current" view.
         SfxLokHelper::setDocumentIdOfView(pDocument->mnDocumentId);
         if (pLib->mpCallback)
         {
diff --git a/include/sfx2/lokhelper.hxx b/include/sfx2/lokhelper.hxx
index 79052a0d6e04..2aad968681b8 100644
--- a/include/sfx2/lokhelper.hxx
+++ b/include/sfx2/lokhelper.hxx
@@ -51,7 +51,7 @@ public:
     /// Set a view shell as current one.
     static void setView(int nId);
     /// Get the currently active view.
-    static int getView(SfxViewShell* pViewShell = nullptr);
+    static int getView(const SfxViewShell* pViewShell = nullptr);
     /// Get the number of views of the current object shell.
     static std::size_t getViewsCount();
     /// Get viewIds of views of the current object shell.
@@ -72,17 +72,24 @@ public:
     static LOKDeviceFormFactor getDeviceFormFactor();
     /// Set the device form factor that should be used for a new view.
     static void setDeviceFormFactor(const OUString& rDeviceFormFactor);
+
     /// Iterate over any view shell, except pThisViewShell, passing it to the f function.
     template<typename ViewShellType, typename FunctionType>
     static void forEachOtherView(ViewShellType* pThisViewShell, FunctionType f);
+
     /// Invoke the LOK callback of all other views showing the same document as pThisView, with a payload of rKey-rPayload.
-    static void notifyOtherViews(SfxViewShell* pThisView, int nType, const OString& rKey, const OString& rPayload);
+    static void notifyOtherViews(const SfxViewShell* pThisView, int nType, const OString& rKey,
+                                 const OString& rPayload);
     /// Invoke the LOK callback of all views except pThisView, with a JSON payload created from the given property tree.
-    static void notifyOtherViews(SfxViewShell* pThisView, int nType, const boost::property_tree::ptree& rTree);
+    static void notifyOtherViews(const SfxViewShell* pThisView, int nType,
+                                 const boost::property_tree::ptree& rTree);
     /// Same as notifyOtherViews(), but works on a selected "other" view, not on all of them.
-    static void notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType, const OString& rKey, const OString& rPayload);
+    static void notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView,
+                                int nType, const OString& rKey, const OString& rPayload);
     /// Same as notifyOtherViews(), the property-tree version, but works on a selected "other" view, not on all of them.
-    static void notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType, const boost::property_tree::ptree& rTree);
+    static void notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView,
+                                int nType, const boost::property_tree::ptree& rTree);
+
     /// Emits a LOK_CALLBACK_STATE_CHANGED
     static void sendUnoStatus(const SfxViewShell* pShell, const SfxPoolItem* pItem);
     /// Emits a LOK_CALLBACK_WINDOW
diff --git a/sfx2/source/view/lokhelper.cxx b/sfx2/source/view/lokhelper.cxx
index fa5b7e0bcb2d..1240209d9fb2 100644
--- a/sfx2/source/view/lokhelper.cxx
+++ b/sfx2/source/view/lokhelper.cxx
@@ -74,7 +74,7 @@ LOKDeviceFormFactor g_deviceFormFactor = LOKDeviceFormFactor::UNKNOWN;
 int SfxLokHelper::createView()
 {
     SfxViewFrame* pViewFrame = SfxViewFrame::GetFirst();
-    if (!pViewFrame)
+    if (pViewFrame == nullptr)
         return -1;
     SfxViewShell* pPrevViewShell = SfxViewShell::Current();
     ViewShellDocId nId;
@@ -83,7 +83,7 @@ int SfxLokHelper::createView()
     SfxRequest aRequest(pViewFrame, SID_NEWWINDOW);
     pViewFrame->ExecView_Impl(aRequest);
     SfxViewShell* pViewShell = SfxViewShell::Current();
-    if (!pViewShell)
+    if (pViewShell == nullptr)
         return -1;
     if (pPrevViewShell)
         pViewShell->SetDocId(nId);
@@ -93,15 +93,15 @@ int SfxLokHelper::createView()
 void SfxLokHelper::destroyView(int nId)
 {
     SfxApplication* pApp = SfxApplication::Get();
-    if (!pApp)
+    if (pApp == nullptr)
         return;
 
-    int nViewShellId = nId;
+    const ViewShellId nViewShellId(nId);
     SfxViewShellArr_Impl& rViewArr = pApp->GetViewShells_Impl();
 
-    for (SfxViewShell* pViewShell : rViewArr)
+    for (const SfxViewShell* pViewShell : rViewArr)
     {
-        if (static_cast<sal_Int32>(pViewShell->GetViewShellId()) == nViewShellId)
+        if (pViewShell->GetViewShellId() == nViewShellId)
         {
             SfxViewFrame* pViewFrame = pViewShell->GetViewFrame();
             SfxRequest aRequest(pViewFrame, SID_CLOSEWIN);
@@ -114,15 +114,15 @@ void SfxLokHelper::destroyView(int nId)
 void SfxLokHelper::setView(int nId)
 {
     SfxApplication* pApp = SfxApplication::Get();
-    if (!pApp)
+    if (pApp == nullptr)
         return;
 
-    int nViewShellId = nId;
+    const ViewShellId nViewShellId(nId);
     SfxViewShellArr_Impl& rViewArr = pApp->GetViewShells_Impl();
 
-    for (SfxViewShell* pViewShell : rViewArr)
+    for (const SfxViewShell* pViewShell : rViewArr)
     {
-        if (static_cast<sal_Int32>(pViewShell->GetViewShellId()) == nViewShellId)
+        if (pViewShell->GetViewShellId() == nViewShellId)
         {
             DisableCallbacks dc;
 
@@ -146,7 +146,7 @@ void SfxLokHelper::setView(int nId)
 
 }
 
-int SfxLokHelper::getView(SfxViewShell* pViewShell)
+int SfxLokHelper::getView(const SfxViewShell* pViewShell)
 {
     if (!pViewShell)
         pViewShell = SfxViewShell::Current();
@@ -291,38 +291,50 @@ static OString lcl_escapeQuotes(const OString &rStr)
     return aBuf.makeStringAndClear();
 }
 
-static OString lcl_generateJSON(SfxViewShell* pView, const boost::property_tree::ptree& rTree)
+static OString lcl_generateJSON(const SfxViewShell* pView, const boost::property_tree::ptree& rTree)
 {
+    assert(pView != nullptr && "pView must be valid");
     boost::property_tree::ptree aMessageProps = rTree;
     aMessageProps.put("viewId", SfxLokHelper::getView(pView));
     aMessageProps.put("part", pView->getPart());
     std::stringstream aStream;
     boost::property_tree::write_json(aStream, aMessageProps, false /* pretty */);
-    return OString(aStream.str().c_str()).trim();
+    const std::string aString = aStream.str();
+    return OString(aString.c_str(), aString.size()).trim();
 }
 
-void SfxLokHelper::notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType, const OString& rKey, const OString& rPayload)
+static inline OString lcl_generateJSON(const SfxViewShell* pView, const OString& rKey,
+                                       const OString& rPayload)
 {
+    assert(pView != nullptr && "pView must be valid");
+    return OStringLiteral("{ \"viewId\": \"") + OString::number(SfxLokHelper::getView(pView))
+           + "\", \"part\": \"" + OString::number(pView->getPart()) + "\", \"" + rKey + "\": \""
+           + lcl_escapeQuotes(rPayload) + "\" }";
+}
+
+void SfxLokHelper::notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView,
+                                   int nType, const OString& rKey, const OString& rPayload)
+{
+    assert(pThisView != nullptr && "pThisView must be valid");
     if (DisableCallbacks::disabled())
         return;
 
-    OString aPayload = OStringLiteral("{ \"viewId\": \"") + OString::number(SfxLokHelper::getView(pThisView)) +
-                       "\", \"part\": \"" + OString::number(pThisView->getPart()) +
-                       "\", \"" + rKey + "\": \"" + lcl_escapeQuotes(rPayload) + "\" }";
-
+    const OString aPayload = lcl_generateJSON(pThisView, rKey, rPayload);
     pOtherView->libreOfficeKitViewCallback(nType, aPayload.getStr());
 }
 
-void SfxLokHelper::notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType,
-                                   const boost::property_tree::ptree& rTree)
+void SfxLokHelper::notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView,
+                                   int nType, const boost::property_tree::ptree& rTree)
 {
+    assert(pThisView != nullptr && "pThisView must be valid");
     if (DisableCallbacks::disabled())
         return;
 
     pOtherView->libreOfficeKitViewCallback(nType, lcl_generateJSON(pThisView, rTree).getStr());
 }
 
-void SfxLokHelper::notifyOtherViews(SfxViewShell* pThisView, int nType, const OString& rKey, const OString& rPayload)
+void SfxLokHelper::notifyOtherViews(const SfxViewShell* pThisView, int nType, const OString& rKey,
+                                    const OString& rPayload)
 {
     if (DisableCallbacks::disabled())
         return;
@@ -337,7 +349,8 @@ void SfxLokHelper::notifyOtherViews(SfxViewShell* pThisView, int nType, const OS
     }
 }
 
-void SfxLokHelper::notifyOtherViews(SfxViewShell* pThisView, int nType, const boost::property_tree::ptree& rTree)
+void SfxLokHelper::notifyOtherViews(const SfxViewShell* pThisView, int nType,
+                                    const boost::property_tree::ptree& rTree)
 {
     if (SfxLokHelper::getViewsCount() <= 1 || DisableCallbacks::disabled())
         return;
@@ -399,26 +412,26 @@ void SfxLokHelper::notifyWindow(const SfxViewShell* pThisView,
                                 const OUString& rAction,
                                 const std::vector<vcl::LOKPayloadItem>& rPayload)
 {
-    assert(pThisView);
+    assert(pThisView != nullptr && "pThisView must be valid");
 
     if (SfxLokHelper::getViewsCount() <= 0 || nLOKWindowId == 0 || DisableCallbacks::disabled())
         return;
 
     OStringBuffer aPayload;
-    aPayload.append("{ \"id\": \"").append(OString::number(nLOKWindowId)).append("\"");
-    aPayload.append(", \"action\": \"").append(OUStringToOString(rAction, RTL_TEXTENCODING_UTF8)).append("\"");
+    aPayload.append("{ \"id\": \"").append(OString::number(nLOKWindowId)).append('"');
+    aPayload.append(", \"action\": \"").append(OUStringToOString(rAction, RTL_TEXTENCODING_UTF8)).append('"');
 
     for (const auto& rItem: rPayload)
     {
         if (!rItem.first.isEmpty() && !rItem.second.isEmpty())
         {
             aPayload.append(", \"").append(rItem.first).append("\": \"")
-                .append(rItem.second).append("\"");
+                .append(rItem.second).append('"');
         }
     }
-    aPayload.append("}");
+    aPayload.append('}');
 
-    auto s = aPayload.makeStringAndClear();
+    const OString s = aPayload.makeStringAndClear();
     pThisView->libreOfficeKitViewCallback(LOK_CALLBACK_WINDOW, s.getStr());
 }
 
diff --git a/sw/source/core/crsr/crsrsh.cxx b/sw/source/core/crsr/crsrsh.cxx
index 4e89e80d711b..1aba8c3ac22f 100644
--- a/sw/source/core/crsr/crsrsh.cxx
+++ b/sw/source/core/crsr/crsrsh.cxx
@@ -2394,7 +2394,7 @@ void SwCursorShell::ShowCursor()
 
     if (comphelper::LibreOfficeKit::isActive())
     {
-        OString aPayload = OString::boolean(m_bSVCursorVis);
+        const OString aPayload = OString::boolean(m_bSVCursorVis);
         GetSfxViewShell()->libreOfficeKitViewCallback(LOK_CALLBACK_CURSOR_VISIBLE, aPayload.getStr());
         SfxLokHelper::notifyOtherViews(GetSfxViewShell(), LOK_CALLBACK_VIEW_CURSOR_VISIBLE, "visible", aPayload);
     }