Type Checking for MutationRecord gives ClassCastException in Chrome

[MorganPHudson]

The follow code gives ClassCastException in Chrome(Version 83.0.4103.61 (Official Build) (64-bit)) and new Edge(Version 83.0.478.37 (Official build) (64-bit)) but not Edge Legacy

    private void setupMutation(Node target) {
        Consumer<MutationRecord> recordConsumer = mutationRecord -> DomGlobal.console.log("oldValue", mutationRecord.oldValue);

        MutationObserverInit mutationObserverInit = MutationObserverInit.create();
        mutationObserverInit.setAttributes(true);
        mutationObserverInit.setChildList(true);
        mutationObserverInit.setSubtree(true);

        MutationObserver mutationObserver = new MutationObserver(new MutationObserver.MutationObserverCallbackFn() {
            public Object onInvoke(JsArray<MutationRecord> recordJsArray, MutationObserver observer) {
                recordJsArray.forEach((recordConsumer1, ignore, params) -> {
                    recordConsumer.accept(recordConsumer1);
                    return null;
                });

                return null;
            }
        });

        mutationObserver.observe(target, mutationObserverInit);
    }

Below is the type check that fails for MutationObserver. In the console there is a test againt $wnd and window for the mutationRecord instance

And the generated code where castToNative is used.

[niloc132]

We iterated on this a bit in gitter, and here's our conclusion:

The original code that worked in elemtental2 1.0.0-RC1 worked fine, using

for (MutationRecord record : recordJsArray)...

since MutationObserver.MutationObserverCallbackFn provided MutationRecord[] recordJsArray as an argument - a java array, so no casting was done on the way out. Now, in 1.0.0 with the shift to JsArray<MutationRecord> recordJsArray, the generics are checked at runtime on the way out when assigning to record for a for-each loop, or when calling JsArray.forEach and invoking the lambda (as in the original report above).

The bug in Chrome is that instead of the objects being an instance of the global window where the mutation occurred, it is an instance of the iframe in which GWT code runs.

This is not a new breakage in Chrome however - it appears to have been wrong before, but it was easier to work around before, since you could iterate the generic collection. The workaround for this JsArray problem is to cast to JsArray<Object>, iterate those Objects instead, and then Js.uncheckedCast them to MutationRecord.

Perhaps change MutationRecord to be some kind of @interface or @record so that it isn't subject to these checks? It doesn't seem like a very satisfying solution, but it does get around Chrome's bug here.

[jDramaix]

It seems to me to be a problem in GWT. I'll ask @rluble to take a look.

Perhaps change MutationRecord to be some kind of @interface or @record so that it isn't subject to these checks? It doesn't seem like a very satisfying solution, but it does get around Chrome's bug here.

This is not a solution. MutationRecord is a valid token at runtime and MutationRecord has a prototype and constructor (Even if you are not allowed to call it directly) so it needs to be a class.

[rluble]

I am not sure there is a good solution here. elemental2 is generated from the closure definitions and the definition is for all purposes correct.

The alternative is to use a workaround like

for(Object o : recordJsArray) {
  MutationRecord rec = Js.unckeckedCast(o);
}

Since this is a browser issue I think that a workaround here is the proper solution.

Since JsArray is not iterable you can use raw types for that purpose, e.g.:

  JsArray rawJsarray = recordJsArray;
  rawJsArray.forEach((recordButTypedAsObject, .......) ->
         blah.consume(Js.uncheckedCast(recordButTypedAsObject));

[niloc132]

@rluble yes, the first workaround is what we did after figuring out what chrome was doing wrong (though we cast the array to JsArray<Object> first, I didn't realize we could just change the type of the "next" variable in the loop).

I think the second solution might be better off without using raw types? I frequently run into issues when mixing raw types with lambdas/references in normal javac, and imagine similar issues could arise in jdt.

@jDramaix agreed that this isn't a solution, but GWT isn't the problem here, Chrome is. There are lots of little bugs like this across various browsers, and having to make Java uglier to deal with them is a bit cumbersome. That is what this bug was meant to convey. I think this is the third or so case of this - I don't think there is an easy solution for these problems, but just opening up a conversation to see if sometimes comes up (and making it easier for future developers who hit this to know how to work around it).

[rluble]

Other possible solution is to change GWT to have a yet another custom casting logic for JsPackage.GLOBAL classes; so that at runtime the logic becomes x instanceof $wnd.Blah || x instanceof window.Blah. It seems to me that for most cases when you reference the global scope you actually don't care if it is the iframe or the top window, especially from the instanceof/cast perspective.

I could review the patch if you want to do such a change but it is not a completely trivial change.

The relevant classes are ImplementCastsAndTypeChecks.java that has the main transformation logic, TypeCategory that defines the types of things and Casts.java that has the runtime support.

[niloc132]

Yeah, and especially if you get into "more than one running gwt module on the page" territory, I'd worry that this still wouldn't be complete. Hard to say if that is a reasonable use with this edge case or not.

Additionally, it doesn't solve things like #16 (comment), where each IFrameElement has a Window contentWindow field, but this window would fail a instanceof $wnd.Window and instanceof window.Window check.

What are your thoughts with regard to j2cl and this kind of issue?

[rluble]

What are your thoughts with regard to j2cl and this kind of issue?

J2CL does not make any assumption about the nature of the global scope. Whenever you say JsPackage.GLOBAL, it just means that it should be emitted unqualified. So it would take whatever global context the code is in. It would work the same as if you had written `x instanceof SomeObject. So if the extern does not model correctly the runtime environment you would have the same problem writing native JavaScript.

I know that JavaScript does not perform instanceof automagically but hey I don't think there is much we can do regarding jsinterop_generator ATM.

[gkdn]

Didn't read all the discussion but if Chrome has a bug here, we can suggest users to workaround this which should be easy to do so:

for (MutationRecord m: recordJsArray.toArray(new MutationRecord[0])) {
  ...
}

At one point, we should figure out a way to 'optionally' disable type checking for all Elemental.

[omriyariv]

Bumped into the same issue. Seems to be wider than just the MutationRecord - the DOM nodes which are monitored by the MutationObserver are created as instances of the inner iFrame. This propagates through the entire app as any naive cast of a Node/Element/HTMLElement (e.g. event.target in event handlers) also throws.

The ultimate workaround I'm looking for is the ability to disable type checking in SuperDevMode. Is there maybe a way to do that? -XdisableCastChecking does not work in SuperDevMode, see gwtproject/gwt#9318.

[omriyariv]

I was able to workaround this by forcing the DevMode linker to run the app code on the main window. This has drawbacks like not being able to use code splitting (I think...)

Anyway this thing solved it for me, perhaps the annotations are redundant:

import com.google.gwt.core.ext.LinkerContext;
import com.google.gwt.core.ext.linker.LinkerOrder;
import com.google.gwt.core.linker.CrossSiteIframeLinker;

@LinkerOrder(LinkerOrder.Order.PRIMARY)
@GwtIncompatible
public class MainWindowDevModeLinker extends CrossSiteIframeLinker {

  @Override
  protected String getJsInstallLocation(LinkerContext context) {
    return "com/google/gwt/core/ext/linker/impl/installLocationMainWindow.js";
  }
}