[Libreoffice-commits] core.git: desktop/qa desktop/source include/LibreOfficeKit include/sfx2 libreofficekit/source sfx2/source

Miklos Vajna vmiklos at collabora.co.uk
Thu Jun 30 06:16:55 UTC 2016


 desktop/qa/desktop_lib/test_desktop_lib.cxx |    4 +--
 desktop/source/lib/init.cxx                 |   16 ++++++------
 include/LibreOfficeKit/LibreOfficeKit.h     |    8 +++---
 include/sfx2/lokhelper.hxx                  |    9 +++----
 libreofficekit/source/gtk/lokdocview.cxx    |   16 ++++++------
 sfx2/source/view/lokhelper.cxx              |   35 +++++++++++++++++++++-------
 6 files changed, 53 insertions(+), 35 deletions(-)

New commits:
commit 615c37503cffa92a663245d7cb140f316ace0506
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Wed Jun 29 17:27:04 2016 +0200

    LOK: change back type of view ids to int
    
    Commit 45c2410041c48c22bd860efb42d4daadad7869b0 (LOK: change type of
    view ids to uintptr_t, 2016-06-17) fixed the problem of view IDs being
    reused for the price of random IDs, which makes debugging harder.
    
    Implement a simple shellToView() function that makes sure view IDs are
    not reused, and stop exposing view shell pointer addresses, which allows
    reverting the LOK API change.
    
    Change-Id: I63089e6de08ee7e1c7706757d43a11f6cf4d6e06
    Reviewed-on: https://gerrit.libreoffice.org/26773
    Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
    Tested-by: Jenkins <ci at libreoffice.org>

diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index d341c76..efe8601 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -295,8 +295,8 @@ void DesktopLOKTest::testCreateView()
     LibLODocument_Impl* pDocument = loadDoc("blank_text.odt");
     CPPUNIT_ASSERT_EQUAL(1, pDocument->m_pDocumentClass->getViews(pDocument));
 
-    std::uintptr_t nId0 = pDocument->m_pDocumentClass->getView(pDocument);
-    std::uintptr_t nId1 = pDocument->m_pDocumentClass->createView(pDocument);
+    int nId0 = pDocument->m_pDocumentClass->getView(pDocument);
+    int nId1 = pDocument->m_pDocumentClass->createView(pDocument);
     CPPUNIT_ASSERT_EQUAL(2, pDocument->m_pDocumentClass->getViews(pDocument));
 
     // Make sure the created view is the active one, then switch to the old
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index ab765dc..10f3430 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -365,10 +365,10 @@ static void doc_setClientZoom(LibreOfficeKitDocument* pThis,
                                     int nTileTwipWidth,
                                     int nTileTwipHeight);
 static void doc_setClientVisibleArea(LibreOfficeKitDocument* pThis, int nX, int nY, int nWidth, int nHeight);
-static uintptr_t doc_createView(LibreOfficeKitDocument* pThis);
-static void doc_destroyView(LibreOfficeKitDocument* pThis, uintptr_t nId);
-static void doc_setView(LibreOfficeKitDocument* pThis, uintptr_t nId);
-static uintptr_t doc_getView(LibreOfficeKitDocument* pThis);
+static int doc_createView(LibreOfficeKitDocument* pThis);
+static void doc_destroyView(LibreOfficeKitDocument* pThis, int nId);
+static void doc_setView(LibreOfficeKitDocument* pThis, int nId);
+static int doc_getView(LibreOfficeKitDocument* pThis);
 static int doc_getViews(LibreOfficeKitDocument* pThis);
 static unsigned char* doc_renderFont(LibreOfficeKitDocument* pThis,
                           const char *pFontName,
@@ -1855,28 +1855,28 @@ static void doc_setClientVisibleArea(LibreOfficeKitDocument* pThis, int nX, int
     pDoc->setClientVisibleArea(aRectangle);
 }
 
-static uintptr_t doc_createView(LibreOfficeKitDocument* /*pThis*/)
+static int doc_createView(LibreOfficeKitDocument* /*pThis*/)
 {
     SolarMutexGuard aGuard;
 
     return SfxLokHelper::createView();
 }
 
-static void doc_destroyView(LibreOfficeKitDocument* /*pThis*/, uintptr_t nId)
+static void doc_destroyView(LibreOfficeKitDocument* /*pThis*/, int nId)
 {
     SolarMutexGuard aGuard;
 
     SfxLokHelper::destroyView(nId);
 }
 
-static void doc_setView(LibreOfficeKitDocument* /*pThis*/, uintptr_t nId)
+static void doc_setView(LibreOfficeKitDocument* /*pThis*/, int nId)
 {
     SolarMutexGuard aGuard;
 
     SfxLokHelper::setView(nId);
 }
 
-static uintptr_t doc_getView(LibreOfficeKitDocument* /*pThis*/)
+static int doc_getView(LibreOfficeKitDocument* /*pThis*/)
 {
     SolarMutexGuard aGuard;
 
diff --git a/include/LibreOfficeKit/LibreOfficeKit.h b/include/LibreOfficeKit/LibreOfficeKit.h
index 559d28a..81d65c1 100644
--- a/include/LibreOfficeKit/LibreOfficeKit.h
+++ b/include/LibreOfficeKit/LibreOfficeKit.h
@@ -212,13 +212,13 @@ struct _LibreOfficeKitDocumentClass
     void (*setClientVisibleArea) (LibreOfficeKitDocument* pThis, int nX, int nY, int nWidth, int nHeight);
 
     /// @see lok::Document::createView().
-    uintptr_t (*createView) (LibreOfficeKitDocument* pThis);
+    int (*createView) (LibreOfficeKitDocument* pThis);
     /// @see lok::Document::destroyView().
-    void (*destroyView) (LibreOfficeKitDocument* pThis, uintptr_t nId);
+    void (*destroyView) (LibreOfficeKitDocument* pThis, int nId);
     /// @see lok::Document::setView().
-    void (*setView) (LibreOfficeKitDocument* pThis, uintptr_t nId);
+    void (*setView) (LibreOfficeKitDocument* pThis, int nId);
     /// @see lok::Document::getView().
-    uintptr_t (*getView) (LibreOfficeKitDocument* pThis);
+    int (*getView) (LibreOfficeKitDocument* pThis);
     /// @see lok::Document::getViews().
     int (*getViews) (LibreOfficeKitDocument* pThis);
 
diff --git a/include/sfx2/lokhelper.hxx b/include/sfx2/lokhelper.hxx
index 4cfe081..e7dfed4 100644
--- a/include/sfx2/lokhelper.hxx
+++ b/include/sfx2/lokhelper.hxx
@@ -12,7 +12,6 @@
 
 #include <sfx2/dllapi.h>
 #include <cstddef>
-#include <cstdint>
 #include <rtl/string.hxx>
 
 class SfxViewShell;
@@ -21,13 +20,13 @@ class SFX2_DLLPUBLIC SfxLokHelper
 {
 public:
     /// Create a new view shell from the current view frame.
-    static std::uintptr_t createView();
+    static int createView();
     /// Destroy a view shell from the global shell list.
-    static void destroyView(std::uintptr_t nId);
+    static void destroyView(int nId);
     /// Set a view shell as current one.
-    static void setView(std::uintptr_t nId);
+    static void setView(int nId);
     /// Get the currently active view.
-    static std::uintptr_t getView(SfxViewShell* pViewShell = nullptr);
+    static int getView(SfxViewShell* pViewShell = nullptr);
     /// Get the number of views of the current object shell.
     static std::size_t getViews();
 
diff --git a/libreofficekit/source/gtk/lokdocview.cxx b/libreofficekit/source/gtk/lokdocview.cxx
index 5510cde..c27a1f9 100644
--- a/libreofficekit/source/gtk/lokdocview.cxx
+++ b/libreofficekit/source/gtk/lokdocview.cxx
@@ -78,7 +78,7 @@ struct LOKDocViewPrivateImpl
     GdkRectangle m_aVisibleCursor;
     /// Position and size of the view cursors. The current view can only see
     /// them, can't modify them. Key is the view id.
-    std::map<std::uintptr_t, GdkRectangle> m_aViewCursors;
+    std::map<int, GdkRectangle> m_aViewCursors;
     /// Cursor overlay is visible or hidden (for blinking).
     gboolean m_bCursorOverlayVisible;
     /// Cursor is visible or hidden (e.g. for graphic selection).
@@ -95,7 +95,7 @@ struct LOKDocViewPrivateImpl
     std::vector<GdkRectangle> m_aTextSelectionRectangles;
     /// Rectangles of view selections. The current view can only see
     /// them, can't modify them. Key is the view id.
-    std::map<std::uintptr_t, std::vector<GdkRectangle>> m_aTextViewSelectionRectangles;
+    std::map<int, std::vector<GdkRectangle>> m_aTextViewSelectionRectangles;
     /// Position and size of the selection start (as if there would be a cursor caret there).
     GdkRectangle m_aTextSelectionStart;
     /// Position and size of the selection end.
@@ -137,7 +137,7 @@ struct LOKDocViewPrivateImpl
     ///@}
 
     /// View ID, returned by createView() or 0 by default.
-    std::uintptr_t m_nViewId;
+    int m_nViewId;
 
     /**
      * Contains a freshly set zoom level: logic size of a tile.
@@ -1180,7 +1180,7 @@ callback (gpointer pData)
         std::stringstream aStream(pCallback->m_aPayload);
         boost::property_tree::ptree aTree;
         boost::property_tree::read_json(aStream, aTree);
-        std::uintptr_t nViewId = aTree.get<std::uintptr_t>("viewId");
+        int nViewId = aTree.get<int>("viewId");
         const std::string& rRectangle = aTree.get<std::string>("rectangle");
         priv->m_aViewCursors[nViewId] = payloadToRectangle(pDocView, rRectangle.c_str());
         gtk_widget_queue_draw(GTK_WIDGET(pDocView));
@@ -1191,7 +1191,7 @@ callback (gpointer pData)
         std::stringstream aStream(pCallback->m_aPayload);
         boost::property_tree::ptree aTree;
         boost::property_tree::read_json(aStream, aTree);
-        std::uintptr_t nViewId = aTree.get<std::uintptr_t>("viewId");
+        int nViewId = aTree.get<int>("viewId");
         const std::string& rSelection = aTree.get<std::string>("selection");
         priv->m_aTextViewSelectionRectangles[nViewId] = payloadToRectangles(pDocView, rSelection.c_str());
         gtk_widget_queue_draw(GTK_WIDGET(pDocView));
@@ -1434,9 +1434,9 @@ renderDocument(LOKDocView* pDocView, cairo_t* pCairo)
     return FALSE;
 }
 
-static const GdkRGBA& getDarkColor(std::uintptr_t nViewId)
+static const GdkRGBA& getDarkColor(int nViewId)
 {
-    static std::map<std::uintptr_t, GdkRGBA> aColorMap;
+    static std::map<int, GdkRGBA> aColorMap;
     auto it = aColorMap.find(nViewId);
     if (it != aColorMap.end())
         return it->second;
@@ -1556,7 +1556,7 @@ renderOverlay(LOKDocView* pDocView, cairo_t* pCairo)
     }
 
     // Selections of other views.
-    for (std::pair<const std::uintptr_t, std::vector<GdkRectangle>>& rPair : priv->m_aTextViewSelectionRectangles)
+    for (std::pair<const int, std::vector<GdkRectangle>>& rPair : priv->m_aTextViewSelectionRectangles)
     {
         for (GdkRectangle& rRectangle : rPair.second)
         {
diff --git a/sfx2/source/view/lokhelper.cxx b/sfx2/source/view/lokhelper.cxx
index b44392c..653dd7f 100644
--- a/sfx2/source/view/lokhelper.cxx
+++ b/sfx2/source/view/lokhelper.cxx
@@ -17,23 +17,42 @@
 
 #include <shellimpl.hxx>
 
-std::uintptr_t SfxLokHelper::createView()
+namespace
+{
+
+/// Assigns a view ID to a view shell.
+int shellToView(SfxViewShell* pViewShell)
+{
+    // Deleted view shells are not removed from this map, so view IDs are not
+    // reused when deleting, then creating a view.
+    static std::map<SfxViewShell*, int> aViewMap;
+    auto it = aViewMap.find(pViewShell);
+    if (it != aViewMap.end())
+        return it->second;
+
+    int nViewId = aViewMap.size();
+    aViewMap[pViewShell] = nViewId;
+    return nViewId;
+}
+}
+
+int SfxLokHelper::createView()
 {
     SfxViewFrame* pViewFrame = SfxViewFrame::Current();
     SfxRequest aRequest(pViewFrame, SID_NEWWINDOW);
     pViewFrame->ExecView_Impl(aRequest);
 
-    return reinterpret_cast<std::uintptr_t>(SfxViewShell::Current());
+    return shellToView(SfxViewShell::Current());
 }
 
-void SfxLokHelper::destroyView(std::uintptr_t nId)
+void SfxLokHelper::destroyView(int nId)
 {
     SfxViewShellArr_Impl& rViewArr = SfxGetpApp()->GetViewShells_Impl();
 
     for (std::size_t i = 0; i < rViewArr.size(); ++i)
     {
         SfxViewShell* pViewShell = rViewArr[i];
-        if (reinterpret_cast<std::uintptr_t>(pViewShell) == nId)
+        if (shellToView(pViewShell) == nId)
         {
             SfxViewFrame* pViewFrame = pViewShell->GetViewFrame();
             SfxRequest aRequest(pViewFrame, SID_CLOSEWIN);
@@ -43,14 +62,14 @@ void SfxLokHelper::destroyView(std::uintptr_t nId)
     }
 }
 
-void SfxLokHelper::setView(std::uintptr_t nId)
+void SfxLokHelper::setView(int nId)
 {
     SfxViewShellArr_Impl& rViewArr = SfxGetpApp()->GetViewShells_Impl();
 
     for (std::size_t i = 0; i < rViewArr.size(); ++i)
     {
         SfxViewShell* pViewShell = rViewArr[i];
-        if (reinterpret_cast<std::uintptr_t>(pViewShell) == nId)
+        if (shellToView(pViewShell) == nId)
         {
             if (pViewShell == SfxViewShell::Current())
                 return;
@@ -63,11 +82,11 @@ void SfxLokHelper::setView(std::uintptr_t nId)
 
 }
 
-std::uintptr_t SfxLokHelper::getView(SfxViewShell* pViewShell)
+int SfxLokHelper::getView(SfxViewShell* pViewShell)
 {
     if (!pViewShell)
         pViewShell = SfxViewShell::Current();
-    return reinterpret_cast<std::uintptr_t>(pViewShell);
+    return shellToView(pViewShell);
 }
 
 std::size_t SfxLokHelper::getViews()


More information about the Libreoffice-commits mailing list