Use ServiceLoader to load RegExp and XML implementations

Use ServiceLoader to load RegExp and XML implementations This is a more modern way than just classloading and could allow for some more flexibility in the future.
mozilla · Jan 24, 2025 · 812579d · 812579d
1 parent 22f6f86
commit 812579d
Show file tree

Hide file tree

Showing 9 changed files with 80 additions and 50 deletions.
diff --git a/rhino-xml/src/main/java/module-info.java b/rhino-xml/src/main/java/module-info.java
@@ -1,6 +1,9 @@
 module org.mozilla.javascript.xml {
     exports org.mozilla.javascript.xmlimpl;
 
+    provides org.mozilla.javascript.xml.XMLLoader with
+            org.mozilla.javascript.xmlimpl.XMLLoaderImpl;
+
     requires transitive org.mozilla.rhino;
     requires transitive java.xml;
 }
diff --git a/rhino-xml/src/main/java/org/mozilla/javascript/xmlimpl/XMLLoaderImpl.java b/rhino-xml/src/main/java/org/mozilla/javascript/xmlimpl/XMLLoaderImpl.java
@@ -0,0 +1,22 @@
+package org.mozilla.javascript.xmlimpl;
+
+import org.mozilla.javascript.LazilyLoadedCtor;
+import org.mozilla.javascript.ScriptableObject;
+import org.mozilla.javascript.xml.XMLLib;
+import org.mozilla.javascript.xml.XMLLoader;
+
+public class XMLLoaderImpl implements XMLLoader {
+    @Override
+    public void load(ScriptableObject scope, boolean sealed) {
+        String implClass = XMLLibImpl.class.getName();
+        new LazilyLoadedCtor(scope, "XML", implClass, sealed, true);
+        new LazilyLoadedCtor(scope, "XMLList", implClass, sealed, true);
+        new LazilyLoadedCtor(scope, "Namespace", implClass, sealed, true);
+        new LazilyLoadedCtor(scope, "QName", implClass, sealed, true);
+    }
+
+    @Override
+    public XMLLib.Factory getFactory() {
+        return XMLLib.Factory.create(XMLLibImpl.class.getName());
+    }
+}
diff --git a/rhino-xml/src/main/resources/META-INF/services/org.mozilla.javascript.xml.XMLLoader b/rhino-xml/src/main/resources/META-INF/services/org.mozilla.javascript.xml.XMLLoader
@@ -0,0 +1 @@
+org.mozilla.javascript.xmlimpl.XMLLoaderImpl
diff --git a/rhino/src/main/java/module-info.java b/rhino/src/main/java/module-info.java
@@ -11,6 +11,12 @@
     exports org.mozilla.javascript.typedarrays;
     exports org.mozilla.javascript.xml;
 
+    uses org.mozilla.javascript.RegExpProxy;
+    uses org.mozilla.javascript.xml.XMLLoader;
+
+    provides org.mozilla.javascript.RegExpProxy with
+            org.mozilla.javascript.regexp.RegExpImpl;
+
     requires java.compiler;
     requires jdk.dynalink;
     requires transitive java.desktop;

diff --git a/rhino/src/main/java/org/mozilla/javascript/Context.java b/rhino/src/main/java/org/mozilla/javascript/Context.java
@@ -35,6 +35,7 @@
 import org.mozilla.javascript.debug.DebuggableScript;
 import org.mozilla.javascript.debug.Debugger;
 import org.mozilla.javascript.xml.XMLLib;
+import org.mozilla.javascript.xml.XMLLoader;
 
 /**
  * This class represents the runtime context of an executing script.
@@ -2318,11 +2319,19 @@ public boolean hasFeature(int featureIndex) {
      * <p>The default implementation uses the implementation provided by this <code>Context</code>'s
      * {@link ContextFactory}.
      *
+     * <p>This is no longer used in E4X -- an implementation is only provided for backward
+     * compatibility.
+     *
      * @return An XMLLib.Factory. Should not return <code>null</code> if {@link #FEATURE_E4X} is
      *     enabled. See {@link #hasFeature}.
      */
