From 6e62e021e88f244c2de9a53a104e67b676fc2913 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Sun, 25 Aug 2024 19:44:07 +0200 Subject: [PATCH] tdf#158556 simplify StylePool API StylePool is only used inside writer, and only in very limited ways. Rather move the logic for the limits functions inside StylePool so I can then reduce the complexity of the iterator. Change-Id: I12bc5965b74abace28ae9190b35661a34f5005be Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172371 Reviewed-by: Noel Grandin Tested-by: Jenkins --- include/svl/stylepool.hxx | 44 ++--------- svl/CppunitTest_svl_items.mk | 1 - svl/qa/unit/items/stylepool.cxx | 105 -------------------------- svl/source/items/stylepool.cxx | 41 +++++++--- sw/source/core/doc/swstylemanager.cxx | 20 +---- 5 files changed, 39 insertions(+), 172 deletions(-) delete mode 100644 svl/qa/unit/items/stylepool.cxx diff --git a/include/svl/stylepool.hxx b/include/svl/stylepool.hxx index 0a22cb1f707e..dd01f2d475e0 100644 --- a/include/svl/stylepool.hxx +++ b/include/svl/stylepool.hxx @@ -16,15 +16,15 @@ * except in compliance with the License. You may obtain a copy of * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ -#ifndef INCLUDED_SVL_STYLEPOOL_HXX -#define INCLUDED_SVL_STYLEPOOL_HXX +#pragma once -#include #include #include +#include +#include +#include class StylePoolImpl; -class IStylePoolIteratorAccess; class SVL_DLLPUBLIC StylePool final { @@ -32,6 +32,7 @@ private: std::unique_ptr pImpl; public: explicit StylePool( SfxItemSet const * pIgnorableItems = nullptr ); + ~StylePool(); /** Insert a SfxItemSet into the style pool. @@ -48,41 +49,12 @@ public: */ std::shared_ptr insertItemSet( const SfxItemSet& rSet, const OUString* pParentName = nullptr ); - /** Create an iterator + void populateCacheMap(std::unordered_map< OUString, std::shared_ptr >& rCacheMap); - The iterator walks through the StylePool - OD 2008-03-07 #i86923# - introduce optional parameter to control, if unused SfxItemsSet are skipped or not - introduce optional parameter to control, if ignorable items are skipped or not - - @attention every change, e.g. destruction, of the StylePool could cause undefined effects. - - @param bSkipUnusedItemSets - input parameter - boolean, indicating if unused SfxItemSets are skipped or not - - @param bSkipIgnorableItems - input parameter - boolean, indicating if ignorable items are skipped or not - - @postcond the iterator "points before the first" SfxItemSet of the pool. - The first StylePoolIterator::getNext() call will deliver the first SfxItemSet. - */ - std::unique_ptr createIterator( const bool bSkipUnusedItemSets = false, - const bool bSkipIgnorableItems = false ); - - ~StylePool(); + // skips unused and ignorable items + void getAllStyles( std::vector> &rStyles ); static OUString nameOf( const std::shared_ptr& pSet ); }; -class SVL_DLLPUBLIC IStylePoolIteratorAccess -{ -public: - /** Delivers a shared pointer to the next SfxItemSet of the pool - If there is no more SfxItemSet, the delivered share_pointer is empty. - */ - virtual std::shared_ptr getNext() = 0; - virtual ~IStylePoolIteratorAccess() {}; -}; -#endif - /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/svl/CppunitTest_svl_items.mk b/svl/CppunitTest_svl_items.mk index 60bccd8a5852..c75f56bfef71 100644 --- a/svl/CppunitTest_svl_items.mk +++ b/svl/CppunitTest_svl_items.mk @@ -15,7 +15,6 @@ $(eval $(call gb_CppunitTest_use_sdk_api,svl_items)) $(eval $(call gb_CppunitTest_add_exception_objects,svl_items, \ svl/qa/unit/items/test_IndexedStyleSheets \ - svl/qa/unit/items/stylepool \ )) $(eval $(call gb_CppunitTest_use_libraries,svl_items, \ diff --git a/svl/qa/unit/items/stylepool.cxx b/svl/qa/unit/items/stylepool.cxx deleted file mode 100644 index 6bee19881c89..000000000000 --- a/svl/qa/unit/items/stylepool.cxx +++ /dev/null @@ -1,105 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ -/* - * This file is part of the LibreOffice project. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - */ - -#include - -#include -#include -#include -#include - -namespace -{ -/// Tests svl StylePool. -class StylePoolTest : public CppUnit::TestFixture -{ -}; - -ItemInfoPackage& getItemInfoPackageTest() -{ - class ItemInfoPackageTest : public ItemInfoPackage - { - typedef std::array ItemInfoArrayTest; - ItemInfoArrayTest maItemInfos{ { // m_nWhich, m_pItem, m_nSlotID, m_nItemInfoFlags - { 1, new SfxStringItem(1), 2, SFX_ITEMINFOFLAG_NONE } } }; - - virtual const ItemInfoStatic& getItemInfoStatic(size_t nIndex) const override - { - return maItemInfos[nIndex]; - } - - public: - virtual size_t size() const override { return maItemInfos.size(); } - virtual const ItemInfo& getItemInfo(size_t nIndex, SfxItemPool& /*rPool*/) override - { - return maItemInfos[nIndex]; - } - }; - - static std::unique_ptr g_aItemInfoPackageTest; - if (!g_aItemInfoPackageTest) - g_aItemInfoPackageTest.reset(new ItemInfoPackageTest); - return *g_aItemInfoPackageTest; -} - -CPPUNIT_TEST_FIXTURE(StylePoolTest, testIterationOrder) -{ - // Set up a style pool with multiple parents. - rtl::Reference pPool = new SfxItemPool(u"test"_ustr); - pPool->registerItemInfoPackage(getItemInfoPackageTest()); - { - // Set up parents in mixed order to make sure we do not sort by pointer address. - SfxItemSet aParent1(*pPool, svl::Items<1, 1>); - SfxItemSet aChild1(*pPool, svl::Items<1, 1>); - aChild1.SetParent(&aParent1); - SfxStringItem aItem1(1, u"Item1"_ustr); - aChild1.Put(aItem1); - - SfxItemSet aParent3(*pPool, svl::Items<1, 1>); - SfxItemSet aChild3(*pPool, svl::Items<1, 1>); - aChild3.SetParent(&aParent3); - SfxStringItem aItem3(1, u"Item3"_ustr); - aChild3.Put(aItem3); - - SfxItemSet aParent2(*pPool, svl::Items<1, 1>); - SfxItemSet aChild2(*pPool, svl::Items<1, 1>); - aChild2.SetParent(&aParent2); - SfxStringItem aItem2(1, u"Item2"_ustr); - aChild2.Put(aItem2); - - // Insert item sets in alphabetical order. - StylePool aStylePool; - OUString aChild1Name(u"Child1"_ustr); - aStylePool.insertItemSet(aChild1, &aChild1Name); - OUString aChild3Name(u"Child3"_ustr); - aStylePool.insertItemSet(aChild3, &aChild3Name); - OUString aChild2Name(u"Child2"_ustr); - aStylePool.insertItemSet(aChild2, &aChild2Name); - std::unique_ptr pIter = aStylePool.createIterator(); - std::shared_ptr pStyle1 = pIter->getNext(); - CPPUNIT_ASSERT(pStyle1); - const SfxStringItem* pItem1 = static_cast(pStyle1->GetItem(1)); - CPPUNIT_ASSERT_EQUAL(u"Item1"_ustr, pItem1->GetValue()); - std::shared_ptr pStyle2 = pIter->getNext(); - CPPUNIT_ASSERT(pStyle2); - const SfxStringItem* pItem2 = static_cast(pStyle2->GetItem(1)); - // Without the accompanying fix in place, this test would have failed with 'Expected: Item2; - // Actual: Item3'. The iteration order depended on the pointer address on the pointer - // address of the parents. - CPPUNIT_ASSERT_EQUAL(u"Item2"_ustr, pItem2->GetValue()); - std::shared_ptr pStyle3 = pIter->getNext(); - CPPUNIT_ASSERT(pStyle3); - const SfxStringItem* pItem3 = static_cast(pStyle3->GetItem(1)); - CPPUNIT_ASSERT_EQUAL(u"Item3"_ustr, pItem3->GetValue()); - CPPUNIT_ASSERT(!pIter->getNext()); - } -} -} - -/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/svl/source/items/stylepool.cxx b/svl/source/items/stylepool.cxx index 0f3959cb4687..af7b9fb6e874 100644 --- a/svl/source/items/stylepool.cxx +++ b/svl/source/items/stylepool.cxx @@ -232,7 +232,7 @@ namespace { return pReturn; } - class Iterator : public IStylePoolIteratorAccess + class Iterator { std::map< const SfxItemSet*, Node >& mrRoot; std::map< const SfxItemSet*, Node >::iterator mpCurrNode; @@ -280,7 +280,7 @@ namespace { if (mpCurrParent != maParents.end()) mpCurrNode = mrRoot.find(*mpCurrParent); } - virtual std::shared_ptr getNext() override; + std::shared_ptr getNext(); }; std::shared_ptr Iterator::getNext() @@ -366,8 +366,7 @@ public: std::shared_ptr insertItemSet( const SfxItemSet& rSet, const OUString* pParentName = nullptr ); // #i86923# - std::unique_ptr createIterator( bool bSkipUnusedItemSets, - bool bSkipIgnorableItems ); + Iterator createIterator( bool bSkipUnusedItemSets, bool bSkipIgnorableItems ); }; @@ -426,12 +425,12 @@ std::shared_ptr StylePoolImpl::insertItemSet( const SfxItemSet& rSet #if OSL_DEBUG_LEVEL >= 2 { sal_Int32 nCheck = -1; - std::unique_ptr pIter = createIterator(false,false); + Iterator aIter = createIterator(false,false); std::shared_ptr pTemp; do { ++nCheck; - pTemp = pIter->getNext(); + pTemp = aIter.getNext(); } while( pTemp.get() ); DBG_ASSERT( mnCount == nCheck, "Wrong counting"); } @@ -440,10 +439,10 @@ std::shared_ptr StylePoolImpl::insertItemSet( const SfxItemSet& rSet } // #i86923# -std::unique_ptr StylePoolImpl::createIterator( bool bSkipUnusedItemSets, +Iterator StylePoolImpl::createIterator( bool bSkipUnusedItemSets, bool bSkipIgnorableItems ) { - return std::make_unique( maRoot, bSkipUnusedItemSets, bSkipIgnorableItems, maParentNames ); + return Iterator( maRoot, bSkipUnusedItemSets, bSkipIgnorableItems, maParentNames ); } // Ctor, Dtor and redirected methods of class StylePool, nearly inline ;-) @@ -455,13 +454,31 @@ StylePool::StylePool( SfxItemSet const * pIgnorableItems ) std::shared_ptr StylePool::insertItemSet( const SfxItemSet& rSet, const OUString* pParentName ) { return pImpl->insertItemSet( rSet, pParentName ); } -// #i86923# -std::unique_ptr StylePool::createIterator( const bool bSkipUnusedItemSets, - const bool bSkipIgnorableItems ) +void StylePool::populateCacheMap(std::unordered_map< OUString, std::shared_ptr >& rCacheMap) { - return pImpl->createIterator( bSkipUnusedItemSets, bSkipIgnorableItems ); + Iterator aIter = pImpl->createIterator(/*bSkipUnusedItemSets*/false, /*bSkipIgnorableItems*/false); + std::shared_ptr pStyle = aIter.getNext(); + while( pStyle ) + { + OUString aName( StylePool::nameOf(pStyle) ); + rCacheMap[ aName ] = pStyle; + pStyle = aIter.getNext(); + } } +void StylePool::getAllStyles( std::vector> &rStyles ) +{ + // setup iterator, which skips unused styles and ignorable items + Iterator aIter = pImpl->createIterator( true, true ); + std::shared_ptr pStyle = aIter.getNext(); + while( pStyle ) + { + rStyles.push_back( pStyle ); + pStyle = aIter.getNext(); + } +} + + StylePool::~StylePool() {} diff --git a/sw/source/core/doc/swstylemanager.cxx b/sw/source/core/doc/swstylemanager.cxx index 6372ab10c89d..0deaeec3e78d 100644 --- a/sw/source/core/doc/swstylemanager.cxx +++ b/sw/source/core/doc/swstylemanager.cxx @@ -45,14 +45,7 @@ public: void SwStyleCache::addCompletePool( StylePool& rPool ) { - std::unique_ptr pIter = rPool.createIterator(); - std::shared_ptr pStyle = pIter->getNext(); - while( pStyle ) - { - OUString aName( StylePool::nameOf(pStyle) ); - mMap[ aName ] = pStyle; - pStyle = pIter->getNext(); - } + rPool.populateCacheMap(mMap); } namespace { @@ -161,16 +154,7 @@ void SwStyleManager::getAllStyles( std::vector> &rSt { StylePool& rAutoPool = eFamily == IStyleAccess::AUTO_STYLE_CHAR ? m_aAutoCharPool : m_aAutoParaPool; - // setup iterator, which skips unused styles and ignorable items - std::unique_ptr pIter = rAutoPool.createIterator( true, true ); - std::shared_ptr pStyle = pIter->getNext(); - while( pStyle ) - { - assert(eFamily != IStyleAccess::AUTO_STYLE_PARA || dynamic_cast(pStyle.get())); - rStyles.push_back( pStyle ); - - pStyle = pIter->getNext(); - } + rAutoPool.getAllStyles(rStyles); } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */