include/toolkit/controls/eventcontainer.hxx | 18 +++++- toolkit/CppunitTest_toolkit.mk | 1 toolkit/qa/cppunit/EventContainer.cxx | 82 ++++++++++++++++++++++++++++ toolkit/source/controls/eventcontainer.cxx | 56 ++++++++++++------- 4 files changed, 136 insertions(+), 21 deletions(-)
New commits: commit ed0474097cebb7de8ce0bfee8f758f54bc74791b Author: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> AuthorDate: Wed May 6 16:32:12 2020 +0200 Commit: Thorsten Behrens <thorsten.behr...@cib.de> CommitDate: Thu May 7 10:35:52 2020 +0200 Test keeping element order in EventContainer Change-Id: Ic33d8a83305f70bb3eb60f44da0d9ac0d2b47e16 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93570 Tested-by: Jenkins Reviewed-by: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93610 Tested-by: Thorsten Behrens <thorsten.behr...@cib.de> Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de> diff --git a/toolkit/CppunitTest_toolkit.mk b/toolkit/CppunitTest_toolkit.mk index 5afa7a616188..496ed0d7bac0 100644 --- a/toolkit/CppunitTest_toolkit.mk +++ b/toolkit/CppunitTest_toolkit.mk @@ -11,6 +11,7 @@ $(eval $(call gb_CppunitTest_CppunitTest,toolkit)) $(eval $(call gb_CppunitTest_add_exception_objects,toolkit, \ toolkit/qa/cppunit/Dialog \ + toolkit/qa/cppunit/EventContainer \ toolkit/qa/cppunit/UnitConversion \ )) diff --git a/toolkit/qa/cppunit/EventContainer.cxx b/toolkit/qa/cppunit/EventContainer.cxx new file mode 100644 index 000000000000..300c8e5adb74 --- /dev/null +++ b/toolkit/qa/cppunit/EventContainer.cxx @@ -0,0 +1,82 @@ +/* -*- 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 <test/bootstrapfixture.hxx> + +#include <com/sun/star/awt/UnoControlDialog.hpp> +#include <com/sun/star/awt/XControlModel.hpp> +#include <com/sun/star/beans/XPropertySet.hpp> +#include <com/sun/star/container/XNameContainer.hpp> +#include <com/sun/star/lang/XMultiComponentFactory.hpp> +#include <com/sun/star/script/ScriptEventDescriptor.hpp> +#include <com/sun/star/script/XScriptEventsSupplier.hpp> +#include <com/sun/star/uno/Reference.hxx> + +#include <comphelper/processfactory.hxx> + +using namespace css; +using namespace css::awt; +using namespace css::container; +using namespace css::lang; +using namespace css::script; +using namespace css::uno; + +namespace +{ +/// Test EventContainer class +class EventContainerTest : public test::BootstrapFixture +{ +protected: + Reference<XComponentContext> mxContext; + +public: + virtual void setUp() override; +}; + +void EventContainerTest::setUp() +{ + test::BootstrapFixture::setUp(); + + mxContext.set(comphelper::getComponentContext(getMultiServiceFactory())); +} + +// Make sure that EventContainer keeps insertion order, and does not reorder its elements. +// Otherwise this would break macro signatures. +CPPUNIT_TEST_FIXTURE(EventContainerTest, testInsertOrder) +{ + Reference<XMultiComponentFactory> xFactory(mxContext->getServiceManager(), UNO_SET_THROW); + Reference<XControlModel> xControlModel( + xFactory->createInstanceWithContext("com.sun.star.awt.UnoControlDialogModel", mxContext), + UNO_QUERY_THROW); + + Reference<beans::XPropertySet> xPropSet(xControlModel, UNO_QUERY_THROW); + + Reference<XScriptEventsSupplier> xEventsSupplier(xPropSet, UNO_QUERY_THROW); + Reference<XNameContainer> xEvents(xEventsSupplier->getEvents(), UNO_SET_THROW); + script::ScriptEventDescriptor descr1; + script::ScriptEventDescriptor descr2; + script::ScriptEventDescriptor descr3; + script::ScriptEventDescriptor descr4; + xEvents->insertByName("b", makeAny(descr1)); + xEvents->insertByName("a", makeAny(descr2)); + xEvents->insertByName("1", makeAny(descr3)); + xEvents->insertByName("A", makeAny(descr4)); + + Sequence<OUString> aEventNames(xEvents->getElementNames()); + sal_Int32 nEventCount = aEventNames.getLength(); + CPPUNIT_ASSERT_EQUAL(4, nEventCount); + + CPPUNIT_ASSERT_EQUAL(OUString("b"), aEventNames[0]); + CPPUNIT_ASSERT_EQUAL(OUString("a"), aEventNames[1]); + CPPUNIT_ASSERT_EQUAL(OUString("1"), aEventNames[2]); + CPPUNIT_ASSERT_EQUAL(OUString("A"), aEventNames[3]); +} +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ commit a85c89cfd5fd008a05081937b9b524f674ffed0f Author: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> AuthorDate: Thu Apr 30 14:24:06 2020 +0200 Commit: Thorsten Behrens <thorsten.behr...@cib.de> CommitDate: Thu May 7 10:35:39 2020 +0200 Revert "remove some "optimisation" insanity in ScriptEventContainer" This broke the event order in basic dialog xml, which in turn broke macro signatures. This reverts commit 85f08e3e34bea01456eaf8989ac4f77d3900d5c5. Change-Id: I49ef2eb200571a0fd862770abc4331b6ea053e2b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93209 Tested-by: Thorsten Behrens <thorsten.behr...@cib.de> Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93609 diff --git a/include/toolkit/controls/eventcontainer.hxx b/include/toolkit/controls/eventcontainer.hxx index 29ce0beecf80..3cb7406f58cd 100644 --- a/include/toolkit/controls/eventcontainer.hxx +++ b/include/toolkit/controls/eventcontainer.hxx @@ -31,13 +31,25 @@ namespace toolkit { +// Hashtable to optimize +typedef std::unordered_map +< + OUString, + sal_Int32, + OUStringHash +> +NameContainerNameMap; + + class ScriptEventContainer : public ::cppu::WeakImplHelper< css::container::XNameContainer, css::container::XContainer > { - std::unordered_map< OUString, css::uno::Any> - mHashMap; - css::uno::Type const mType; + NameContainerNameMap mHashMap; + css::uno::Sequence< OUString > mNames; + std::vector< css::uno::Any > mValues; + sal_Int32 mnElementCount; + css::uno::Type mType; ContainerListenerMultiplexer maContainerListeners; diff --git a/toolkit/source/controls/eventcontainer.cxx b/toolkit/source/controls/eventcontainer.cxx index 8873c638ad0a..b64b9a80d404 100644 --- a/toolkit/source/controls/eventcontainer.cxx +++ b/toolkit/source/controls/eventcontainer.cxx @@ -46,33 +46,33 @@ Type ScriptEventContainer::getElementType() sal_Bool ScriptEventContainer::hasElements() { - return !mHashMap.empty(); + bool bRet = (mnElementCount > 0); + return bRet; } // Methods XNameAccess Any ScriptEventContainer::getByName( const OUString& aName ) { - auto aIt = mHashMap.find( aName ); + NameContainerNameMap::iterator aIt = mHashMap.find( aName ); if( aIt == mHashMap.end() ) { throw NoSuchElementException(); } - return aIt->second; + sal_Int32 iHashResult = (*aIt).second; + Any aRetAny = mValues[ iHashResult ]; + return aRetAny; } Sequence< OUString > ScriptEventContainer::getElementNames() { - Sequence<OUString> aRet(mHashMap.size()); - int i = 0; - for (auto const & pair : mHashMap) - aRet[i++] = pair.first; - return aRet; + return mNames; } sal_Bool ScriptEventContainer::hasByName( const OUString& aName ) { - auto aIt = mHashMap.find( aName ); - return aIt != mHashMap.end(); + NameContainerNameMap::iterator aIt = mHashMap.find( aName ); + bool bRet = ( aIt != mHashMap.end() ); + return bRet; } @@ -83,13 +83,14 @@ void ScriptEventContainer::replaceByName( const OUString& aName, const Any& aEle if( mType != aAnyType ) throw IllegalArgumentException(); - auto aIt = mHashMap.find( aName ); + NameContainerNameMap::iterator aIt = mHashMap.find( aName ); if( aIt == mHashMap.end() ) { throw NoSuchElementException(); } - Any aOldElement = aIt->second; - aIt->second = aElement; + sal_Int32 iHashResult = (*aIt).second; + Any aOldElement = mValues[ iHashResult ]; + mValues[ iHashResult ] = aElement; // Fire event ContainerEvent aEvent; @@ -108,13 +109,18 @@ void ScriptEventContainer::insertByName( const OUString& aName, const Any& aElem if( mType != aAnyType ) throw IllegalArgumentException(); - auto aIt = mHashMap.find( aName ); + NameContainerNameMap::iterator aIt = mHashMap.find( aName ); if( aIt != mHashMap.end() ) { throw ElementExistException(); } - mHashMap[ aName ] = aElement; + sal_Int32 nCount = mNames.getLength(); + mNames.realloc( nCount + 1 ); + mValues.resize( nCount + 1 ); + mNames.getArray()[ nCount ] = aName; + mValues[ nCount ] = aElement; + mHashMap[ aName ] = nCount; // Fire event ContainerEvent aEvent; @@ -126,20 +132,33 @@ void ScriptEventContainer::insertByName( const OUString& aName, const Any& aElem void ScriptEventContainer::removeByName( const OUString& Name ) { - auto aIt = mHashMap.find( Name ); + NameContainerNameMap::iterator aIt = mHashMap.find( Name ); if( aIt == mHashMap.end() ) { throw NoSuchElementException(); } + sal_Int32 iHashResult = (*aIt).second; + Any aOldElement = mValues[ iHashResult ]; + // Fire event ContainerEvent aEvent; aEvent.Source = *this; - aEvent.Element = aIt->second; + aEvent.Element = aOldElement; aEvent.Accessor <<= Name; maContainerListeners.elementRemoved( aEvent ); mHashMap.erase( aIt ); + sal_Int32 iLast = mNames.getLength() - 1; + if( iLast != iHashResult ) + { + OUString* pNames = mNames.getArray(); + pNames[ iHashResult ] = pNames[ iLast ]; + mValues[ iHashResult ] = mValues[ iLast ]; + mHashMap[ pNames[ iHashResult ] ] = iHashResult; + } + mNames.realloc( iLast ); + mValues.resize( iLast ); } // Methods XContainer @@ -155,7 +174,8 @@ void ScriptEventContainer::removeContainerListener( const css::uno::Reference< c ScriptEventContainer::ScriptEventContainer() - : mType( cppu::UnoType<ScriptEventDescriptor>::get() ), + : mnElementCount( 0 ), + mType( cppu::UnoType<ScriptEventDescriptor>::get() ), maContainerListeners( *this ) { } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits