Disallow private and package-private fields in JSObject subclasses

This was the original intent. This way, not state is split between the
JS and Java mirror. It is also the less confusing behavior, allowing
private, fields, but not exposing them to JS, could be unexpected

Author: patrick96

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/codegen/JSBodyFeature.java

@@ -125,7 +125,7 @@ private boolean canBeJavaScriptCall(AnalysisMethod method) {
 
             @Override
             public boolean handleLoadField(GraphBuilderContext b, ValueNode object, ResolvedJavaField field) {
-                if (ClassWithMirrorLowerer.isJSObjectSubtype(((AnalysisType) field.getDeclaringClass()).getJavaClass())) {
+                if (ClassWithMirrorLowerer.isFieldRepresentedInJavaScript(field)) {
                     genJSObjectFieldAccess(b, object, field, null);
                     return true;
                 }
@@ -134,7 +134,7 @@ public boolean handleLoadField(GraphBuilderContext b, ValueNode object, Resolved
 
             @Override
             public boolean handleStoreField(GraphBuilderContext b, ValueNode object, ResolvedJavaField field, ValueNode value) {
-                if (ClassWithMirrorLowerer.isJSObjectSubtype(((AnalysisType) field.getDeclaringClass()).getJavaClass())) {
+                if (ClassWithMirrorLowerer.isFieldRepresentedInJavaScript(field)) {
                     genJSObjectFieldAccess(b, object, field, value);
                     return true;
                 }

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/codegen/oop/ClassLowerer.java

@@ -36,7 +36,6 @@
 import java.util.function.Consumer;
 import java.util.function.Function;
 
-import org.graalvm.webimage.api.JSObject;
 import org.graalvm.webimage.api.JSResource;
 
 import com.oracle.svm.hosted.meta.HostedField;
@@ -133,8 +132,8 @@ private void lowerClassHeader() {
          * we only collect the instance fields declared by this type, inherited fields are resolved
          * via a super call
          */
-        if (!JSObject.class.isAssignableFrom(type.getJavaClass())) {
-            for (HostedField field : type.getInstanceFields(false)) {
+        for (HostedField field : type.getInstanceFields(false)) {
+            if (!ClassWithMirrorLowerer.isFieldRepresentedInJavaScript(field)) {
                 genFieldInitialization(codeGenTool, masm, field);
             }
         }

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/codegen/oop/ClassWithMirrorLowerer.java

@@ -26,7 +26,9 @@
 
 import static com.oracle.svm.hosted.webimage.codegen.RuntimeConstants.RUNTIME_SYMBOL;
 
+import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
 
@@ -35,6 +37,7 @@
 import org.graalvm.webimage.api.JS;
 import org.graalvm.webimage.api.JSObject;
 
+import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider;
 import com.oracle.svm.hosted.classinitialization.ClassInitializationSupport;
 import com.oracle.svm.hosted.meta.HostedClass;
 import com.oracle.svm.hosted.meta.HostedField;
@@ -55,6 +58,7 @@
 import jdk.graal.compiler.hightiercodegen.CodeBuffer;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.options.OptionValues;
+import jdk.vm.ci.meta.ResolvedJavaField;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 import jdk.vm.ci.meta.Signature;
 
@@ -189,6 +193,17 @@ public ClassWithMirrorLowerer(OptionValues options, DebugContext debug, JSCodeGe
         this.externClassDescriptor = null;
     }
 
+    /**
+     * Public and protected fields in {@link JSObject} subclasses are represented in the JavaScript
+     * mirror.
+     * <p>
+     * Accesses to those fields must be intercepted. The fields also do not appear in the Java
+     * object.
+     */
+    public static boolean isFieldRepresentedInJavaScript(ResolvedJavaField field) {
+        return !field.isStatic() && isJSObjectSubtype(OriginalClassProvider.getJavaClass(field.getDeclaringClass()));
+    }
+
     /**
      * An imported Javascript class needs extern file for Closure compiler if the source code is not
      * included.
@@ -205,6 +220,18 @@ public static boolean isJSObjectSubtype(Class<?> cls) {
         return JSObject.class.isAssignableFrom(cls);
     }
 
+    public static List<HostedField> getOwnFieldOnJSSide(HostedType type) {
+        List<HostedField> fields = new ArrayList<>();
+
+        for (HostedField instanceField : type.getInstanceFields(false)) {
+            if (isFieldRepresentedInJavaScript(instanceField)) {
+                fields.add(instanceField);
+            }
+        }
+
+        return fields;
+    }
+
     @Override
     public void lower(WebImageTypeControl typeControl) {
         if (needExternDeclaration()) {
@@ -228,7 +255,7 @@ protected void lowerPreamble(JSCodeGenTool tool) {
 
             if (needExternDeclaration()) {
                 // We need to mark the fields in the externs file.
-                for (HostedField field : type.getInstanceFields(false)) {
+                for (HostedField field : getOwnFieldOnJSSide(type)) {
                     externClassDescriptor.addProperty(field.getName());
                 }
             }
@@ -302,7 +329,7 @@ private void genJavaScriptMirrorConstructor(JSCodeGenTool tool, JSCodeBuffer buf
         }
 
         // Initialize properties.
-        for (HostedField field : type.getInstanceFields(false)) {
+        for (HostedField field : getOwnFieldOnJSSide(type)) {
             tool.genResolvedVarDeclThisPrefix(field.getName());
             genDefaultValue(tool, buffer, field);
             tool.genResolvedVarDeclPostfix(null);

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/js/JSObjectAccessMethodSupport.java

@@ -72,6 +72,8 @@ private AnalysisMethod lookup(AnalysisMetaAccess aMetaAccess, AnalysisField fiel
         GraalError.guarantee(JSObject.class.isAssignableFrom(field.getDeclaringClass().getJavaClass()), "Field must be in JSObject class: %s", field);
         GraalError.guarantee(!field.isStatic(), "Field must not be static: %s", field);
         UserError.guarantee(!field.isFinal(), "Instance fields in subclasses of %s must not be final: %s", JSObject.class.getSimpleName(), field.format("%H.%n"));
+        UserError.guarantee(field.isPublic() || field.isProtected(), "Only public and protected instance fields in subclasses of %s are allowed: %s", JSObject.class.getSimpleName(),
+                        field.format("%H.%n"));
         JSObjectAccessMethod accessMethod = accessMethods.computeIfAbsent(
                         new AccessorDescription(field, isLoad),
                         key -> createAccessMethod(aMetaAccess, field, isLoad));

web-image/src/com.oracle.svm.webimage.jtt/src/com/oracle/svm/webimage/jtt/api/JSObjectSubclassTest.java

@@ -30,6 +30,7 @@
 
 public class JSObjectSubclassTest {
     public static final String[] OUTPUT = {
+                    // jsObjectSubclass
                     "Declared(made in Java)",
                     "made in Java",
                     "Declared(made in JavaScript)",
@@ -56,12 +57,15 @@ public class JSObjectSubclassTest {
                     "inner imported in Java",
                     "InnerImported(inner exported from JavaScript)",
                     "inner exported from JavaScript",
+                    // exportedReturningObject
                     "non-exported subclass",
                     "non-exported",
                     "non-exported subclass",
                     "non-exported",
+                    // heapGeneratedObjects
                     "5",
                     "8",
+                    // nonInstantiatedImported
                     "7.0",
     };
 

web-image/src/com.oracle.svm.webimage.jtt/src/com/oracle/svm/webimage/jtt/api/JavaDocExamplesTest.java

@@ -189,7 +189,7 @@ class Rectangle extends JSObject {
 
 @JS.Export
 class Randomizer extends JSObject {
-    private Random rng = new Random(719513L);
+    protected Random rng = new Random(719513L);
 
     public byte[] randomBytes(int length) {
         byte[] bytes = new byte[length];