[Libreoffice-commits] core.git: sd/qa svx/source

[Miklos Vajna (via logerrit)]

 sd/qa/unit/tiledrendering/data/duplicate-undo.odp |binary
 sd/qa/unit/tiledrendering/tiledrendering.cxx      |   50 ++++++++++++++++++++++
 svx/source/svdraw/svdedxv.cxx                     |    6 ++
 3 files changed, 56 insertions(+)

New commits:
commit 014f33066a99488c52d10d8f5ff470ca6e2242f6
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon May 10 14:19:16 2021 +0200
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Mon May 10 15:19:52 2021 +0200

    svx: fix crash with active text edit vs slide delete
    
    The problem is that SdrObjEditView::HideSdrPage() may delete
    SdrPaintView::mpPageView when it calls SdrGlueEditView::HideSdrPage().
    
    But SdrObjEditView::pTextEditPV is a non-owning reference to that.
    
    GetTextEditBackgroundColor() in svx/ calls
    SdrObjEditView::GetTextEditPageView(), so in case
    SdrObjEditView::pTextEditPV is not cleared, we would access a deleted
    SdrPageView.
    
    Change-Id: I948bae8e0e8d557e38aa8f243e9eea522b21a043
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115324
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    Tested-by: Jenkins

diff --git a/sd/qa/unit/tiledrendering/data/duplicate-undo.odp b/sd/qa/unit/tiledrendering/data/duplicate-undo.odp
new file mode 100644
index 000000000000..f66c9f6081d5
Binary files /dev/null and b/sd/qa/unit/tiledrendering/data/duplicate-undo.odp differ
diff --git a/sd/qa/unit/tiledrendering/tiledrendering.cxx b/sd/qa/unit/tiledrendering/tiledrendering.cxx
index c9dbbd74fa14..627ef8e8412a 100644
--- a/sd/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sd/qa/unit/tiledrendering/tiledrendering.cxx
@@ -129,6 +129,7 @@ public:
     void testLanguageAllText();
     void testInsertDeletePageInvalidation();
     void testSpellOnlineRenderParameter();
+    void testSlideDuplicateUndo();
 
     CPPUNIT_TEST_SUITE(SdTiledRenderingTest);
     CPPUNIT_TEST(testCreateDestroy);
@@ -183,6 +184,7 @@ public:
     CPPUNIT_TEST(testLanguageAllText);
     CPPUNIT_TEST(testInsertDeletePageInvalidation);
     CPPUNIT_TEST(testSpellOnlineRenderParameter);
+    CPPUNIT_TEST(testSlideDuplicateUndo);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -2543,6 +2545,54 @@ void SdTiledRenderingTest::testSpellOnlineRenderParameter()
     CPPUNIT_ASSERT_EQUAL(!bSet, pXImpressDocument->GetDoc()->GetOnlineSpell());
 }
 
+void SdTiledRenderingTest::testSlideDuplicateUndo()
+{
+    // Create two views.
+    SdXImpressDocument* pXImpressDocument = createDoc("duplicate-undo.odp");
+    int nView0 = SfxLokHelper::getView();
+    SfxLokHelper::createView();
+    pXImpressDocument->initializeForTiledRendering({});
+    int nView1 = SfxLokHelper::getView();
+    SfxLokHelper::setView(nView0);
+
+    // Switch to the 3rd slide on view 0, and start text editing.
+    {
+        pXImpressDocument->setPart(2);
+        sd::ViewShell* pViewShell0 = pXImpressDocument->GetDocShell()->GetViewShell();
+        SdrView* pView = pViewShell0->GetView();
+        SdPage* pActualPage = pViewShell0->GetActualPage();
+        SdrObject* pObject = pActualPage->GetObj(1);
+        SdrTextObj* pTextObj = static_cast<SdrTextObj*>(pObject);
+        pView->MarkObj(pTextObj, pView->GetSdrPageView());
+        SfxStringItem aInputString(SID_ATTR_CHAR, "x");
+        pViewShell0->GetViewFrame()->GetDispatcher()->ExecuteList(SID_ATTR_CHAR,
+                SfxCallMode::SYNCHRON, { &aInputString });
+        CPPUNIT_ASSERT(pView->IsTextEdit());
+        CPPUNIT_ASSERT(pView->GetTextEditPageView());
+    }
+
+    // Duplicate the first slide on view 1 and undo it.
+    SfxLokHelper::setView(nView1);
+    comphelper::dispatchCommand(".uno:DuplicatePage", {});
+    Scheduler::ProcessEventsToIdle();
+    pXImpressDocument->setPart(0, /*bAllowChangeFocus=*/false);
+    pXImpressDocument->setPart(1, /*bAllowChangeFocus=*/false);
+    SfxLokHelper::setView(nView0);
+    pXImpressDocument->setPart(0, /*bAllowChangeFocus=*/false);
+    pXImpressDocument->setPart(3, /*bAllowChangeFocus=*/false);
+    SfxLokHelper::setView(nView1);
+    pXImpressDocument->getUndoManager()->undo();
+    // Without the accompanying fix in place, this would have tried to access the outdated page view
+    // pointer, potentially leading to a crash.
+    pXImpressDocument->setPart(2, /*bAllowChangeFocus=*/false);
+
+    // Make sure that view 0 now doesn't have an outdated page view pointer.
+    SfxLokHelper::setView(nView0);
+    sd::ViewShell* pViewShell0 = pXImpressDocument->GetDocShell()->GetViewShell();
+    SdrView* pView0 = pViewShell0->GetView();
+    CPPUNIT_ASSERT(!pView0->GetTextEditPageView());
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SdTiledRenderingTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/svx/source/svdraw/svdedxv.cxx b/svx/source/svdraw/svdedxv.cxx
index 95379012be54..bb56488428db 100644
--- a/svx/source/svdraw/svdedxv.cxx
+++ b/svx/source/svdraw/svdedxv.cxx
@@ -195,6 +195,12 @@ void SdrObjEditView::HideSdrPage()
 {
     lcl_RemoveTextEditOutlinerViews(this, GetSdrPageView(), GetFirstOutputDevice());
 
+    if (pTextEditPV == GetSdrPageView())
+    {
+        // HideSdrPage() will clear mpPageView, avoid a dangling pointer.
+        pTextEditPV = nullptr;
+    }
+
     SdrGlueEditView::HideSdrPage();
 }