+    @Deprecated
     public XMLLib.Factory getE4xImplementationFactory() {
-        return getFactory().getE4xImplementationFactory();
+        XMLLoader loader = ScriptRuntime.loadOneServiceImplementation(XMLLoader.class);
+        if (loader != null) {
+            return loader.getFactory();
+        }
+        return null;
     }
 
     /**
@@ -2682,10 +2691,7 @@ private static boolean frameMatches(StackTraceElement e) {
 
     RegExpProxy getRegExpProxy() {
         if (regExpProxy == null) {
-            Class<?> cl = Kit.classOrNull("org.mozilla.javascript.regexp.RegExpImpl");
-            if (cl != null) {
-                regExpProxy = (RegExpProxy) Kit.newInstanceOrNull(cl);
-            }
+            regExpProxy = ScriptRuntime.loadOneServiceImplementation(RegExpProxy.class);
         }
         return regExpProxy;
     }

diff --git a/rhino/src/main/java/org/mozilla/javascript/ContextFactory.java b/rhino/src/main/java/org/mozilla/javascript/ContextFactory.java
@@ -295,42 +295,6 @@ protected boolean hasFeature(Context cx, int featureIndex) {
         throw new IllegalArgumentException(String.valueOf(featureIndex));
     }
 
-    private static boolean isDom3Present() {
-        Class<?> nodeClass = Kit.classOrNull("org.w3c.dom.Node");
-        if (nodeClass == null) return false;
-        // Check to see whether DOM3 is present; use a new method defined in
-        // DOM3 that is vital to our implementation
-        try {
-            nodeClass.getMethod("getUserData", String.class);
-            return true;
-        } catch (NoSuchMethodException e) {
-            return false;
-        }
-    }
-
-    /**
-     * Provides a default {@link org.mozilla.javascript.xml.XMLLib.Factory XMLLib.Factory} to be
-     * used by the <code>Context</code> instances produced by this factory. See {@link
-     * Context#getE4xImplementationFactory} for details.
-     *
-     * <p>May return null, in which case E4X functionality is not supported in Rhino.
-     *
-     * <p>The default implementation now prefers the DOM3 E4X implementation.
-     */
-    protected org.mozilla.javascript.xml.XMLLib.Factory getE4xImplementationFactory() {
-        // Must provide default implementation, rather than abstract method,
-        // so that past implementors of ContextFactory do not fail at runtime
-        // upon invocation of this method.
-        // Note that the default implementation returns null if we
-        // neither have XMLBeans nor a DOM3 implementation present.
-
-        if (isDom3Present()) {
-            return org.mozilla.javascript.xml.XMLLib.Factory.create(
-                    "org.mozilla.javascript.xmlimpl.XMLLibImpl");
-        }
-        return null;
-    }
-
     /**
      * Create class loader for generated classes. This method creates an instance of the default
      * implementation of {@link GeneratedClassLoader}. Rhino uses this interface to load generated

diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java b/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java
@@ -14,15 +14,18 @@
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Locale;
 import java.util.Optional;
 import java.util.ResourceBundle;
+import java.util.ServiceLoader;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import org.mozilla.javascript.ast.FunctionNode;
 import org.mozilla.javascript.v8dtoa.DoubleConversion;
 import org.mozilla.javascript.v8dtoa.FastDtoa;
 import org.mozilla.javascript.xml.XMLLib;
+import org.mozilla.javascript.xml.XMLLoader;
 import org.mozilla.javascript.xml.XMLObject;
 
 /**
@@ -194,19 +197,15 @@ public static ScriptableObject initSafeStandardObjects(
         NativeJavaObject.init(scope, sealed);
         NativeJavaMap.init(scope, sealed);
 
-        boolean withXml =
-                cx.hasFeature(Context.FEATURE_E4X) && cx.getE4xImplementationFactory() != null;
-
         // define lazy-loaded properties using their class name
         new LazilyLoadedCtor(
                 scope, "Continuation", "org.mozilla.javascript.NativeContinuation", sealed, true);
 
-        if (withXml) {
-            String xmlImpl = cx.getE4xImplementationFactory().getImplementationClassName();
-            new LazilyLoadedCtor(scope, "XML", xmlImpl, sealed, true);
-            new LazilyLoadedCtor(scope, "XMLList", xmlImpl, sealed, true);
-            new LazilyLoadedCtor(scope, "Namespace", xmlImpl, sealed, true);
-            new LazilyLoadedCtor(scope, "QName", xmlImpl, sealed, true);
+        if (cx.hasFeature(Context.FEATURE_E4X)) {
+            XMLLoader loader = loadOneServiceImplementation(XMLLoader.class);
+            if (loader != null) {
+                loader.load(scope, sealed);
+            }
         }
 
         if (((cx.getLanguageVersion() >= Context.VERSION_1_8)
@@ -5626,6 +5625,24 @@ public static void throwDeleteOnSuperPropertyNotAllowed() {
         throw referenceError("msg.delete.super");
     }
 
+    /**
+     * Load a single implementation of "serviceClass" using the ServiceLoader. If there are no
+     * implementations, return null. If there is more than one implementation, throw a fatal
+     * exception, since this indicates that the classpath was configured incorrectly.
+     */
+    static <T> T loadOneServiceImplementation(Class<T> serviceClass) {
+        Iterator<T> it = ServiceLoader.load(serviceClass).iterator();
+        if (it.hasNext()) {
+            T result = it.next();
+            if (it.hasNext()) {
+                throw Kit.codeBug(
+                        "Invalid configuration: more than one implementation of " + serviceClass);
+            }
+            return result;
+        }
+        return null;
+    }
+
     public static final Object[] emptyArgs = new Object[0];
     public static final String[] emptyStrings = new String[0];
 }
diff --git a/rhino/src/main/java/org/mozilla/javascript/xml/XMLLoader.java b/rhino/src/main/java/org/mozilla/javascript/xml/XMLLoader.java
@@ -0,0 +1,10 @@
+package org.mozilla.javascript.xml;
+
+import org.mozilla.javascript.ScriptableObject;
+
+/** This interface is used to load the XML implementation using the ServiceLoader pattern. */
+public interface XMLLoader {
+    void load(ScriptableObject scope, boolean sealed);
+
+    XMLLib.Factory getFactory();
+}
diff --git a/rhino/src/main/resources/META-INF/services/org.mozilla.javascript.RegExpProxy b/rhino/src/main/resources/META-INF/services/org.mozilla.javascript.RegExpProxy
@@ -0,0 +1 @@
+org.mozilla.javascript.regexp.RegExpImpl