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 <noel.grandin@collabora.co.uk>
Tested-by: Jenkins

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
+#pragma once
-#define INCLUDED_SVL_STYLEPOOL_HXX
 
-#include <memory>
 #include <rtl/ustring.hxx>
 #include <svl/itemset.hxx>
+#include <memory>
+#include <vector>
+#include <unordered_map>
 
 class StylePoolImpl;
-class IStylePoolIteratorAccess;
 
 class SVL_DLLPUBLIC StylePool final
 {
@@ -32,6 +32,7 @@ private:
     std::unique_ptr<StylePoolImpl> pImpl;
 public:
     explicit StylePool( SfxItemSet const * pIgnorableItems = nullptr );
+    ~StylePool();
 
     /** Insert a SfxItemSet into the style pool.
 
@@ -48,41 +49,12 @@ public:
     */
     std::shared_ptr<SfxItemSet> insertItemSet( const SfxItemSet& rSet, const OUString* pParentName = nullptr );
 
-    /** Create an iterator
+    void populateCacheMap(std::unordered_map< OUString, std::shared_ptr<SfxItemSet> >& rCacheMap);
 
-        The iterator walks through the StylePool
+    // skips unused and ignorable items
-        OD 2008-03-07 #i86923#
+    void getAllStyles( std::vector<std::shared_ptr<SfxItemSet>> &rStyles );
-        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<IStylePoolIteratorAccess> createIterator( const bool bSkipUnusedItemSets = false,
-                                                      const bool bSkipIgnorableItems = false );
-
-    ~StylePool();
 
     static OUString nameOf( const std::shared_ptr<SfxItemSet>& 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<SfxItemSet> getNext() = 0;
-    virtual ~IStylePoolIteratorAccess() {};
-};
-#endif
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

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, \

svl/qa/unit/items/stylepool.cxx

@@ -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 <unotest/bootstrapfixturebase.hxx>
-
-#include <svl/itempool.hxx>
-#include <svl/itemset.hxx>
-#include <svl/stylepool.hxx>
-#include <svl/stritem.hxx>
-
-namespace
-{
-/// Tests svl StylePool.
-class StylePoolTest : public CppUnit::TestFixture
-{
-};
-
-ItemInfoPackage& getItemInfoPackageTest()
-{
-    class ItemInfoPackageTest : public ItemInfoPackage
-    {
-        typedef std::array<ItemInfoStatic, 1> 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<ItemInfoPackageTest> 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<SfxItemPool> 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<IStylePoolIteratorAccess> pIter = aStylePool.createIterator();
-        std::shared_ptr<SfxItemSet> pStyle1 = pIter->getNext();
-        CPPUNIT_ASSERT(pStyle1);
-        const SfxStringItem* pItem1 = static_cast<const SfxStringItem*>(pStyle1->GetItem(1));
-        CPPUNIT_ASSERT_EQUAL(u"Item1"_ustr, pItem1->GetValue());
-        std::shared_ptr<SfxItemSet> pStyle2 = pIter->getNext();
-        CPPUNIT_ASSERT(pStyle2);
-        const SfxStringItem* pItem2 = static_cast<const SfxStringItem*>(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<SfxItemSet> pStyle3 = pIter->getNext();
-        CPPUNIT_ASSERT(pStyle3);
-        const SfxStringItem* pItem3 = static_cast<const SfxStringItem*>(pStyle3->GetItem(1));
-        CPPUNIT_ASSERT_EQUAL(u"Item3"_ustr, pItem3->GetValue());
-        CPPUNIT_ASSERT(!pIter->getNext());
-    }
-}
-}
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */

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<SfxItemSet> getNext() override;
+        std::shared_ptr<SfxItemSet> getNext();
     };
 
     std::shared_ptr<SfxItemSet> Iterator::getNext()
@@ -366,8 +366,7 @@ public:
     std::shared_ptr<SfxItemSet> insertItemSet( const SfxItemSet& rSet, const OUString* pParentName = nullptr );
 
     // #i86923#
-    std::unique_ptr<IStylePoolIteratorAccess> createIterator( bool bSkipUnusedItemSets,
+    Iterator createIterator( bool bSkipUnusedItemSets, bool bSkipIgnorableItems );
-                                              bool bSkipIgnorableItems );
 };
 
 
