[Libreoffice-commits] core.git: sc/source

[Stephan Bergmann (via logerrit)]

 sc/source/ui/Accessibility/AccessibleDocumentPagePreview.cxx |   25 +++++++++--
 1 file changed, 21 insertions(+), 4 deletions(-)

New commits:
commit 23cfd3d1004f5ddda8cd79878c00b6c64f20068c
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Thu Dec 12 16:56:52 2019 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Thu Dec 12 18:25:44 2019 +0100

    ScShapeChild has broken copy functions
    
    ...as its dtor calls mpAccShape->dispose() on the copied pointer member, thus
    potentially calling dispose too early (and multiple times, but which in itself
    should be harmless).  The copy functions are used when vectors of ScShapeChild
    (as used in ScShapeRange) get sorted with std::stort (which requires
    ScShapeChild to be moveable, but as the move functions are implicitly deleted
    because of the user-declared dtor, the implicitly defined copy functions are
    used instead).
    
    So turn ScShapeChild into a move-only class, clearing mpAccShape in moved-from
    instances.
    
    (The issue appears to exist ever since the code got introduced with
    e916b33bde0e863d33fdbb09dfd08c847ab76f20 "#95584#; add notes and shape support".
    It was found with Clang 10 trunk -Wdeprecated-copy-dtor.)
    
    Change-Id: Ib71262048c5af3f02716d03c3acd7d977bc66e07
    Reviewed-on: https://gerrit.libreoffice.org/85066
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/sc/source/ui/Accessibility/AccessibleDocumentPagePreview.cxx b/sc/source/ui/Accessibility/AccessibleDocumentPagePreview.cxx
index f887784bd92d..f27e98f5742c 100644
--- a/sc/source/ui/Accessibility/AccessibleDocumentPagePreview.cxx
+++ b/sc/source/ui/Accessibility/AccessibleDocumentPagePreview.cxx
@@ -55,6 +55,7 @@
 #include <vector>
 #include <algorithm>
 #include <memory>
+#include <utility>
 
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::accessibility;
@@ -580,7 +581,17 @@ struct ScShapeChild
         : mnRangeId(0)
     {
     }
+    ScShapeChild(ScShapeChild const &) = delete;
+    ScShapeChild(ScShapeChild &&) = default;
     ~ScShapeChild();
+    ScShapeChild & operator =(ScShapeChild const &) = delete;
+    ScShapeChild & operator =(ScShapeChild && other) {
+        std::swap(mpAccShape, other.mpAccShape);
+        mxShape = std::move(other.mxShape);
+        mnRangeId = other.mnRangeId;
+        return *this;
+    }
+
     mutable rtl::Reference< ::accessibility::AccessibleShape > mpAccShape;
     css::uno::Reference< css::drawing::XShape > mxShape;
     sal_Int32 mnRangeId;
@@ -617,6 +628,12 @@ namespace {
 
 struct ScShapeRange
 {
+    ScShapeRange() = default;
+    ScShapeRange(ScShapeRange const &) = delete;
+    ScShapeRange(ScShapeRange &&) = default;
+    ScShapeRange & operator =(ScShapeRange const &) = delete;
+    ScShapeRange & operator =(ScShapeRange &&) = default;
+
     ScShapeChildVec maBackShapes;
     ScShapeChildVec maForeShapes; // inclusive internal shapes
     ScShapeChildVec maControls;
@@ -747,7 +764,7 @@ void ScShapeChildren::FindChanged(ScShapeRange& rOld, ScShapeRange& rNew) const
 
 void ScShapeChildren::DataChanged()
 {
-    ScShapeRangeVec aOldShapeRanges(maShapeRanges);
+    ScShapeRangeVec aOldShapeRanges(std::move(maShapeRanges));
     maShapeRanges.clear();
     maShapeRanges.resize(SC_PREVIEW_MAXRANGES);
     Init();
@@ -1012,17 +1029,17 @@ void ScShapeChildren::FillShapes(const tools::Rectangle& aPixelPaintRect, const
                         aShape.mnRangeId = nRangeId;
                         if (pObj->GetLayer().anyOf(SC_LAYER_INTERN, SC_LAYER_FRONT))
                         {
-                            maShapeRanges[nRangeId].maForeShapes.push_back(aShape);
+                            maShapeRanges[nRangeId].maForeShapes.push_back(std::move(aShape));
                             bForeAdded = true;
                         }
                         else if (pObj->GetLayer() == SC_LAYER_BACK)
                         {
-                            maShapeRanges[nRangeId].maBackShapes.push_back(aShape);
+                            maShapeRanges[nRangeId].maBackShapes.push_back(std::move(aShape));
                             bBackAdded = true;
                         }
                         else if (pObj->GetLayer() == SC_LAYER_CONTROLS)
                         {
-                            maShapeRanges[nRangeId].maControls.push_back(aShape);
+                            maShapeRanges[nRangeId].maControls.push_back(std::move(aShape));
                             bControlAdded = true;
                         }
                         else