Consistently set document view also for zoom event

And extract this code to a dedicated function.

The problem was, that creating a new view sets the document's current
view shell (in SwViewGlueDocShell); upon destruction, document's view
shell is set to nullptr. In desktop case, document view shell is then
reset to another active shell e.g. inside "grab focus" event, which
makes all following operations safe. But in gtktiledviewer case, the
"grab focus" event is not handled, so setClientZoomInThread resulted
in eventual nullptr dereference.

It was workarounded in commit 25de85bc3ce2d2f9b7736f65492f42579048da27
Date   Fri Oct 21 21:14:04 2022 +0200
nullptr check

but that was not the proper fix.

Possibly it could also be reasonable to start handling "focus-in-event"
[1] in gtktiledviewer; but since just setting the view consistently in
setClientZoomInThread solves the problem, it looks excessive.

This reverts commit 25de85bc3ce2d2f9b7736f65492f42579048da27.

[1] https://docs.gtk.org/gtk3/signal.Widget.focus-in-event.html

Change-Id: Iac4ff4e9b043aec0e0af6ed811c0e7f018378271
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141753
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

1 files changed, 24 insertions, 55 deletions

diff --git a/libreofficekit/source/gtk/lokdocview.cxx b/libreofficekit/source/gtk/lokdocview.cxx
index 0486d16e9213..fba48d9293ef 100644
--- a/libreofficekit/source/gtk/lokdocview.cxx
+++ b/libreofficekit/source/gtk/lokdocview.cxx
@@ -258,6 +258,15 @@ struct LOKDocViewPrivateImpl
     }
 };
 
+// Must be run with g_aLOKMutex locked
+void setDocumentView(LibreOfficeKitDocument* pDoc, int viewId)
+{
+    assert(pDoc);
+    std::stringstream ss;
+    ss << "lok::Document::setView(" << viewId << ")";
+    g_info("%s", ss.str().c_str());
+    pDoc->pClass->setView(pDoc, viewId);
+}
 }
 
 /// Wrapper around LOKDocViewPrivateImpl, managed by malloc/memset/free.
@@ -596,10 +605,8 @@ postKeyEventInThread(gpointer data)
     gint nTileSizePixelsScaled = nTileSizePixels * nScaleFactor;
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
 
     if (priv->m_nTileSizeTwips)
     {
@@ -2172,10 +2179,7 @@ lok_doc_view_signal_motion (GtkWidget* pWidget, GdkEventMotion* pEvent)
     GError* error = nullptr;
 
     std::unique_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     if (priv->m_bInDragMiddleHandle)
     {
         g_info("lcl_signalMotion: dragging the middle handle");
@@ -2273,11 +2277,8 @@ setGraphicSelectionInThread(gpointer data)
     LOEvent* pLOEvent = static_cast<LOEvent*>(g_task_get_task_data(task));
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
-    ss.str(std::string());
     ss << "lok::Document::setGraphicSelection(" << pLOEvent->m_nSetGraphicSelectionType;
     ss << ", " << pLOEvent->m_nSetGraphicSelectionX;
     ss << ", " << pLOEvent->m_nSetGraphicSelectionY << ")";
@@ -2297,6 +2298,7 @@ setClientZoomInThread(gpointer data)
     LOEvent* pLOEvent = static_cast<LOEvent*>(g_task_get_task_data(task));
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     priv->m_pDocument->pClass->setClientZoom(priv->m_pDocument,
                                              pLOEvent->m_nTilePixelWidth,
                                              pLOEvent->m_nTilePixelHeight,
@@ -2313,11 +2315,8 @@ postMouseEventInThread(gpointer data)
     LOEvent* pLOEvent = static_cast<LOEvent*>(g_task_get_task_data(task));
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
-    ss.str(std::string());
     ss << "lok::Document::postMouseEvent(" << pLOEvent->m_nPostMouseEventType;
     ss << ", " << pLOEvent->m_nPostMouseEventX;
     ss << ", " << pLOEvent->m_nPostMouseEventY;
@@ -2379,10 +2378,7 @@ setPartInThread(gpointer data)
     int nPart = pLOEvent->m_nPart;
 
     std::unique_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     priv->m_pDocument->pClass->setPart( priv->m_pDocument, nPart );
     aGuard.unlock();
 
@@ -2399,10 +2395,7 @@ setPartmodeInThread(gpointer data)
     int nPartMode = pLOEvent->m_nPartMode;
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     priv->m_pDocument->pClass->setPartMode( priv->m_pDocument, nPartMode );
 }
 
@@ -2422,10 +2415,7 @@ setEditInThread(gpointer data)
     {
         g_info("lok_doc_view_set_edit: leaving edit mode");
         std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
-        std::stringstream ss;
-        ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-        g_info("%s", ss.str().c_str());
-        priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+        setDocumentView(priv->m_pDocument, priv->m_nViewId);
         priv->m_pDocument->pClass->resetSelection(priv->m_pDocument);
     }
     priv->m_bEdit = bEdit;
@@ -2442,11 +2432,8 @@ postCommandInThread (gpointer data)
     LOKDocViewPrivate& priv = getPrivate(pDocView);
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
-    ss.str(std::string());
     ss << "lok::Document::postUnoCommand(" << pLOEvent->m_pCommand << ", " << pLOEvent->m_pArguments << ")";
     g_info("%s", ss.str().c_str());
     priv->m_pDocument->pClass->postUnoCommand(priv->m_pDocument, pLOEvent->m_pCommand, pLOEvent->m_pArguments, pLOEvent->m_bNotifyWhenFinished);
@@ -2493,11 +2480,8 @@ paintTileInThread (gpointer data)
     aTileRectangle.y = pixelToTwip(nTileSizePixelsScaled, pLOEvent->m_fPaintTileZoom * nScaleFactor) * pLOEvent->m_nPaintTileX;
 
     std::unique_lock<std::mutex> aGuard(g_aLOKMutex);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
-    ss.str(std::string());
     GTimer* aTimer = g_timer_new();
     gulong nElapsedMs;
     ss << "lok::Document::paintTile(" << static_cast<void*>(pBuffer) << ", "
@@ -2771,12 +2755,9 @@ static void lok_doc_view_destroy (GtkWidget* widget)
 
     // Ignore notifications sent to this view on shutdown.
     std::unique_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
     if (priv->m_pDocument)
     {
-        priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+        setDocumentView(priv->m_pDocument, priv->m_nViewId);
         priv->m_pDocument->pClass->registerCallback(priv->m_pDocument, nullptr, nullptr);
     }
 
@@ -3740,10 +3721,7 @@ lok_doc_view_get_parts (LOKDocView* pDocView)
         return -1;
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     return priv->m_pDocument->pClass->getParts( priv->m_pDocument );
 }
 
@@ -3755,10 +3733,7 @@ lok_doc_view_get_part (LOKDocView* pDocView)
         return -1;
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     return priv->m_pDocument->pClass->getPart( priv->m_pDocument );
 }
 
@@ -3802,10 +3777,7 @@ SAL_DLLPUBLIC_EXPORT void lok_doc_view_send_content_control_event(LOKDocView* pD
     }
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::sendContentControlEvent('" << pArguments << "')";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     return priv->m_pDocument->pClass->sendContentControlEvent(priv->m_pDocument, pArguments);
 }
 
@@ -3817,10 +3789,7 @@ lok_doc_view_get_part_name (LOKDocView* pDocView, int nPart)
         return nullptr;
 
     std::scoped_lock<std::mutex> aGuard(g_aLOKMutex);
-    std::stringstream ss;
-    ss << "lok::Document::setView(" << priv->m_nViewId << ")";
-    g_info("%s", ss.str().c_str());
-    priv->m_pDocument->pClass->setView(priv->m_pDocument, priv->m_nViewId);
+    setDocumentView(priv->m_pDocument, priv->m_nViewId);
     return priv->m_pDocument->pClass->getPartName( priv->m_pDocument, nPart );
 }