@@ -426,12 +425,12 @@ std::shared_ptr<SfxItemSet> StylePoolImpl::insertItemSet( const SfxItemSet& rSet
 #if OSL_DEBUG_LEVEL >= 2
     {
         sal_Int32 nCheck = -1;
-        std::unique_ptr<IStylePoolIteratorAccess> pIter = createIterator(false,false);
+        Iterator aIter = createIterator(false,false);
         std::shared_ptr<SfxItemSet> 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<SfxItemSet> StylePoolImpl::insertItemSet( const SfxItemSet& rSet
 }
 
 // #i86923#
-std::unique_ptr<IStylePoolIteratorAccess> StylePoolImpl::createIterator( bool bSkipUnusedItemSets,
+Iterator StylePoolImpl::createIterator( bool bSkipUnusedItemSets,
                                                          bool bSkipIgnorableItems )
 {
-    return std::make_unique<Iterator>( 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<SfxItemSet> StylePool::insertItemSet( const SfxItemSet& rSet, const OUString* pParentName )
 { return pImpl->insertItemSet( rSet, pParentName ); }
 
-// #i86923#
+void StylePool::populateCacheMap(std::unordered_map< OUString, std::shared_ptr<SfxItemSet> >& rCacheMap)
-std::unique_ptr<IStylePoolIteratorAccess> StylePool::createIterator( const bool bSkipUnusedItemSets,
-                                                     const bool bSkipIgnorableItems )
 {
-    return pImpl->createIterator( bSkipUnusedItemSets, bSkipIgnorableItems );
+    Iterator aIter = pImpl->createIterator(/*bSkipUnusedItemSets*/false, /*bSkipIgnorableItems*/false);
+    std::shared_ptr<SfxItemSet> pStyle = aIter.getNext();
+    while( pStyle )
+    {
+        OUString aName( StylePool::nameOf(pStyle) );
+        rCacheMap[ aName ] = pStyle;
+        pStyle = aIter.getNext();
+    }
 }
 
+void StylePool::getAllStyles( std::vector<std::shared_ptr<SfxItemSet>> &rStyles )
+{
+    // setup <StylePool> iterator, which skips unused styles and ignorable items
+    Iterator aIter = pImpl->createIterator( true, true );
+    std::shared_ptr<SfxItemSet> pStyle = aIter.getNext();
+    while( pStyle )
+    {
+        rStyles.push_back( pStyle );
+        pStyle = aIter.getNext();
+    }
+}
+
+
 StylePool::~StylePool()
 {}
 

sw/source/core/doc/swstylemanager.cxx

@@ -45,14 +45,7 @@ public:
 
 void SwStyleCache::addCompletePool( StylePool& rPool )
 {
-    std::unique_ptr<IStylePoolIteratorAccess> pIter = rPool.createIterator();
+    rPool.populateCacheMap(mMap);
-    std::shared_ptr<SfxItemSet> pStyle = pIter->getNext();
-    while( pStyle )
-    {
-        OUString aName( StylePool::nameOf(pStyle) );
-        mMap[ aName ] = pStyle;
-        pStyle = pIter->getNext();
-    }
 }
 
 namespace {
@@ -161,16 +154,7 @@ void SwStyleManager::getAllStyles( std::vector<std::shared_ptr<SfxItemSet>> &rSt
 {
     StylePool& rAutoPool
         = eFamily == IStyleAccess::AUTO_STYLE_CHAR ? m_aAutoCharPool : m_aAutoParaPool;
-    // setup <StylePool> iterator, which skips unused styles and ignorable items
+    rAutoPool.getAllStyles(rStyles);
-    std::unique_ptr<IStylePoolIteratorAccess> pIter = rAutoPool.createIterator( true, true );
-    std::shared_ptr<SfxItemSet> pStyle = pIter->getNext();
-    while( pStyle )
-    {
-        assert(eFamily != IStyleAccess::AUTO_STYLE_PARA || dynamic_cast<SwAttrSet*>(pStyle.get()));
-        rStyles.push_back( pStyle );
-
-        pStyle = pIter->getNext();
-    }
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */