LOK: change type of view ids to uintptr_t

This fixes the following problem:

- createView() = 1
- createView() = 2
- destroyView(1)

and then view #2 was renumbered to 1.

Instead expose the pointer address of the SfxViewShell as the ID, which
is not changing in such a situation.

Note that the SfxViewShell <-> ID mapping is an implementation detail of
SfxLokHelper, and only pointers are converted to IDs, user-supplied IDs
are never converted back to pointers.

Change-Id: If79ef8b99ba391011b5d82b219ad13447d44cd5a
Reviewed-on: https://gerrit.libreoffice.org/26423
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Jenkins <ci@libreoffice.org>

desktop/qa/desktop_lib/test_desktop_lib.cxx

@@ -295,16 +295,17 @@ void DesktopLOKTest::testCreateView()
     LibLODocument_Impl* pDocument = loadDoc("blank_text.odt");
     CPPUNIT_ASSERT_EQUAL(1, pDocument->m_pDocumentClass->getViews(pDocument));
 
-    int nId = pDocument->m_pDocumentClass->createView(pDocument);
+    std::uintptr_t nId0 = pDocument->m_pDocumentClass->getView(pDocument);
+    std::uintptr_t 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
     // one.
-    CPPUNIT_ASSERT_EQUAL(1, pDocument->m_pDocumentClass->getView(pDocument));
-    pDocument->m_pDocumentClass->setView(pDocument, 0);
-    CPPUNIT_ASSERT_EQUAL(0, pDocument->m_pDocumentClass->getView(pDocument));
+    CPPUNIT_ASSERT_EQUAL(nId1, pDocument->m_pDocumentClass->getView(pDocument));
+    pDocument->m_pDocumentClass->setView(pDocument, nId0);
+    CPPUNIT_ASSERT_EQUAL(nId0, pDocument->m_pDocumentClass->getView(pDocument));
 
-    pDocument->m_pDocumentClass->destroyView(pDocument, nId);
+    pDocument->m_pDocumentClass->destroyView(pDocument, nId1);
     CPPUNIT_ASSERT_EQUAL(1, pDocument->m_pDocumentClass->getViews(pDocument));
 }
 

desktop/source/lib/init.cxx

