[PATCH] replace ShapeList::getNextShape with STL like iterator.

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/4191

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/91/4191/1

replace ShapeList::getNextShape with STL like iterator.

Replaced getNextShape() because it is not thread safe.

Change-Id: I840c76201805f46b4d8ea7ea4e4fa4904eb51c79
---
M sd/inc/drawdoc.hxx
M sd/inc/shapelist.hxx
M sd/source/core/drawdoc.cxx
M sd/source/core/drawdoc4.cxx
M sd/source/core/sdpage.cxx
M sd/source/core/sdpage2.cxx
M sd/source/core/shapelist.cxx
M sd/source/ui/view/drviews1.cxx
M sd/source/ui/view/sdview5.cxx
M sd/source/ui/view/viewoverlaymanager.cxx
10 files changed, 70 insertions(+), 84 deletions(-)



diff --git a/sd/inc/drawdoc.hxx b/sd/inc/drawdoc.hxx
index 77ac6b6..1d05eb3 100644
--- a/sd/inc/drawdoc.hxx
+++ b/sd/inc/drawdoc.hxx
@@ -145,6 +145,7 @@
     Timer*              mpWorkStartupTimer;
     Timer*              mpOnlineSpellingTimer;
     sd::ShapeList*      mpOnlineSpellingList;
+    sd::ShapeList::const_iterator maShapeListIterator;
     SvxSearchItem*      mpOnlineSearchItem;
     std::vector<sd::FrameView*> maFrameViewList;
     SdCustomShowList*   mpCustomShowList;
diff --git a/sd/inc/shapelist.hxx b/sd/inc/shapelist.hxx
index f828ebc6..2d759a2 100644
--- a/sd/inc/shapelist.hxx
+++ b/sd/inc/shapelist.hxx
@@ -29,6 +29,10 @@
     class ShapeList : public sdr::ObjectUser
     {
     public:
+        /** const_iterator guarantee only that the list itself is not
+           altered. The objects referenced by the list are still mutable. */
+        typedef std::list< SdrObject* >::const_iterator const_iterator;
+
         ShapeList();
         virtual ~ShapeList();
 
@@ -48,26 +52,17 @@
         /** @return true if given shape is part of this list */
         bool hasShape( SdrObject& rObject ) const;
 
-        /** returns the shape the internal iterator points to, or 0 if
-         * the list end is reached. moves the internal iterator to the
-         * next shape. */
-        SdrObject* getNextShape();
+        /** @return const_iterator pointing to the first element */
+        const_iterator cbegin() const;
 
-        /** Sets the internal iterator to the shape at given index. */
-        void seekShape( sal_uInt32 nIndex );
-
-        /**
-        */
-        bool hasMore() const;
-
-        const std::list< SdrObject* >& getList() const { return maShapeList; }
+        /** @return const_iterator pointing to the list termination element */
+        const_iterator cend() const;
 
     private:
         virtual void ObjectInDestruction(const SdrObject& rObject);
 
         typedef std::list< SdrObject* > ListImpl;
         ListImpl maShapeList;
-        ListImpl::iterator maIter;
     };
 }
 
diff --git a/sd/source/core/drawdoc.cxx b/sd/source/core/drawdoc.cxx
index aca4996..ed8d1fc 100644
--- a/sd/source/core/drawdoc.cxx
+++ b/sd/source/core/drawdoc.cxx
@@ -144,6 +144,7 @@
 , mpWorkStartupTimer(NULL)
 , mpOnlineSpellingTimer(NULL)
 , mpOnlineSpellingList(NULL)
+, maShapeListIterator()
 , mpOnlineSearchItem(NULL)
 , mpCustomShowList(NULL)
 , mpDocSh(static_cast< ::sd::DrawDocShell*>(pDrDocSh))
