Revert "optimize ConfigurationProperty::get()"

This reverts commit 7df433cdc33b4d6ba38eafad9282d015571433ef
and its follow-ups:

* b4b63d0c46979ad6b6aae5d6a4ea6672ea248e10 "try to fix some shutdown
crashes"
* 203ad037ccb9fdebffea4f622229ded90635eb8b "try to fix shutdown crashes in
ConfigurationWrapper"

since it introduced a crash starting with LibreOffice 7.4
See https://crashreport.libreoffice.org/stats/signature/void%20rtl::str::release%3C_rtl_uString%3E(_rtl_uString*)
Later, it was reverted in libreoffice-7-4 branch with
df79a29ea20fb698d650be45a48c607f54476dea "Revert "optimize
ConfigurationProperty::get()" (7.4 only)" so the crash
is no longer reported in that branch.
OTOH, Noel tried to fix it in master/libreoffice-7-5 branches
with the two commits mentioned above but the crash is still being
reported in LibreOffice 7.5
After talking to him, he suggested to revert it all

Finally, adapt code to make loplugin:stringviewparam happy
with getPropertyValue
Change-Id: Id9fa97d2a81a590e53abd027e978d2d6179222b8

Change-Id: Id2e3475c342770be5705a74b9c0b45f45d6be5ad
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146586
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
(cherry picked from commit 05b2bfc289df8712097cc1e640bf7d3bc6b86a84)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146529

2 files changed, 32 insertions, 95 deletions

diff --git a/comphelper/source/misc/configuration.cxx b/comphelper/source/misc/configuration.cxx
index b84f705f1db0..59631dbccd83 100644
--- a/comphelper/source/misc/configuration.cxx
+++ b/comphelper/source/misc/configuration.cxx
@@ -10,8 +10,11 @@
 #include <sal/config.h>
 
 #include <cassert>
+#include <map>
+#include <memory>
+#include <mutex>
+#include <string_view>
 
-#include <com/sun/star/beans/NamedValue.hpp>
 #include <com/sun/star/beans/PropertyAttribute.hpp>
 #include <com/sun/star/configuration/ReadOnlyAccess.hpp>
 #include <com/sun/star/configuration/ReadWriteAccess.hpp>
@@ -21,16 +24,12 @@
 #include <com/sun/star/container/XHierarchicalNameReplace.hpp>
 #include <com/sun/star/container/XNameAccess.hpp>
 #include <com/sun/star/container/XNameContainer.hpp>
-#include <com/sun/star/util/XChangesListener.hpp>
-#include <com/sun/star/util/XChangesNotifier.hpp>
-#include <com/sun/star/lang/DisposedException.hpp>
 #include <com/sun/star/lang/XLocalizable.hpp>
 #include <com/sun/star/uno/Any.hxx>
 #include <com/sun/star/uno/Reference.hxx>
 #include <comphelper/solarmutex.hxx>
 #include <comphelper/configuration.hxx>
 #include <comphelper/configurationlistener.hxx>
-#include <cppuhelper/implbase.hxx>
 #include <rtl/ustring.hxx>
 #include <sal/log.hxx>
 #include <i18nlangtag/languagetag.hxx>
@@ -107,65 +106,12 @@ comphelper::detail::ConfigurationWrapper::get()
     return WRAPPER;
 }
 
-class comphelper::detail::ConfigurationChangesListener
-    : public ::cppu::WeakImplHelper<css::util::XChangesListener>
-{
-     comphelper::detail::ConfigurationWrapper& mrConfigurationWrapper;
-public:
-    ConfigurationChangesListener(comphelper::detail::ConfigurationWrapper& rWrapper)
-        : mrConfigurationWrapper(rWrapper)
-    {}
-    // util::XChangesListener
-    virtual void SAL_CALL changesOccurred( const css::util::ChangesEvent& ) override
-    {
-        std::scoped_lock aGuard(mrConfigurationWrapper.maMutex);
-        mrConfigurationWrapper.maPropertyCache.clear();
-    }
-    virtual void SAL_CALL disposing(const css::lang::EventObject&) override
-    {
-        std::scoped_lock aGuard(mrConfigurationWrapper.maMutex);
-        mrConfigurationWrapper.mbDisposed = true;
-        mrConfigurationWrapper.maPropertyCache.clear();
-    }
-};
-
 comphelper::detail::ConfigurationWrapper::ConfigurationWrapper():
     context_(comphelper::getProcessComponentContext()),
-    access_(css::configuration::ReadWriteAccess::create(context_, "*")),
-    mbDisposed(false)
-{
-    // Set up a configuration notifier to invalidate the cache as needed.
-    try
-    {
-        css::uno::Reference< css::lang::XMultiServiceFactory > xConfigProvider(
-            css::configuration::theDefaultProvider::get( context_ ) );
-
-        // set root path
-        css::uno::Sequence< css::uno::Any > params {
-            css::uno::Any( css::beans::NamedValue{ "nodepath", css::uno::Any( OUString("/"))} ),
-            css::uno::Any( css::beans::NamedValue{ "locale", css::uno::Any( OUString("*"))} ) };
-
-        css::uno::Reference< css::uno::XInterface > xCfg
-            = xConfigProvider->createInstanceWithArguments(u"com.sun.star.configuration.ConfigurationAccess",
-                params);
-
-        maNotifier = css::uno::Reference< css::util::XChangesNotifier >(xCfg, css::uno::UNO_QUERY);
-        assert(maNotifier.is());
-        maListener = css::uno::Reference< ConfigurationChangesListener >(new ConfigurationChangesListener(*this));
-        maNotifier->addChangesListener(maListener);
-    }
-    catch(const css::uno::Exception&)
-    {
-        assert(false);
-    }
-}
+    access_(css::configuration::ReadWriteAccess::create(context_, "*"))
+{}
 
-comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper()
-{
-    maPropertyCache.clear();
-    maNotifier.clear();
-    maListener.clear();
-}
+comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper() {}
 
 bool comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const & path)
     const
@@ -176,26 +122,31 @@ bool comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const & path)
         != 0;
 }
 
-css::uno::Any comphelper::detail::ConfigurationWrapper::getPropertyValue(OUString const& path) const
+css::uno::Any comphelper::detail::ConfigurationWrapper::getPropertyValue(std::u16string_view path) const
 {
-    std::scoped_lock aGuard(maMutex);
-    if (mbDisposed)
-        throw css::lang::DisposedException();
     // Cache the configuration access, since some of the keys are used in hot code.
-    auto it = maPropertyCache.find(path);
-    if( it != maPropertyCache.end())
-        return it->second;
+    // Note that this cache is only used by the officecfg:: auto-generated code, using it for anything
+    // else would be unwise because the cache could end up containing stale entries.
+    static std::mutex gMutex;
+    static std::map<OUString, css::uno::Reference< css::container::XNameAccess >> gAccessMap;
 
-    sal_Int32 idx = path.lastIndexOf("/");
+    sal_Int32 idx = path.rfind('/');
     assert(idx!=-1);
-    OUString parentPath = path.copy(0, idx);
-    OUString childName = path.copy(idx+1);
-
-    css::uno::Reference<css::container::XNameAccess> access(
-        access_->getByHierarchicalName(parentPath), css::uno::UNO_QUERY_THROW);
-    css::uno::Any property = access->getByName(childName);
-    maPropertyCache.emplace(path, property);
-    return property;
+    OUString parentPath(path.substr(0, idx));
+    OUString childName(path.substr(idx+1));
+
+    std::scoped_lock aGuard(gMutex);
+
+    // check cache
+    auto it = gAccessMap.find(parentPath);
+    if (it == gAccessMap.end())
+    {
+        // not in the cache, look it up
+        css::uno::Reference<css::container::XNameAccess> access(
+            access_->getByHierarchicalName(parentPath), css::uno::UNO_QUERY_THROW);
+        it = gAccessMap.emplace(parentPath, access).first;
+    }
+    return it->second->getByName(childName);
 }
 
 void comphelper::detail::ConfigurationWrapper::setPropertyValue(
diff --git a/include/comphelper/configuration.hxx b/include/comphelper/configuration.hxx
index 45228b700944..222b9d5af124 100644
--- a/include/comphelper/configuration.hxx
+++ b/include/comphelper/configuration.hxx
@@ -12,16 +12,15 @@
 
 #include <sal/config.h>
 
+#include <optional>
+#include <string_view>
+
 #include <com/sun/star/uno/Any.hxx>
 #include <com/sun/star/uno/Reference.h>
 #include <comphelper/comphelperdllapi.h>
 #include <comphelper/processfactory.hxx>
 #include <sal/types.h>
 #include <memory>
-#include <mutex>
-#include <optional>
-#include <string_view>
-#include <unordered_map>
 
 namespace com::sun::star {
     namespace configuration { class XReadWriteAccess; }
@@ -32,10 +31,6 @@ namespace com::sun::star {
         class XNameContainer;
     }
     namespace uno { class XComponentContext; }
-    namespace util {
-        class XChangesListener;
-        class XChangesNotifier;
-    }
 }
 
 namespace comphelper {
@@ -85,17 +80,14 @@ private:
 
 namespace detail {
 
-class ConfigurationChangesListener;
-
 /// @internal
 class COMPHELPER_DLLPUBLIC ConfigurationWrapper {
-friend class ConfigurationChangesListener;
 public:
     static ConfigurationWrapper const & get();
 
     bool isReadOnly(OUString const & path) const;
 
-    css::uno::Any getPropertyValue(OUString const & path) const;
+    css::uno::Any getPropertyValue(std::u16string_view path) const;
 
     static void setPropertyValue(
         std::shared_ptr< ConfigurationChanges > const & batch,
@@ -143,12 +135,6 @@ private:
         // css.beans.XHierarchicalPropertySetInfo), but then
         // configmgr::Access::asProperty() would report all properties as
         // READONLY, so isReadOnly() would not work
-
-    mutable std::mutex maMutex;
-    bool mbDisposed;
-    mutable std::unordered_map<OUString, css::uno::Any> maPropertyCache;
-    css::uno::Reference< css::util::XChangesNotifier > maNotifier;
-    css::uno::Reference< css::util::XChangesListener > maListener;
 };
 
 /// @internal