@@ -364,10 +364,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 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 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_getViews(LibreOfficeKitDocument* pThis);
 static unsigned char* doc_renderFont(LibreOfficeKitDocument* pThis,
                           const char *pFontName,
@@ -1850,28 +1850,28 @@ static void doc_setClientVisibleArea(LibreOfficeKitDocument* pThis, int nX, int
     pDoc->setClientVisibleArea(aRectangle);
 }
 
-static int doc_createView(LibreOfficeKitDocument* /*pThis*/)
+static uintptr_t doc_createView(LibreOfficeKitDocument* /*pThis*/)
 {
     SolarMutexGuard aGuard;
 
     return SfxLokHelper::createView();
 }
 
-static void doc_destroyView(LibreOfficeKitDocument* /*pThis*/, int nId)
+static void doc_destroyView(LibreOfficeKitDocument* /*pThis*/, uintptr_t nId)
 {
     SolarMutexGuard aGuard;
 
     SfxLokHelper::destroyView(nId);
 }
 
-static void doc_setView(LibreOfficeKitDocument* /*pThis*/, int nId)
+static void doc_setView(LibreOfficeKitDocument* /*pThis*/, uintptr_t nId)
 {
     SolarMutexGuard aGuard;
 
     SfxLokHelper::setView(nId);
 }
 
-static int doc_getView(LibreOfficeKitDocument* /*pThis*/)
+static uintptr_t doc_getView(LibreOfficeKitDocument* /*pThis*/)
 {
     SolarMutexGuard aGuard;
 

include/LibreOfficeKit/LibreOfficeKit.h

@@ -209,13 +209,13 @@ struct _LibreOfficeKitDocumentClass
     void (*setClientVisibleArea) (LibreOfficeKitDocument* pThis, int nX, int nY, int nWidth, int nHeight);
 
     /// @see lok::Document::createView().
-    int (*createView) (LibreOfficeKitDocument* pThis);
+    uintptr_t (*createView) (LibreOfficeKitDocument* pThis);
     /// @see lok::Document::destroyView().
-    void (*destroyView) (LibreOfficeKitDocument* pThis, int nId);
+    void (*destroyView) (LibreOfficeKitDocument* pThis, uintptr_t nId);
     /// @see lok::Document::setView().
-    void (*setView) (LibreOfficeKitDocument* pThis, int nId);
+    void (*setView) (LibreOfficeKitDocument* pThis, uintptr_t nId);
     /// @see lok::Document::getView().
-    int (*getView) (LibreOfficeKitDocument* pThis);
+    uintptr_t (*getView) (LibreOfficeKitDocument* pThis);
     /// @see lok::Document::getViews().
     int (*getViews) (LibreOfficeKitDocument* pThis);
 

include/LibreOfficeKit/LibreOfficeKit.hxx

@@ -363,7 +363,7 @@ public:
      * By default a loaded document has 1 view.
      * @return the ID of the new view.
      */
-    int createView()
+    uintptr_t createView()
     {
         return mpDoc->pClass->createView(mpDoc);
     }
@@ -372,7 +372,7 @@ public:
      * Destroy a view of an existing document.
      * @param nId a view ID, returned by createView().
      */
-    void destroyView(int nId)
+    void destroyView(uintptr_t nId)
     {
         mpDoc->pClass->destroyView(mpDoc, nId);
     }
@@ -381,7 +381,7 @@ public:
      * Set an existing view of an existing document as current.
      * @param nId a view ID, returned by createView().
      */
-    void setView(int nId)
+    void setView(uintptr_t nId)
     {
         mpDoc->pClass->setView(mpDoc, nId);
     }
@@ -390,7 +390,7 @@ public:
      * Get the current view.
      * @return a view ID, previously returned by createView().
      */
-    int getView()
+    uintptr_t getView()
     {
         return mpDoc->pClass->getView(mpDoc);
     }

include/sfx2/lokhelper.hxx

@@ -12,6 +12,7 @@
 
 #include <sfx2/dllapi.h>
 #include <cstddef>
+#include <cstdint>
 
 class SfxViewShell;
 
@@ -19,13 +20,13 @@ class SFX2_DLLPUBLIC SfxLokHelper
 {
 public:
     /// Create a new view shell from the current view frame.
-    static int createView();
+    static std::uintptr_t createView();
     /// Destroy a view shell from the global shell list.
-    static void destroyView(std::size_t nId);
+    static void destroyView(std::uintptr_t nId);
     /// Set a view shell as current one.
-    static void setView(std::size_t nId);
+    static void setView(std::uintptr_t nId);
     /// Get the currently active view.
-    static std::size_t getView();
+    static std::uintptr_t getView();
     /// Get the number of views of the current object shell.
     static std::size_t getViews();
 };

libreofficekit/source/gtk/lokdocview.cxx

@@ -127,7 +127,7 @@ struct LOKDocViewPrivateImpl
     ///@}
 
     /// View ID, returned by createView() or 0 by default.
-    int m_nViewId;
+    std::uintptr_t m_nViewId;
 
     /**
      * Contains a freshly set zoom level: logic size of a tile.
@@ -837,6 +837,7 @@ static gboolean postDocumentLoad(gpointer pData)
     g_info("%s", ss.str().c_str());
     priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
     priv->m_pDocument->pClass->initializeForRendering(priv->m_pDocument, priv->m_aRenderingArguments.c_str());
+    priv->m_nViewId = priv->m_pDocument->pClass->getView(priv->m_pDocument);
     priv->m_pDocument->pClass->registerCallback(priv->m_pDocument, callbackWorker, pLOKDocView);
     priv->m_pDocument->pClass->getDocumentSize(priv->m_pDocument, &priv->m_nDocumentWidthTwips, &priv->m_nDocumentHeightTwips);
     priv->m_nParts = priv->m_pDocument->pClass->getParts(priv->m_pDocument);
@@ -2691,7 +2692,9 @@ SAL_DLLPUBLIC_EXPORT GtkWidget* lok_doc_view_new_from_widget(LOKDocView* pOldLOK
     // No documentLoad(), just a createView().
     LibreOfficeKitDocument* pDocument = lok_doc_view_get_document(LOK_DOC_VIEW(pNewDocView));
     LOKDocViewPrivate& pNewPriv = getPrivate(LOK_DOC_VIEW(pNewDocView));
-    pNewPriv->m_nViewId = pDocument->pClass->createView(pDocument);
+    // Store the view id only later in postDocumentLoad(), as
+    // initializeForRendering() changes the id in Impress.
+    pDocument->pClass->createView(pDocument);
     pNewPriv->m_aRenderingArguments = pOldPriv->m_aRenderingArguments;
 
     postDocumentLoad(pNewDocView);

sfx2/source/view/lokhelper.cxx

@@ -14,54 +14,55 @@
 
 #include <shellimpl.hxx>
 
-int SfxLokHelper::createView()
+std::uintptr_t SfxLokHelper::createView()
 {
     SfxViewFrame* pViewFrame = SfxViewFrame::Current();
     SfxRequest aRequest(pViewFrame, SID_NEWWINDOW);
     pViewFrame->ExecView_Impl(aRequest);
 
-    // The SfxViewShell ctor always puts the view shell to the end of the vector.
-    SfxViewShellArr_Impl& rViewArr = SfxGetpApp()->GetViewShells_Impl();
-    return rViewArr.size() - 1;
+    return reinterpret_cast<std::uintptr_t>(SfxViewShell::Current());
 }
 
-void SfxLokHelper::destroyView(std::size_t nId)
+void SfxLokHelper::destroyView(std::uintptr_t nId)
 {
     SfxViewShellArr_Impl& rViewArr = SfxGetpApp()->GetViewShells_Impl();
-    if (nId > rViewArr.size() - 1)
-        return;
 
-    SfxViewShell* pViewShell = rViewArr[nId];
-    SfxViewFrame* pViewFrame = pViewShell->GetViewFrame();
-    SfxRequest aRequest(pViewFrame, SID_CLOSEWIN);
-    pViewFrame->Exec_Impl(aRequest);
-}
-
-void SfxLokHelper::setView(std::size_t nId)
-{
-    SfxViewShellArr_Impl& rViewArr = SfxGetpApp()->GetViewShells_Impl();
-    if (nId > rViewArr.size() - 1)
-        return;
-
-    SfxViewShell* pViewShell = rViewArr[nId];
-    if (pViewShell->GetViewFrame() == SfxViewFrame::Current())
-        return;
-
-    if (SfxViewFrame* pViewFrame = pViewShell->GetViewFrame())
-        pViewFrame->MakeActive_Impl(false);
-}
-
-std::size_t SfxLokHelper::getView()
-{
-    SfxViewShellArr_Impl& rViewArr = SfxGetpApp()->GetViewShells_Impl();
-    SfxViewFrame* pViewFrame = SfxViewFrame::Current();
     for (std::size_t i = 0; i < rViewArr.size(); ++i)
     {
-        if (rViewArr[i]->GetViewFrame() == pViewFrame)
-            return i;
+        SfxViewShell* pViewShell = rViewArr[i];
+        if (reinterpret_cast<std::uintptr_t>(pViewShell) == nId)
+        {
+            SfxViewFrame* pViewFrame = pViewShell->GetViewFrame();
+            SfxRequest aRequest(pViewFrame, SID_CLOSEWIN);
+            pViewFrame->Exec_Impl(aRequest);
+            break;
+        }
     }
-    assert(false);
-    return 0;
+}
+
+void SfxLokHelper::setView(std::uintptr_t 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 (pViewShell == SfxViewShell::Current())
+                return;
+
+            SfxViewFrame* pViewFrame = pViewShell->GetViewFrame();
+            pViewFrame->MakeActive_Impl(false);
+            return;
+        }
+    }
+
+}
+
+std::uintptr_t SfxLokHelper::getView()
+{
+    return reinterpret_cast<std::uintptr_t>(SfxViewShell::Current());
 }
 
 std::size_t SfxLokHelper::getViews()