Make sure that root listeners are notified first

The problem appeared when in a configuration listener's changesOccurred,
a configuration value was read using officecfg machinery, which could
return the old value of the configuration, or an updated one, at random.

This was caused by use of a cached value in
comphelper::detail::ConfigurationWrapper::getPropertyValue, which is
cleaned in ConfigurationChangesListener::changesOccurred; but the order
in which the listeners' changesOccurred methods were called depended on
the implementation detail of configmgr::Components::roots_, which was
previously a std::set of pointers, and now is a sorted vector. This
made the pointers sorted in order of the pointers' addresses (basically
random), and when a broadcaster's common list of change listeners was
prepared in Components::initGlobalBroadcaster, ConfigurationWrapper's
listener could arrive last. This meant, that the cache could be cleaned
too late, after its obsolete content was already used in a previous
listener.

The problem could be partially mitigated by clearing the cache in the
comphelper::detail::ConfigurationWrapper::setPropertyValue, but that
would only handle cases of config modifications using comphelper.

Instead, take into account that ConfigurationWrapper listens on the
root of configuration tree; and introduce a separate container in
configmgr::Broadcaster for root listeners. They would be triggered
first, before all other listeners.

Still, this would not guarantee the proper order, if another listener
is registered on root. To handle all cases, a special listener category
could be used, which could be filled using a dedicated internal API, so
comphelper could use it to register its privileged listener close to
the heart of the broadcaster :) This might be implemented later.

Change-Id: I956b7989b3927dca2683f63cf92b0dda04db9bdf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154561
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

3 files changed, 19 insertions, 11 deletions

diff --git a/configmgr/source/broadcaster.cxx b/configmgr/source/broadcaster.cxx
index 03bcf8f273de..365e25f9eb02 100644
--- a/configmgr/source/broadcaster.cxx
+++ b/configmgr/source/broadcaster.cxx
@@ -97,9 +97,12 @@ void Broadcaster::addPropertiesChangeNotification(
 
 void Broadcaster::addChangesNotification(
     css::uno::Reference< css::util::XChangesListener > const & listener,
-    css::util::ChangesEvent const & event)
+    css::util::ChangesEvent const & event, bool bRootListener)
 {
-    changesNotifications_.emplace_back(listener, event);
+    if (bRootListener)
+        rootChangesNotifications_.emplace_back(listener, event);
+    else
+        changesNotifications_.emplace_back(listener, event);
 }
 
 void Broadcaster::send() {
@@ -164,13 +167,17 @@ void Broadcaster::send() {
             appendMessage(messages, e);
         }
     }
-    for (auto& rNotification : changesNotifications_) {
-        try {
-            rNotification.listener->changesOccurred(rNotification.event);
-        } catch (css::lang::DisposedException &) {
-        } catch (css::uno::Exception & e) {
-            exception = cppu::getCaughtException();
-            appendMessage(messages, e);
+    // First root listeners, then the rest
+    for (const auto& container : { rootChangesNotifications_ , changesNotifications_})
+    {
+        for (auto& rNotification : container) {
+            try {
+                rNotification.listener->changesOccurred(rNotification.event);
+            } catch (css::lang::DisposedException &) {
+            } catch (css::uno::Exception & e) {
+                exception = cppu::getCaughtException();
+                appendMessage(messages, e);
+            }
         }
     }
     if (exception.hasValue()) {
diff --git a/configmgr/source/broadcaster.hxx b/configmgr/source/broadcaster.hxx
index d0c461e731d8..711e7a83bd9e 100644
--- a/configmgr/source/broadcaster.hxx
+++ b/configmgr/source/broadcaster.hxx
@@ -72,7 +72,7 @@ public:
 
     void addChangesNotification(
         css::uno::Reference< css::util::XChangesListener > const & listener,
-        css::util::ChangesEvent const & event);
+        css::util::ChangesEvent const & event, bool bRootListener);
 
     void send();
 
@@ -131,6 +131,7 @@ private:
     std::vector< ContainerNotification > containerElementReplacedNotifications_;
     std::vector< PropertyChangeNotification > propertyChangeNotifications_;
     std::vector< PropertiesChangeNotification > propertiesChangeNotifications_;
+    std::vector< ChangesNotification > rootChangesNotifications_;
     std::vector< ChangesNotification > changesNotifications_;
 };
 
diff --git a/configmgr/source/rootaccess.cxx b/configmgr/source/rootaccess.cxx
index e2455556b6db..94be58e8660b 100644
--- a/configmgr/source/rootaccess.cxx
+++ b/configmgr/source/rootaccess.cxx
@@ -87,7 +87,7 @@ void RootAccess::initBroadcaster(
         broadcaster->addChangesNotification(
             changesListener,
             css::util::ChangesEvent(
-                pSource, css::uno::Any( xBase ), set));
+                pSource, css::uno::Any( xBase ), set), path_.empty());
     }
 }