[Libreoffice-commits] core.git: cui/source include/comphelper scripting/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Mon Oct 8 14:12:44 UTC 2018


 cui/source/customize/cfgutil.cxx                   |   34 -----
 include/comphelper/DisableInteractionHelper.hxx    |   49 ++++++++
 scripting/source/provider/MasterScriptProvider.cxx |  126 ++++++++++-----------
 3 files changed, 111 insertions(+), 98 deletions(-)

New commits:
commit f3ce30ec75a4d7116b9cd4d7b21d9aaa0e237eeb
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Sat Oct 6 17:08:57 2018 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Mon Oct 8 16:12:20 2018 +0200

    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 at collabora.com>

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;
 }


More information about the Libreoffice-commits mailing list