bug in permutations.js?

[Stephen Haberman]

Hey,

We upgraded to GWT 2.6 last week, and our seeing weirdness with
StyleInjector. Firefox ends up using the StyleInjectImplIE and errors
out because $doc.createStyleSheet is IE only.

FF gets the wrong deferred binding because its permutationId is
undefined, when it should be 0. (We use collapse-all-properties and
direct_install linker.)

Looking at permutations.js, it does "var softPermutationId", then later
when parsing the strongName, our FF strongName doesn't have ":" in it
(see below), so it ends effectively up doing:

module.__softPermutationId = undefined;

In our module.nocache.js file, here's the gecko1_8 entry:

unflattenKeylistIntoAnswers(['gecko1_8'],
'9181777BF8BB65802D36B21DCBB83FE1');

No ":0" at the end.

So, surely __softPermutationId being undefined is a bug?

Should I try fixing this by changing getCompiledCodeFilename's
softPermutationId to default to 0? Or is the problem with the strong
name itself, in that it should always have a :<digit> suffix?

Hm. PermutationsUtil:131 insinuates :0 is left off on purpose.

Looking at git log/git blame, doesn't seem like any of this has changed
recently? We had been running some post-2.5 GWT trunk.

- Stephen

[Brian Slesinsky]

Yes it sounds like a bug. Want to add that to the issue tracker?

I wonder why more people aren't seeing this? Does it only affect soft permutations?

- Brian

- Stephen

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-co...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Colin Alworth]

In CrossSiteIframeTemplate.js this is handled by assigning __MODULE_FUNC__.__softPermutationId to 0 to begin with, and then only change that value if : was present in the permutation string. I'm not seeing any other js files that init __softPermutationId to 0, and only permutations.js assigns it to another value (but does so conditionally, so it never *assigns* undefined to __softPermutationId, it just leaves it not defined).

    var idx = strongName.indexOf(':');
    if (idx != -1) {
      softPermutationId = parseInt(strongName.substring(idx + 1), 10);
      strongName = strongName.substring(0, idx);
    }

In researching https://code.google.com/p/google-web-toolkit/issues/detail?id=8539 recently, this seemed to make sense (except in the context of the bug, where the permutationId in CollapsedPropertyHolder where it gets overwritten when it loads in a split point...). Looking at permutations.js, it defines a method getCompiledCodeFilename(), but from a grep of the codebase, this is only ever called from CrossSiteIframeTemplate.js, which as mentioned above, sets a default value of zero.

The key direct_install maps to com.google.gwt.core.linker.DirectInstallLinker, which extends CrossSiteIframeLinker, and overrides getJsInstallScript to use com/.../installScriptDirect.js instead of com/.../installScriptEarlyDownload.js and change another boolean, but neither of those appear to block getSelectionScriptTemplate from being used to determine the actual selection script. Which should then mean that softPermutationId gets init'd to zero.

That's just at a quick glance - aside from that I only have an absence of evidence of the bug but no evidence of absence. We use a lot of soft permutations, and 8539 is the only time we've seen an issue (where the first time the value gets used is in a split point) - any chance that is what is biting you instead?

[Stephen Haberman]

> and only permutations.js assigns it to another value (but does so
> conditionally, so it never *assigns* undefined to
> __softPermutationId, it just leaves it not defined).

But line 61:

https://gwt.googlesource.com/gwt/+/master/dev/core/src/com/google/gwt/core/ext/linker/impl/permutations.js

Always assigns module.__softPermutationId to the value of the
softPermutationId local variable, declared on line 23. And it will be
undefined if idx == -1. Right?

That is what's happening when I debug through our app in FF.

> any chance that is what is biting you instead?

We have split points disabled.

- Stephen

[Stephen Haberman]

> Want to add that to the issue tracker?

Sure, I'll do that soon...

> I wonder why more people aren't seeing this? Does it only affect soft
> permutations?

Removing collapse-all-properties does fix it.

I'll do that for now and then work on submitting a patch. No idea how I
would add a test for it.

- Stephen

[Stephen Haberman]

Here is the difference in the generated JS. From a ~10/2013 trunk build:

function com_google_gwt_dom_client_StyleInjector_StyleInjectorImpl(){
switch (permutationId) {
case 1:
case 2:
case 3:
return new StyleInjector$StyleInjectorImplIE_0;
}
return new StyleInjector$StyleInjectorImpl_0;
}

So, even when permutationId was undefined (which it was for gecko_18,
which was basically permutation 0), it was fine, it'd get the non-IE
version.

But now with 2.6:

function com_google_gwt_dom_client_StyleInjector_StyleInjectorImpl(){
switch (permutationId) {
case 0:
case 4:
return new StyleInjector$StyleInjectorImpl_0;
}
return new StyleInjector$StyleInjectorImplIE_0;
}

The output changed; now when permutationId is undefined, which it still
is for gecko_1.8 (no change there), then IE is the default.

AFAICT the DOM.gwt.xml rules for StyleInjector haven't changed
recently. The permutations.js hasn't changed (permutationId was
undefined both pre-/post-2.6).

So, I can file a patch to make permutationId=0, but any ideas why this
would have changed since October-ish?

It must have been that, previously, "case 0" would never have been put
inside the runtime switch statement, and instead always have been the
fallback.

- Stephen

[Stephen Haberman]

> Yes it sounds like a bug. Want to add that to the issue tracker?

https://code.google.com/p/google-web-toolkit/issues/detail?id=8575

I've verified that the patch fixes the behavior in our application.

Any good suggestions about how to test this? Or volunteers to review
the patch?

Given it was, I assume, a bug in permutations.js, I imagine I would
have to create a mini test app with collapse-all-properties, have it
compile to JS, and then somehow verify that the right deferred binding
for permutation 0 (which ever browser that happened to be) got the
right selection.

Thanks,
Stephen

[Brian Slesinsky]

Maybe turn on soft permutations in a sample app, since we do at least test those manually before a release.

Long-term, I'd like to see us using soft permutations by default, perhaps to collapse some browser permutations. If it were more commonly used then we'd likely notice that it's broken pretty quickly.

- Brian

Stephen