tdf#120363: try to avoid asking user to enable JVM when looking ...

... for a provider for an operation.

When another provider actually handles an operation, it's useless to
ask user to enable disabled JVM just to learn that it doesn't handle
the request. So, this patch does the MasterScriptProvider operations
in two steps: first with "Enable JVM" interaction disabled, and if
failed, again with the interaction enabled to try disabled providers.

This shouldn't typically give performance penalties in case when JVM
is enabled, and when it's disabled and the operation is addressed to
another provider.

A context class designed to disable "Enable JVM" interaction is moved
from cui/source/customize/cfgutil.cxx to a new comphelper header,
which is supposed to hold similar helper context classes in needed.

Change-Id: I21be922bfd80a276d9c8f1215d62a47bb3c225f5
Reviewed-on: https://gerrit.libreoffice.org/61468
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

3 files changed, 111 insertions, 98 deletions

diff --git a/cui/source/customize/cfgutil.cxx b/cui/source/customize/cfgutil.cxx
index 56d91c1eaf82..fbc07d50f41a 100644
--- a/cui/source/customize/cfgutil.cxx
+++ b/cui/source/customize/cfgutil.cxx
@@ -47,6 +47,7 @@
 #include <bitmaps.hlst>
 #include <sfx2/app.hxx>
 #include <sfx2/minfitem.hxx>
+#include <comphelper/DisableInteractionHelper.hxx>
 #include <comphelper/documentinfo.hxx>
 #include <comphelper/processfactory.hxx>
 #include <comphelper/sequenceashashmap.hxx>
@@ -65,7 +66,6 @@
 #include <vcl/help.hxx>
 #include <vcl/vclmedit.hxx>
 #include <o3tl/make_unique.hxx>
-#include <uno/current_context.hxx>
 
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::uno;
@@ -542,36 +542,6 @@ namespace
     }
 }
 
-namespace
-{
-class NoEnableJavaInteractionContext : public cppu::WeakImplHelper<css::uno::XCurrentContext>
-{
-public:
-    explicit NoEnableJavaInteractionContext(
-        css::uno::Reference<css::uno::XCurrentContext> const& xContext)
-        : mxContext(xContext)
-    {
-    }
-    NoEnableJavaInteractionContext(const NoEnableJavaInteractionContext&) = delete;
-    NoEnableJavaInteractionContext& operator=(const NoEnableJavaInteractionContext&) = delete;
-
-private:
-    virtual ~NoEnableJavaInteractionContext() override {}
-
-    virtual css::uno::Any SAL_CALL getValueByName(OUString const& Name) override
-    {
-        if (Name == "DontEnableJava")
-            return css::uno::Any(true);
-        else if (mxContext.is())
-            return mxContext->getValueByName(Name);
-        else
-            return css::uno::Any();
-    }
-
-    css::uno::Reference<css::uno::XCurrentContext> mxContext;
-};
-
-} // namespace
 
 void SfxConfigGroupListBox::FillScriptList(const css::uno::Reference< css::script::browse::XBrowseNode >& xRootNode,
                                            SvTreeListEntry* pParentEntry, bool bCheapChildrenOnDemand)
@@ -581,7 +551,7 @@ void SfxConfigGroupListBox::FillScriptList(const css::uno::Reference< css::scrip
         {
             // tdf#120362: Don't ask to enable disabled Java when filling script list
             css::uno::ContextLayer layer(
-                new NoEnableJavaInteractionContext(css::uno::getCurrentContext()));
+                new comphelper::NoEnableJavaInteractionContext(css::uno::getCurrentContext()));
 
             Sequence< Reference< browse::XBrowseNode > > children =
                 xRootNode->getChildNodes();
diff --git a/include/comphelper/DisableInteractionHelper.hxx b/include/comphelper/DisableInteractionHelper.hxx
new file mode 100644
index 000000000000..7d99112edeaa
--- /dev/null
+++ b/include/comphelper/DisableInteractionHelper.hxx
@@ -0,0 +1,49 @@
+/* -*- 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/.
+ */
+
+#ifndef INCLUDED_COMPHELPER_DISABLEINTERACTIONHELPER_HXX
+#define INCLUDED_COMPHELPER_DISABLEINTERACTIONHELPER_HXX
+
+#include <cppuhelper/implbase.hxx>
+#include <uno/current_context.hxx>
+
+namespace comphelper
+{
+class NoEnableJavaInteractionContext : public cppu::WeakImplHelper<css::uno::XCurrentContext>
+{
+public:
+    explicit NoEnableJavaInteractionContext(
+        css::uno::Reference<css::uno::XCurrentContext> const& xContext)
+        : mxContext(xContext)
+    {
+    }
+    NoEnableJavaInteractionContext(const NoEnableJavaInteractionContext&) = delete;
+    NoEnableJavaInteractionContext& operator=(const NoEnableJavaInteractionContext&) = delete;
+
+private:
+    virtual ~NoEnableJavaInteractionContext() override {}
+
+    virtual css::uno::Any SAL_CALL getValueByName(rtl::OUString const& Name) override
+    {
+        if (Name == "DontEnableJava")
+            return css::uno::Any(true);
+        else if (mxContext.is())
+            return mxContext->getValueByName(Name);
+        else
+            return css::uno::Any();
+    }
+
+    css::uno::Reference<css::uno::XCurrentContext> mxContext;
+};
+
+} // namespace comphelper
+
+#endif // INCLUDED_COMPHELPER_DISABLEINTERACTIONHELPER_HXX
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/scripting/source/provider/MasterScriptProvider.cxx b/scripting/source/provider/MasterScriptProvider.cxx
index 56082753e864..7f91fb83da12 100644
--- a/scripting/source/provider/MasterScriptProvider.cxx
+++ b/scripting/source/provider/MasterScriptProvider.cxx
@@ -18,6 +18,7 @@
  */
 
 
+#include <comphelper/DisableInteractionHelper.hxx>
 #include <comphelper/documentinfo.hxx>
 
 #include <cppuhelper/implementationentry.hxx>
@@ -450,6 +451,49 @@ MasterScriptProvider::parseLocationName( const OUString& location )
     return temp;
 }
 
+namespace
+{
+template <typename Proc> bool FindProviderAndApply(ProviderCache& rCache, Proc p)
+{
+    auto pass = [&rCache, &p]() -> bool
+    {
+        bool bResult = false;
+        for (auto& rProv : rCache.getAllProviders())
+        {
+            Reference<container::XNameContainer> xCont(rProv, UNO_QUERY);
+            if (!xCont.is())
+            {
+                continue;
+            }
+            try
+            {
+                bResult = p(xCont);
+                break;
+            }
+            catch (Exception& e)
+            {
+                SAL_INFO("scripting.provider", "ignoring " << e);
+            }
+        }
+        return bResult;
+    };
+    bool bSuccess = false;
+    // 1. Try to perform the operation without trying to enable JVM (if disabled)
+    // This allows us to avoid useless user interaction in case when other provider
+    // (not JVM) actually handles the operation.
+    {
+        css::uno::ContextLayer layer(
+            new comphelper::NoEnableJavaInteractionContext(css::uno::getCurrentContext()));
+        bSuccess = pass();
+    }
+    // 2. Now retry asking to enable JVM in case we didn't succeed first time
+    if (!bSuccess)
+    {
+        bSuccess = pass();
+    }
+    return bSuccess;
+}
+} // namespace
 
 // Register Package
 void SAL_CALL
@@ -487,29 +531,12 @@ MasterScriptProvider::insertByName( const OUString& aName, const Any& aElement )
                 "insertByName cannot instantiate "
                 "child script providers." );
         }
-        Sequence < Reference< provider::XScriptProvider > > xSProviders =
-            providerCache()->getAllProviders();
-        sal_Int32 index = 0;
-
-        for ( ; index < xSProviders.getLength(); index++ )
-        {
-            Reference< container::XNameContainer > xCont( xSProviders[ index ], UNO_QUERY );
-            if ( !xCont.is() )
-            {
-                continue;
-            }
-            try
-            {
-                xCont->insertByName( aName, aElement );
-                break;
-            }
-            catch ( Exception& e )
-            {
-                SAL_INFO("scripting.provider", "ignoring " << e);
-            }
-
-        }
-        if ( index == xSProviders.getLength() )
+        const bool bSuccess = FindProviderAndApply(
+            *providerCache(), [&aName, &aElement](Reference<container::XNameContainer>& xCont) {
+                xCont->insertByName(aName, aElement);
+                return true;
+            });
+        if (!bSuccess)
         {
             // No script providers could process the package
             throw lang::IllegalArgumentException( "Failed to register package for " + aName,
@@ -550,27 +577,12 @@ MasterScriptProvider::removeByName( const OUString& Name )
                 "removeByName() cannot instantiate "
                 "child script providers." );
         }
-        Sequence < Reference< provider::XScriptProvider > > xSProviders =
-            providerCache()->getAllProviders();
-        sal_Int32 index = 0;
-        for ( ; index < xSProviders.getLength(); index++ )
-        {
-            Reference< container::XNameContainer > xCont( xSProviders[ index ], UNO_QUERY );
-            if ( !xCont.is() )
-            {
-                continue;
-            }
-            try
-            {
-                xCont->removeByName( Name );
-                break;
-            }
-            catch ( Exception& )
-            {
-            }
-
-        }
-        if ( index == xSProviders.getLength() )
+        const bool bSuccess = FindProviderAndApply(
+            *providerCache(), [&Name](Reference<container::XNameContainer>& xCont) {
+                xCont->removeByName(Name);
+                return true;
+            });
+        if (!bSuccess)
         {
             // No script providers could process the package
             throw lang::IllegalArgumentException( "Failed to revoke package for " + Name,
@@ -632,28 +644,10 @@ MasterScriptProvider::hasByName( const OUString& aName )
                 "removeByName() cannot instantiate "
                 "child script providers." );
         }
-        Sequence < Reference< provider::XScriptProvider > > xSProviders =
-            providerCache()->getAllProviders();
-        for ( sal_Int32 index = 0; index < xSProviders.getLength(); index++ )
-        {
-            Reference< container::XNameContainer > xCont( xSProviders[ index ], UNO_QUERY );
-            if ( !xCont.is() )
-            {
-                continue;
-            }
-            try
-            {
-                result = xCont->hasByName( aName );
-                if ( result )
-                {
-                    break;
-                }
-            }
-            catch ( Exception& )
-            {
-            }
-
-        }
+        result = FindProviderAndApply(
+            *providerCache(), [&aName](Reference<container::XNameContainer>& xCont) {
+                return xCont->hasByName(aName);
+            });
     }
     return result;
 }