desktop/source/lib/init.cxx    |    4 +-
 include/sfx2/lokhelper.hxx     |   17 +++++++---
 sfx2/source/view/lokhelper.cxx |   69 ++++++++++++++++++++++++-----------------
 sw/source/core/crsr/crsrsh.cxx |    2 -
 4 files changed, 56 insertions(+), 36 deletions(-)

New commits:
commit 44f6329330e8c8f000cb310b04ac13777e8bfff3
Author:     Ashod Nakashian <ashod.nakash...@collabora.co.uk>
AuthorDate: Fri Jul 24 11:33:46 2020 -0400
Commit:     Tor Lillqvist <t...@collabora.com>
CommitDate: Fri Nov 20 14:34:00 2020 +0100

    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 <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Ashod Nakashian <a...@collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106221
    Tested-by: Jenkins
    Reviewed-by: Tor Lillqvist <t...@collabora.com>

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);
     }
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to