sd/inc/stlpool.hxx | 10 +++---- sd/inc/stlsheet.hxx | 13 +++++++++ sd/source/core/drawdoc.cxx | 2 - sd/source/core/drawdoc3.cxx | 42 +++++++++++++++++++------------- sd/source/core/stlpool.cxx | 26 +++++++++---------- sd/source/ui/app/sdxfer.cxx | 2 - sd/source/ui/func/unmovss.cxx | 23 ++++++++++------- sd/source/ui/inc/unmovss.hxx | 4 +-- sd/source/ui/sidebar/DocumentHelper.cxx | 2 - 9 files changed, 75 insertions(+), 49 deletions(-)
New commits: commit b6b6d5c22e52c83af60eba077f0c5098f2198782 Author: Caolán McNamara <[email protected]> AuthorDate: Wed Aug 22 10:08:42 2018 +0100 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Sep 3 16:34:02 2018 +0200 Resolves: tdf#119259 reused in-use SdStyleSheet removed by undo since... commit a4cd841541a729d7b8126d27d91fa28e30b01403 Date: Thu Feb 13 16:10:47 2014 +0530 n#757432: Styles (rename &) copy to different decks. While copying slides to different slide decks, styles were not being copied if there is already one with the same name. This patch renames and copies those to keep the formatting intact. if a SdStyleSheet is not freshly created, but reused, it ends up in the rCreatedSheets list, and so when the copy is undone (via SdMoveStyleSheetsUndoAction), the SdStyleSheet is unconditionally remove from the StylePool and so its ref count hits 0 and is removed, even though there are still objects depending on it. the problem becames more common since... bash-4.4$ git show 57db6e24b5ad43d447c30e44a112c74c7e75b46b commit 57db6e24b5ad43d447c30e44a112c74c7e75b46b Date: Tue Aug 15 02:51:07 2017 +0530 Removing old SfxItemSet::getHash usage improves the SfxItemSet comparison. Assuming that its correct to leave SdStyleSheetPool::CopySheets with those reused SdStyleSheets in the list, change things so that a flag is passed around to indicate if the SdStyleSheet should be removed or not by undo Change-Id: I82b5bd93183fd1ff9e67957ccfb5babef81fd36d Reviewed-on: https://gerrit.libreoffice.org/59434 Tested-by: Jenkins Tested-by: Xisco Faulí <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sd/inc/stlpool.hxx b/sd/inc/stlpool.hxx index cfe639038f9a..f4cf930d26e3 100644 --- a/sd/inc/stlpool.hxx +++ b/sd/inc/stlpool.hxx @@ -70,12 +70,12 @@ public: SD_DLLPUBLIC void CreateLayoutStyleSheets(const OUString& rLayoutName, bool bCheck = false ); static void CreateLayoutSheetNames(const OUString& rLayoutName, std::vector<OUString> &aNameList); void CreateLayoutSheetList(const OUString& rLayoutName, SdStyleSheetVector& rLayoutSheets); - void CopyLayoutSheets(const OUString& rLayoutName, SdStyleSheetPool& rSourcePool, SdStyleSheetVector& rCreatedSheets ); + void CopyLayoutSheets(const OUString& rLayoutName, SdStyleSheetPool& rSourcePool, StyleSheetCopyResultVector& rCreatedSheets); void CopyGraphicSheets(SdStyleSheetPool& rSourcePool); void CopyCellSheets(SdStyleSheetPool& rSourcePool); void CopyTableStyles(SdStyleSheetPool const & rSourcePool); - void CopyCellSheets(SdStyleSheetPool& rSourcePool, SdStyleSheetVector& rCreatedSheets); - void RenameAndCopyGraphicSheets(SdStyleSheetPool& rSourcePool, SdStyleSheetVector& rCreatedSheets, OUString const &rRenameSuffix); + void CopyCellSheets(SdStyleSheetPool& rSourcePool, StyleSheetCopyResultVector& rCreatedSheets); + void RenameAndCopyGraphicSheets(SdStyleSheetPool& rSourcePool, StyleSheetCopyResultVector& rCreatedSheets, OUString const &rRenameSuffix); void CreatePseudosIfNecessary(); void UpdateStdNames(); @@ -117,8 +117,8 @@ public: private: void CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily ); - void CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, SdStyleSheetVector& rCreatedSheets ); - void CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, SdStyleSheetVector& rCreatedSheets, const OUString &rRenameSuffix ); + void CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, StyleSheetCopyResultVector& rCreatedSheets ); + void CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, StyleSheetCopyResultVector& rCreatedSheets, const OUString &rRenameSuffix ); virtual SfxStyleSheetBase* Create(const OUString& rName, SfxStyleFamily eFamily, SfxStyleSearchBits nMask) override; diff --git a/sd/inc/stlsheet.hxx b/sd/inc/stlsheet.hxx index ae21105070a8..e8e0842e00c7 100644 --- a/sd/inc/stlsheet.hxx +++ b/sd/inc/stlsheet.hxx @@ -155,6 +155,19 @@ private: typedef std::vector< rtl::Reference< SdStyleSheet > > SdStyleSheetVector; +struct StyleSheetCopyResult +{ + rtl::Reference<SdStyleSheet> m_xStyleSheet; + bool m_bCreatedByCopy; + StyleSheetCopyResult(SdStyleSheet* pStyleSheet, bool bCreatedByCopy) + : m_xStyleSheet(pStyleSheet) + , m_bCreatedByCopy(bCreatedByCopy) + { + } +}; + +typedef std::vector<StyleSheetCopyResult> StyleSheetCopyResultVector; + #endif // INCLUDED_SD_INC_STLSHEET_HXX /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sd/source/core/drawdoc.cxx b/sd/source/core/drawdoc.cxx index 0c152e067594..00b21c0deba8 100644 --- a/sd/source/core/drawdoc.cxx +++ b/sd/source/core/drawdoc.cxx @@ -637,7 +637,7 @@ SdDrawDocument* SdDrawDocument::AllocSdDrawDocument() const // Move with all of the master page's layouts OUString aOldLayoutName(const_cast<SdDrawDocument*>(this)->GetMasterSdPage(i, PageKind::Standard)->GetLayoutName()); aOldLayoutName = aOldLayoutName.copy( 0, aOldLayoutName.indexOf( SD_LT_SEPARATOR ) ); - SdStyleSheetVector aCreatedSheets; + StyleSheetCopyResultVector aCreatedSheets; pNewStylePool->CopyLayoutSheets(aOldLayoutName, *pOldStylePool, aCreatedSheets ); } diff --git a/sd/source/core/drawdoc3.cxx b/sd/source/core/drawdoc3.cxx index e598dc35e981..631796dde2ae 100644 --- a/sd/source/core/drawdoc3.cxx +++ b/sd/source/core/drawdoc3.cxx @@ -336,26 +336,26 @@ namespace { void -lcl_removeUnusedStyles(SfxStyleSheetBasePool* const pStyleSheetPool, SdStyleSheetVector& rStyles) +lcl_removeUnusedStyles(SfxStyleSheetBasePool* const pStyleSheetPool, StyleSheetCopyResultVector& rStyles) { - SdStyleSheetVector aUsedStyles; + StyleSheetCopyResultVector aUsedStyles; aUsedStyles.reserve(rStyles.size()); - for (SdStyleSheetVector::const_iterator aIt(rStyles.begin()), aLast(rStyles.end()); aIt != aLast; ++aIt) + for (const auto& a : rStyles) { - if ((*aIt)->IsUsed()) - aUsedStyles.push_back(*aIt); + if (a.m_xStyleSheet->IsUsed()) + aUsedStyles.push_back(a); else - pStyleSheetPool->Remove((*aIt).get()); + pStyleSheetPool->Remove(a.m_xStyleSheet.get()); } rStyles = aUsedStyles; } -SfxStyleSheet *lcl_findStyle(SdStyleSheetVector& rStyles, const OUString& aStyleName) +SfxStyleSheet *lcl_findStyle(StyleSheetCopyResultVector& rStyles, const OUString& aStyleName) { - for(SdStyleSheetVector::const_iterator aIt(rStyles.begin()), aLast(rStyles.end()); aIt != aLast; ++aIt) + for (const auto& a : rStyles) { - if((*aIt)->GetName().startsWith(aStyleName)) - return (*aIt).get(); + if (a.m_xStyleSheet->GetName().startsWith(aStyleName)) + return a.m_xStyleSheet.get(); } return nullptr; } @@ -504,7 +504,7 @@ bool SdDrawDocument::InsertBookmarkAsPage( std::vector<OUString>::const_iterator pIter; for ( pIter = aLayoutsToTransfer.begin(); pIter != aLayoutsToTransfer.end(); ++pIter ) { - SdStyleSheetVector aCreatedStyles; + StyleSheetCopyResultVector aCreatedStyles; OUString layoutName = *pIter; rStyleSheetPool.CopyLayoutSheets(layoutName, rBookmarkStyleSheetPool,aCreatedStyles); @@ -523,12 +523,12 @@ bool SdDrawDocument::InsertBookmarkAsPage( // that are not used in any of the inserted pages. The unused styles // are then removed at the end of the function, where we also create // undo records for the inserted styles. - SdStyleSheetVector aNewGraphicStyles; + StyleSheetCopyResultVector aNewGraphicStyles; OUString aRenameStr; if(!bReplace && !bNoDialogs) aRenameStr = "_"; rStyleSheetPool.RenameAndCopyGraphicSheets(rBookmarkStyleSheetPool, aNewGraphicStyles, aRenameStr); - SdStyleSheetVector aNewCellStyles; + StyleSheetCopyResultVector aNewCellStyles; rStyleSheetPool.CopyCellSheets(rBookmarkStyleSheetPool, aNewCellStyles); // TODO handle undo of table styles too @@ -1304,8 +1304,12 @@ void SdDrawDocument::RemoveUnnecessaryMasterPages(SdPage* pMasterPage, bool bOnl if( bUndo ) { + StyleSheetCopyResultVector aUndoRemove; + aUndoRemove.reserve(aRemove.size()); + for (const auto& a : aRemove) + aUndoRemove.emplace_back(a.get(), true); // This list belongs to UndoAction - SdMoveStyleSheetsUndoAction* pMovStyles = new SdMoveStyleSheetsUndoAction( this, aRemove, false ); + SdMoveStyleSheetsUndoAction* pMovStyles = new SdMoveStyleSheetsUndoAction(this, aUndoRemove, false); if (pUndoMgr) pUndoMgr->AddUndoAction(pMovStyles); @@ -1544,7 +1548,7 @@ void SdDrawDocument::SetMasterPage(sal_uInt16 nSdPageNum, pSourceStyleSheetPool->SetSearchMask(SfxStyleFamily::Page); static_cast<SdStyleSheetPool*>( mxStyleSheetPool.get())->SetSearchMask(SfxStyleFamily::Page); - SdStyleSheetVector aCreatedStyles; // List of created stylesheets + StyleSheetCopyResultVector aCreatedStyles; // List of created stylesheets SfxStyleSheetBase* pHisSheet = pSourceStyleSheetPool->First(); while (pHisSheet) @@ -1589,7 +1593,7 @@ void SdDrawDocument::SetMasterPage(sal_uInt16 nSdPageNum, pMySheet->GetItemSet().ClearItem(); // Delete all pMySheet->GetItemSet().Put(pHisSheet->GetItemSet()); - aCreatedStyles.emplace_back( static_cast< SdStyleSheet* >( pMySheet ) ); + aCreatedStyles.emplace_back(static_cast<SdStyleSheet*>(pMySheet), true); } StyleReplaceData aReplData; @@ -1801,7 +1805,11 @@ void SdDrawDocument::SetMasterPage(sal_uInt16 nSdPageNum, if( bUndo ) { - SdMoveStyleSheetsUndoAction* pMovStyles = new SdMoveStyleSheetsUndoAction(this, aCreatedStyles, true); + StyleSheetCopyResultVector aUndoInsert; + aUndoInsert.reserve(aCreatedStyles.size()); + for (const auto& a : aCreatedStyles) + aUndoInsert.emplace_back(a.get(), true); + SdMoveStyleSheetsUndoAction* pMovStyles = new SdMoveStyleSheetsUndoAction(this, aUndoInsert, true); pUndoMgr->AddUndoAction(pMovStyles); } diff --git a/sd/source/core/stlpool.cxx b/sd/source/core/stlpool.cxx index 506cdb08a2f4..884c9ffa1e68 100644 --- a/sd/source/core/stlpool.cxx +++ b/sd/source/core/stlpool.cxx @@ -90,14 +90,14 @@ OUString lcl_findRenamedStyleName(std::vector< std::pair< OUString, OUString > > return OUString(); } -SfxStyleSheet *lcl_findStyle(SdStyleSheetVector& rStyles, const OUString& aStyleName) +SfxStyleSheet *lcl_findStyle(StyleSheetCopyResultVector& rStyles, const OUString& aStyleName) { if( aStyleName.isEmpty() ) return nullptr; - for(SdStyleSheetVector::const_iterator aIt(rStyles.begin()), aLast(rStyles.end()); aIt != aLast; ++aIt) + for (const auto& a : rStyles) { - if((*aIt)->GetName() == aStyleName) - return (*aIt).get(); + if (a.m_xStyleSheet->GetName() == aStyleName) + return a.m_xStyleSheet.get(); } return nullptr; } @@ -586,23 +586,23 @@ void SdStyleSheetPool::CopyTableStyles(SdStyleSheetPool const & rSourcePool) } } -void SdStyleSheetPool::CopyCellSheets(SdStyleSheetPool& rSourcePool, SdStyleSheetVector& rCreatedSheets) +void SdStyleSheetPool::CopyCellSheets(SdStyleSheetPool& rSourcePool, StyleSheetCopyResultVector& rCreatedSheets) { CopySheets( rSourcePool, SfxStyleFamily::Frame, rCreatedSheets ); } -void SdStyleSheetPool::RenameAndCopyGraphicSheets(SdStyleSheetPool& rSourcePool, SdStyleSheetVector& rCreatedSheets, OUString const &rRenameSuffix) +void SdStyleSheetPool::RenameAndCopyGraphicSheets(SdStyleSheetPool& rSourcePool, StyleSheetCopyResultVector& rCreatedSheets, OUString const &rRenameSuffix) { CopySheets( rSourcePool, SfxStyleFamily::Para, rCreatedSheets, rRenameSuffix ); } void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily ) { - SdStyleSheetVector aTmpSheets; + StyleSheetCopyResultVector aTmpSheets; CopySheets(rSourcePool, eFamily, aTmpSheets); } -void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, SdStyleSheetVector& rCreatedSheets) +void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, StyleSheetCopyResultVector& rCreatedSheets) { CopySheets(rSourcePool, eFamily, rCreatedSheets, ""); } @@ -624,7 +624,7 @@ struct HasFamilyPredicate : svl::StyleSheetPredicate } -void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, SdStyleSheetVector& rCreatedSheets, const OUString& rRenameSuffix) +void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily eFamily, StyleSheetCopyResultVector& rCreatedSheets, const OUString& rRenameSuffix) { std::vector< std::pair< rtl::Reference< SfxStyleSheetBase >, OUString > > aNewStyles; std::vector< std::pair< OUString, OUString > > aRenamedList; @@ -689,13 +689,13 @@ void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily } xNewSheet->GetItemSet().Put( xSheet->GetItemSet() ); - rCreatedSheets.emplace_back( static_cast< SdStyleSheet* >( xNewSheet.get() ) ); + rCreatedSheets.emplace_back(static_cast<SdStyleSheet*>(xNewSheet.get()), true); aRenamedList.emplace_back( xSheet->GetName(), aName ); } else if (bAddToList) { // Add to list - used for renaming - rCreatedSheets.emplace_back( static_cast< SdStyleSheet* >( pExistingSheet ) ); + rCreatedSheets.emplace_back(static_cast<SdStyleSheet*>(pExistingSheet), false); aRenamedList.emplace_back( xSheet->GetName(), aName ); } } @@ -731,7 +731,7 @@ void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily |* \************************************************************************/ -void SdStyleSheetPool::CopyLayoutSheets(const OUString& rLayoutName, SdStyleSheetPool& rSourcePool, SdStyleSheetVector& rCreatedSheets) +void SdStyleSheetPool::CopyLayoutSheets(const OUString& rLayoutName, SdStyleSheetPool& rSourcePool, StyleSheetCopyResultVector& rCreatedSheets) { SfxStyleSheetBase* pSheet = nullptr; @@ -752,7 +752,7 @@ void SdStyleSheetPool::CopyLayoutSheets(const OUString& rLayoutName, SdStyleShee OUString file; rNewSheet.SetHelpId( file, pSourceSheet->GetHelpId( file ) ); rNewSheet.GetItemSet().Put(pSourceSheet->GetItemSet()); - rCreatedSheets.emplace_back( static_cast< SdStyleSheet* >( &rNewSheet ) ); + rCreatedSheets.emplace_back(static_cast<SdStyleSheet*>(&rNewSheet), true); } } } diff --git a/sd/source/ui/app/sdxfer.cxx b/sd/source/ui/app/sdxfer.cxx index 5db900b412bf..5bb95259fcbe 100644 --- a/sd/source/ui/app/sdxfer.cxx +++ b/sd/source/ui/app/sdxfer.cxx @@ -303,7 +303,7 @@ void SdTransferable::CreateData() sal_Int32 nPos = aOldLayoutName.indexOf( SD_LT_SEPARATOR ); if( nPos != -1 ) aOldLayoutName = aOldLayoutName.copy( 0, nPos ); - SdStyleSheetVector aCreatedSheets; + StyleSheetCopyResultVector aCreatedSheets; pNewStylePool->CopyLayoutSheets( aOldLayoutName, *pOldStylePool, aCreatedSheets ); } diff --git a/sd/source/ui/func/unmovss.cxx b/sd/source/ui/func/unmovss.cxx index f035a873e2b9..8805b324a1cc 100644 --- a/sd/source/ui/func/unmovss.cxx +++ b/sd/source/ui/func/unmovss.cxx @@ -23,7 +23,7 @@ #include <stlsheet.hxx> #include <stlpool.hxx> -SdMoveStyleSheetsUndoAction::SdMoveStyleSheetsUndoAction( SdDrawDocument* pTheDoc, SdStyleSheetVector& rTheStyles, bool bInserted) +SdMoveStyleSheetsUndoAction::SdMoveStyleSheetsUndoAction( SdDrawDocument* pTheDoc, StyleSheetCopyResultVector& rTheStyles, bool bInserted) : SdUndoAction(pTheDoc) , mbMySheets( !bInserted ) { @@ -32,9 +32,9 @@ SdMoveStyleSheetsUndoAction::SdMoveStyleSheetsUndoAction( SdDrawDocument* pTheDo maListOfChildLists.resize( maStyles.size() ); // create list with lists of style sheet children std::size_t i = 0; - for(SdStyleSheetVector::iterator iter = maStyles.begin(); iter != maStyles.end(); ++iter ) + for (auto& a : maStyles) { - maListOfChildLists[i++] = SdStyleSheetPool::CreateChildList( (*iter).get() ); + maListOfChildLists[i++] = SdStyleSheetPool::CreateChildList(a.m_xStyleSheet.get()); } } @@ -47,28 +47,33 @@ void SdMoveStyleSheetsUndoAction::Undo() // the styles have to be inserted in the pool // first insert all styles to the pool - for(SdStyleSheetVector::iterator iter = maStyles.begin(); iter != maStyles.end(); ++iter ) + for (auto& a : maStyles) { - pPool->Insert((*iter).get()); + if (!a.m_bCreatedByCopy) // tdf#119259, existed before this action, so leave it alone + continue; + pPool->Insert(a.m_xStyleSheet.get()); } // now assign the children again std::vector< SdStyleSheetVector >::iterator childlistiter( maListOfChildLists.begin() ); - for(SdStyleSheetVector::iterator iter = maStyles.begin(); iter != maStyles.end(); ++iter, ++childlistiter ) + for (auto& a : maStyles) { - OUString aParent((*iter)->GetName()); + OUString aParent(a.m_xStyleSheet->GetName()); for( SdStyleSheetVector::iterator childiter = (*childlistiter).begin(); childiter != (*childlistiter).end(); ++childiter ) { (*childiter)->SetParent(aParent); } + ++childlistiter; } } else { // remove the styles again from the pool - for(SdStyleSheetVector::iterator iter = maStyles.begin(); iter != maStyles.end(); ++iter ) + for (auto& a : maStyles) { - pPool->Remove((*iter).get()); + if (!a.m_bCreatedByCopy) // tdf#119259, existed before this action, so leave it alone + continue; + pPool->Remove(a.m_xStyleSheet.get()); } } mbMySheets = !mbMySheets; diff --git a/sd/source/ui/inc/unmovss.hxx b/sd/source/ui/inc/unmovss.hxx index f1f80384c106..474274d0cf57 100644 --- a/sd/source/ui/inc/unmovss.hxx +++ b/sd/source/ui/inc/unmovss.hxx @@ -28,12 +28,12 @@ class SdDrawDocument; class SdMoveStyleSheetsUndoAction : public SdUndoAction { - SdStyleSheetVector maStyles; + StyleSheetCopyResultVector maStyles; std::vector< SdStyleSheetVector > maListOfChildLists; bool mbMySheets; public: - SdMoveStyleSheetsUndoAction(SdDrawDocument* pTheDoc, SdStyleSheetVector& rTheStyles, bool bInserted); + SdMoveStyleSheetsUndoAction(SdDrawDocument* pTheDoc, StyleSheetCopyResultVector& rTheStyles, bool bInserted); virtual ~SdMoveStyleSheetsUndoAction() override; virtual void Undo() override; diff --git a/sd/source/ui/sidebar/DocumentHelper.cxx b/sd/source/ui/sidebar/DocumentHelper.cxx index 7198a19217d8..4d3f5d144403 100644 --- a/sd/source/ui/sidebar/DocumentHelper.cxx +++ b/sd/source/ui/sidebar/DocumentHelper.cxx @@ -258,7 +258,7 @@ void DocumentHelper::ProvideStyles ( static_cast<SdStyleSheetPool*>(rSourceDocument.GetStyleSheetPool()); SdStyleSheetPool* pTargetStyleSheetPool = static_cast<SdStyleSheetPool*>(rTargetDocument.GetStyleSheetPool()); - SdStyleSheetVector aCreatedStyles; + StyleSheetCopyResultVector aCreatedStyles; pTargetStyleSheetPool->CopyLayoutSheets ( sLayoutName, *pSourceStyleSheetPool, _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