@@ -694,7 +695,7 @@
 */
 void SdDrawDocument::NewOrLoadCompleted( SdPage* pPage, SdStyleSheetPool* pSPool )
 {
-    sd::ShapeList& rPresentationShapes( pPage->GetPresentationShapeList() );
+    const sd::ShapeList& rPresentationShapes( pPage->GetPresentationShapeList() );
     if(!rPresentationShapes.isEmpty())
     {
         // Create lists of title and outline styles
@@ -706,13 +707,13 @@
 
         SfxStyleSheet* pTitleSheet = (SfxStyleSheet*)pSPool->GetTitleSheet(aName);
 
-        SdrObject* pObj = 0;
-        rPresentationShapes.seekShape(0);
-
         // Now look for title and outline text objects, then make those objects
         // listeners.
-        while( (pObj = rPresentationShapes.getNextShape()) )
+        for( ShapeList::const_iterator aIter (rPresentationShapes.cbegin() );
+             aIter != rPresentationShapes.cend(); ++aIter )
         {
+            SdrObject* pObj = *aIter;
+
             if (pObj->GetObjInventor() == SdrInventor)
             {
                 OutlinerParaObject* pOPO = pObj->GetOutlinerParaObject();
diff --git a/sd/source/core/drawdoc4.cxx b/sd/source/core/drawdoc4.cxx
index 6954913..6c90cac 100644
--- a/sd/source/core/drawdoc4.cxx
+++ b/sd/source/core/drawdoc4.cxx
@@ -755,6 +755,7 @@
         pOutl->SetDefaultLanguage( meLanguage );
 
         mpOnlineSpellingList = new ShapeList;
+        maShapeListIterator = mpOnlineSpellingList->cend();
         sal_uInt16 nPage;
 
         for ( nPage = 0; nPage < GetPageCount(); nPage++ )
@@ -769,7 +770,7 @@
             FillOnlineSpellingList((SdPage*) GetMasterPage(nPage));
         }
 
-        mpOnlineSpellingList->seekShape(0);
+        maShapeListIterator = mpOnlineSpellingList->cbegin();
         mpOnlineSpellingTimer = new Timer();
         mpOnlineSpellingTimer->SetTimeoutHdl( LINK(this, SdDrawDocument, OnlineSpellingHdl) );
         mpOnlineSpellingTimer->SetTimeout(250);
@@ -823,11 +824,14 @@
 // OnlineSpelling in the background
 IMPL_LINK_NOARG(SdDrawDocument, OnlineSpellingHdl)
 {
+    bool bHasMore = maShapeListIterator != mpOnlineSpellingList->cend();
+
     if (mpOnlineSpellingList!=NULL
-        && ( !mbOnlineSpell || mpOnlineSpellingList->hasMore()))
+        && ( !mbOnlineSpell || bHasMore))
     {
         // Spell next object
-        SdrObject* pObj = mpOnlineSpellingList->getNextShape();
+        SdrObject* pObj = *maShapeListIterator;
+        ++maShapeListIterator;
 
         if (pObj)
         {
diff --git a/sd/source/core/sdpage.cxx b/sd/source/core/sdpage.cxx
index 71d407b..bff354e 100644
--- a/sd/source/core/sdpage.cxx
+++ b/sd/source/core/sdpage.cxx
@@ -155,11 +155,11 @@
     // first sort all matching shapes with z-order
     std::vector< SdrObject* > aMatches;
 
-    SdrObject* pObj = 0;
-    maPresentationShapeList.seekShape(0);
-
-    while( (pObj = maPresentationShapeList.getNextShape()) )
+    for( ShapeList::const_iterator aIter( maPresentationShapeList.cbegin() );
+        aIter != maPresentationShapeList.cend(); ++aIter )
     {
+        SdrObject* pObj = *aIter;
+
         SdAnimationInfo* pInfo = SdDrawDocument::GetShapeUserData(*pObj);
         if( pInfo )
         {
@@ -1574,27 +1574,38 @@
     // now delete all empty presentation objects that are no longer used by the new layout
     if( bInit )
     {
-        SdrObject* pObj = 0;
-        maPresentationShapeList.seekShape(0);
+        std::list< SdrObject* > aRemoveList;
 
-        while( (pObj = maPresentationShapeList.getNextShape()) )
+        for( ShapeList::const_iterator aIter = maPresentationShapeList.cbegin();
+             aIter != maPresentationShapeList.cend(); ++aIter )
         {
+            SdrObject* pObj = *aIter;
+
             if( aUsedPresentationObjects.count(pObj) == 0 )
             {
 
                 if( pObj->IsEmptyPresObj() )
                 {
-                    if( bUndo )
-                        
pUndoManager->AddUndoAction(pModel->GetSdrUndoFactory().CreateUndoDeleteObject(*pObj));
-
-                    RemoveObject( pObj->GetOrdNum() );
-
-                    if( !bUndo )
-                        SdrObject::Free( pObj );
+                    // remove the object now would invalidate the iterator and lead to a seg fault
+                    aRemoveList.push_back(pObj);
                 }
 /* #i108541# keep non empty pres obj as pres obj even if they are not part of the current layout */
             }
         }
+
+        for( std::list<SdrObject*>::iterator aIter = aRemoveList.begin();
+             aIter != aRemoveList.end(); ++aIter )
+        {
+            SdrObject* pObj = *aIter;
+
+            if( bUndo )
+                
pUndoManager->AddUndoAction(pModel->GetSdrUndoFactory().CreateUndoDeleteObject(*pObj));
+
+            RemoveObject( pObj->GetOrdNum() );
+
+            if( !bUndo )
+                SdrObject::Free( pObj );
+        }
     }
 }
 
diff --git a/sd/source/core/sdpage2.cxx b/sd/source/core/sdpage2.cxx
index 1b4f9ab..ff88f06 100644
--- a/sd/source/core/sdpage2.cxx
+++ b/sd/source/core/sdpage2.cxx
@@ -379,10 +379,8 @@
     mePageKind           = rSrcPage.mePageKind;
     meAutoLayout         = rSrcPage.meAutoLayout;
 
-    // use shape list directly to preserve constness of rSrcPage
-    const std::list< SdrObject* >& rShapeList = rSrcPage.maPresentationShapeList.getList();
-    for( std::list< SdrObject* >::const_iterator aIter = rShapeList.begin();
-         aIter != rShapeList.end(); ++aIter )
+    for( ShapeList::const_iterator aIter( rSrcPage.maPresentationShapeList.cbegin() );
+         aIter != rSrcPage.maPresentationShapeList.cend(); ++aIter )
     {
         SdrObject* pObj = *aIter;
         InsertPresObj(GetObj(pObj->GetOrdNum()), rSrcPage.GetPresObjKind(pObj));
diff --git a/sd/source/core/shapelist.cxx b/sd/source/core/shapelist.cxx
index a265e9c..c3aedde 100644
--- a/sd/source/core/shapelist.cxx
+++ b/sd/source/core/shapelist.cxx
@@ -26,7 +26,6 @@
 
 ShapeList::ShapeList()
 {
-    maIter = maShapeList.end();
 }
 
 ShapeList::~ShapeList()
@@ -55,13 +54,8 @@
     ListImpl::iterator aIter( std::find( maShapeList.begin(), maShapeList.end(), &rObject ) );
     if( aIter != maShapeList.end() )
     {
-        bool bIterErased = aIter == maIter;
-
         (*aIter)->RemoveObjectUser(*this);
         aIter = maShapeList.erase( aIter );
-
-        if( bIterErased )
-            maIter = aIter;
 
         if( aIter != maShapeList.end() )
             return (*aIter);
@@ -83,8 +77,6 @@
     ListImpl::iterator aIter( aShapeList.begin() );
     while( aIter != aShapeList.end() )
         (*aIter++)->RemoveObjectUser(*this);
-
-    maIter = aShapeList.end();
 }
 
 /** returns true if this list is empty */
@@ -99,46 +91,27 @@
     return std::find( maShapeList.begin(), maShapeList.end(), &rObject )  != maShapeList.end();
 }
 
+ShapeList::const_iterator ShapeList::cbegin() const
+{
+    return maShapeList.begin();
+}
+
+ShapeList::const_iterator ShapeList::cend() const
+{
+    return maShapeList.end();
+}
+
 void ShapeList::ObjectInDestruction(const SdrObject& rObject)
 {
     ListImpl::iterator aIter( std::find( maShapeList.begin(), maShapeList.end(), &rObject ) );
     if( aIter != maShapeList.end() )
     {
-        bool bIterErased = aIter == maIter;
-
-        aIter = maShapeList.erase( aIter );
-
-        if( bIterErased )
-            maIter = aIter;
+        maShapeList.erase( aIter );
     }
     else
     {
         OSL_FAIL("sd::ShapeList::ObjectInDestruction(), got a call from an unknown friend!");
     }
-}
-
-SdrObject* ShapeList::getNextShape()
-{
-    if( maIter != maShapeList.end() )
-    {
-        return (*maIter++);
-    }
-    else
-    {
-        return 0;
-    }
-}
-
-void ShapeList::seekShape( sal_uInt32 nIndex )
-{
-    maIter = maShapeList.begin();
-    while( nIndex-- && (maIter != maShapeList.end()) )
-        maIter++;
-}
-
-bool ShapeList::hasMore() const
-{
-    return maIter != maShapeList.end();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sd/source/ui/view/drviews1.cxx b/sd/source/ui/view/drviews1.cxx
index fe0f1dd..8ad0ce3 100644
--- a/sd/source/ui/view/drviews1.cxx
+++ b/sd/source/ui/view/drviews1.cxx
@@ -1059,11 +1059,12 @@
             {
                 // set pages for all available handout presentation objects
                 sd::ShapeList& rShapeList = pMaster->GetPresentationShapeList();
-                SdrObject* pObj = 0;
-                rShapeList.seekShape(0);
 
-                while( (pObj = rShapeList.getNextShape()) )
+                for( ShapeList::const_iterator aIter( rShapeList.cbegin() );
+                    aIter != rShapeList.cend(); ++aIter )
                 {
+                    SdrObject* pObj = *aIter;
+
                     if( pMaster->GetPresObjKind(pObj) == PRESOBJ_HANDOUT )
                     {
                         // #i105146# We want no content to be displayed for PK_HANDOUT,
diff --git a/sd/source/ui/view/sdview5.cxx b/sd/source/ui/view/sdview5.cxx
index 9023d02..3cb0829 100644
--- a/sd/source/ui/view/sdview5.cxx
+++ b/sd/source/ui/view/sdview5.cxx
@@ -101,9 +101,10 @@
         // last try to find empty pres obj of multiple type
         if( !pEmptyObj )
         {
-            const std::list< SdrObject* >& rShapes = pPage->GetPresentationShapeList().getList();
+            ShapeList& rShapeList = pPage->GetPresentationShapeList();
 
-            for( std::list< SdrObject* >::const_iterator iter( rShapes.begin() ); iter != 
rShapes.end(); ++iter )
+            for( ShapeList::const_iterator iter( rShapeList.cbegin() );
+                 iter != rShapeList.cend(); ++iter )
             {
                 if( (*iter)->IsEmptyPresObj() && implIsMultiPresObj(pPage->GetPresObjKind(*iter)) )
                 {
diff --git a/sd/source/ui/view/viewoverlaymanager.cxx b/sd/source/ui/view/viewoverlaymanager.cxx
index e834999..f5c6bc6 100644
--- a/sd/source/ui/view/viewoverlaymanager.cxx
+++ b/sd/source/ui/view/viewoverlaymanager.cxx
@@ -546,9 +546,10 @@
 
     if( pPage && !pPage->IsMasterPage() && (pPage->GetPageKind() == PK_STANDARD) )
     {
-        const std::list< SdrObject* >& rShapes = pPage->GetPresentationShapeList().getList();
+        ShapeList& rShapeList =  pPage->GetPresentationShapeList();
 
-        for( std::list< SdrObject* >::const_iterator iter( rShapes.begin() ); iter != 
rShapes.end(); ++iter )
+        for( ShapeList::const_iterator iter( rShapeList.cbegin() );
+             iter != rShapeList.cend(); ++iter )
         {
             if( (*iter)->IsEmptyPresObj() && ((*iter)->GetObjIdentifier() == OBJ_OUTLINETEXT) && 
(mrBase.GetDrawView()->GetTextEditObject() != (*iter)) )
             {

-- 
To view, visit https://gerrit.libreoffice.org/4191
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I840c76201805f46b4d8ea7ea4e4fa4904eb51c79
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: master
Gerrit-Owner: mhofmann <borim7@web.de>