tdf#125609 toolkit: don't use XTabController::getControls

Calling XTabController::getControls was supposed to give
performance improvements (coded in OOo), but it
pulls cached information which is not up to date if
this listener for elementInserted events is handled before
the formController's listener. It is missing the most recently
created control - and thus it never sees the last control
in the form, and fails to create the radio group.

Additionally, when all of the controls are not yet
created, this function seems to be designed to catch that
and immediately return. With the "optimization" the
missing controls were never noticed, and so unnecessary
processing continued - a performance detriment while
the form is being built.

My impresssion is that the local getControl() function
is not terribly inefficient, so the performance impact
seems minimal, especially since it now only
makes the call once and caches the result itself.

Since not-yet-peered controls cause the function to again
terminate early (as it was designed to do),
this may have unintended side effects,
in case anything was designed in the past 10+ years
expecting the old behaviour, so I have no intention
of back-porting this.

Change-Id: Ica8ddab69043a30b23d008cd8db5df1c13b94ad2
Reviewed-on: https://gerrit.libreoffice.org/75163
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth@sil.org>

1 files changed, 7 insertions, 8 deletions

diff --git a/toolkit/source/controls/stdtabcontroller.cxx b/toolkit/source/controls/stdtabcontroller.cxx
index 7f89093a8dde..61900b4d76ce 100644
--- a/toolkit/source/controls/stdtabcontroller.cxx
+++ b/toolkit/source/controls/stdtabcontroller.cxx
@@ -308,18 +308,17 @@ void StdTabController::activateTabOrder(  )
     if ( !xC.is() || !xVclContainerPeer.is() )
         return;
 
-    // This may return a TabController, which returns desired list of controls faster
-    Reference< XTabController >  xTabController(static_cast< ::cppu::OWeakObject* >(this), UNO_QUERY);
-
     // Get a flattened list of controls sequences
     Sequence< Reference< XControlModel > > aModels = mxModel->getControlModels();
     Sequence< Reference< XWindow > > aCompSeq;
     Sequence< Any> aTabSeq;
 
-    // DG: For the sake of optimization, retrieve Controls from getControls(),
-    // this may sound counterproductive, but leads to performance improvements
-    // in practical scenarios (Forms)
-    Sequence< Reference< XControl > > aControls = xTabController->getControls();
+    // Previously used aControls = xTabController->getControls() "for the sake of optimization",
+    // but that list isn't valid during the creation phase (missing last created control) because
+    // listenermultiplexer.cxx handles fmvwimp::elementinserted before formcontroller::elementInserted
+    // Perhaps other places using the same optimization need to be reviewed?  (tdf#125609)
+    Sequence< Reference< XControl > > aCachedControls = getControls();
+    Sequence< Reference< XControl > > aControls = aCachedControls;
 
     // #58317# Some Models may be missing from the Container. Plus there is a
     // autoTabOrder call later on.
@@ -337,7 +336,7 @@ void StdTabController::activateTabOrder(  )
     {
         mxModel->getGroup( nG, aThisGroupModels, aName );
 
-        aControls = xTabController->getControls();
+        aControls = aCachedControls;
             // ImplCreateComponentSequence has a really strange semantics regarding it's first parameter:
             // upon method entry, it expects a super set of the controls which it returns
             // this means we need to completely fill this sequence with all available controls before