GWT and WebKit issues

[Jens]

Hi,

I just wanted to start a discussion on all the WebKit issues that appeared after the release of Safari 6 / iOS 6. To get an impression here are some links I found so far:

GWT Group:

https://groups.google.com/d/topic/google-web-toolkit/gh7NbjaSb-k/discussion

https://groups.google.com/d/topic/google-web-toolkit/Snj52ylNSF4/discussion

https://groups.google.com/d/topic/google-web-toolkit/hduYnZG8bws/discussion

https://groups.google.com/d/topic/google-web-toolkit/ZmMNVGWRkVU/discussion

Vaadin:

http://dev.vaadin.com/ticket/10722

http://dev.vaadin.com/ticket/10753

Stackoverflow:

http://stackoverflow.com/questions/12534409/ios-6-javascipt-intermittent-issues-with-object-defineproperty

http://stackoverflow.com/questions/13147026/disabling-jit-in-safari-6-to-workaround-severe-javascript-jit-bugs

The GWT groups discussions are mainly dealing with random app crashes caused by ClassCastExceptions. Often variables of type HashMap<String, ?> are involved. Problems with HashMaps containing String keys are also reported to Vaadin.

The last bold link is the one I am stumbled upon yesterday and its pretty interesting. It states that WebKit's JIT compiler has some serious bugs and it also contains a working proof of concept demonstrating an issue where var++ and var-- do not work correctly when JIT is active. 

This happens on iOS 6 devices, WebKit nightly builds, Safari 6 in Mac OS 10.7 and 10.8. So I would guess in most or even all WebKit based browsers that have JIT enabled. 

As soon as you connect to an iOS device using Safari for remote debugging or when you open the Safari DevTools on Mac OS the issues disappear. I would assume that WebKit simply disables JIT as soon as a debugger runs and thats why all issues disappear magically. This would also explain why Google Chrome for iOS does not suffer from these issues because I think only Safari has access to the JIT compiler on iOS for security reasons.

Currently as soon as a customer uses Safari/WebKit (maybe except Chrome because of V8) to access a GWT app it is not guaranteed to work. We just tested it with our application on Mac OS 10.7 / 10.8 Safari 6 and discovered that it indeed has strange random behavior if you only try it often enough. In our case for example we have a Map and containsKey with a String key randomly stops working which results in wrong information shown in the UI. So before today we were under the impression that its an iOS bug only (which is still present in iOS 6.1) but it seems like its more wide spread.

So whats your opinion on this? Should GWT try to workaround these issues somehow? On Stackoverflow a suggested workaround is to disable JIT for a given method by wrapping the methods content in a try catch block that re-throws possible exceptions. Would it be possible to introduce a flag to GWT that enables such specific "hacks" for Safari permutations? As it sounds like that JIT compilation is the main issue its maybe worth it to give it a try?

The author on stackoverflow already has an Apple ticket (ID 12606761) and we also have created a ticked for WebKit itself which needs to be confirmed and some votes I guess: https://bugs.webkit.org/show_bug.cgi?id=109036

Do you guys at Google know these issues and working on it? Don't you have problems with your own GWT apps?

Currently we have to tell customers to not use Safari at all and use Chrome app on iOS devices. The whole situation is somewhat frustrating.

-- J.

[John A. Tamplin]

On Wed, Feb 6, 2013 at 7:03 AM, Jens <jens.ne...@gmail.com> wrote:

So whats your opinion on this? Should GWT try to workaround these issues somehow? On Stackoverflow a suggested workaround is to disable JIT for a given method by wrapping the methods content in a try catch block that re-throws possible exceptions. Would it be possible to introduce a flag to GWT that enables such specific "hacks" for Safari permutations? As it sounds like that JIT compilation is the main issue its maybe worth it to give it a try?

The proposed workarounds seem completely unrealistic -- are you going to wrap every single statement?  With a problem this severe, it seems like they will have to fix it quickly.

Also, you would penalize Chrome users, since they share the same permutation.

 

Currently we have to tell customers to not use Safari at all and use Chrome app on iOS devices. The whole situation is somewhat frustrating.

Does Chrome on iOS avoid the problem?  I thought V8 wasn't used on iOS due to Apple policies.

--
John A. Tamplin

[Jens]

The proposed workarounds seem completely unrealistic -- are you going to wrap every single statement?

You would wrap the whole method body with a single JS try/catch/re-throw if I understand it correctly.

 

 With a problem this severe, it seems like they will have to fix it quickly.

Hopefully, but maybe Google can rise some attention so that it will be fixed quickly? Isn't Google a member of the WebKit group?

 

Does Chrome on iOS avoid the problem?  I thought V8 wasn't used on iOS due to Apple policies.

Chrome on iOS avoids the problem because Apple only allows mobile Safari to use the JIT compiler. So every app on iOS that uses UIWebView to render web pages (including Chrome) do not have JIT compilation enabled for security reasons and thus don't have these problems.

-- J.

[Ray Cromwell]

We added a pass to GWT before to work around a Safari Nitro bug with the shift operator (>>), we could do something similar. See here: http://code.google.com/p/google-web-toolkit/source/browse/trunk/dev/core/src/com/google/gwt/dev/js/JsCoerceIntShift.java

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
 
 

[John A. Tamplin]

On Wed, Feb 6, 2013 at 11:51 PM, Ray Cromwell <cromw...@google.com> wrote:

We added a pass to GWT before to work around a Safari Nitro bug with the shift operator (>>), we could do something similar. See here: http://code.google.com/p/google-web-toolkit/source/browse/trunk/dev/core/src/com/google/gwt/dev/js/JsCoerceIntShift.java

Speaking of that hack, maybe it is time to remove it :).

But in this case, it sounds like a much more expensive workaround. 

--
John A. Tamplin

[Jens]

We added a pass to GWT before to work around a Safari Nitro bug with the shift operator (>>), we could do something similar. See here: http://code.google.com/p/google-web-toolkit/source/browse/trunk/dev/core/src/com/google/gwt/dev/js/JsCoerceIntShift.java

Speaking of that hack, maybe it is time to remove it :).

But in this case, it sounds like a much more expensive workaround. 

Rereading the Stackoverflow article and all comments it seems like that you actually have two options:

1.) wrapping a method body that contains a++ / a-- with try/catch/re-throw to disable JIT for that method. 

2.) Only fix the a++ (and a--) operator by rewriting it to a = a + 1 or (a, a++)

I'll give both a try and test it against our app to see how it plays out.

-- J.

[Jens]

I first tried to write a visitor that rewrites a++ and a-- but there are quite some cases to consider for correctly rewriting these unary operations in different contexts. 

So for now I have implemented a visitor that rewrites all functions in endVisit(JsFunction ...) by wrapping the original function body with a try/catch/throw. I don't know if I also have to update the SourceInfo instance somehow because the rewritten function has different line numbers?! Currently the implementation looks like:

@Override
public void endVisit(final JsFunction x, final JsContext ctx) {

  SourceInfo sourceInfo = x.getSourceInfo();

  //catch block
  JsBlock catchBody = new JsBlock(sourceInfo);
  JsCatch catchNode = new JsCatch(sourceInfo, x.getScope(), "e");
  catchNode.setBody(catchBody);

  //statement: throw e
  JsParameter catchParam = catchNode.getParameter();
  JsNameRef catchParamNameRef = new JsNameRef(sourceInfo, catchParam.getName());
  JsThrow throwNode = new JsThrow(sourceInfo, catchParamNameRef);
  catchBody.getStatements().add(throwNode);

  //try block with original method body and with catch added.
  JsTry tryNode = new JsTry(sourceInfo);
  tryNode.getCatches().add(catchNode);
  tryNode.setTryBlock(x.getBody());

  //new function body with try block added.
  JsBlock functionBody = new JsBlock(sourceInfo);
  functionBody.getStatements().add(tryNode);
  x.setBody(functionBody);

}

We recompiled our app to test the result using iPhone/iPad and it seems like that all issues are solved in Safari 6. We did not recognize performance issues but obviously that greatly depends on the type of app you have.

I am not sure if you want that visitor in GWT temporarily, as every function will be rewritten. On the other hand this approach may fix a lot more (still unknown) Nitro JIT compiler issues. One could add a GWT compiler option to activate such invasive workarounds. 

So if you want, I could create an issue/patch for it..with or without an option to activate the visitor. 

-- J.

[Roberto Lublinerman]

Other option is to rewrite a++ as (a = a+1, a-1) which preserves the semantics and probably as less of a performance impact.

Roberto Lublinerman | Software Engineer | rlu...@google.com | 408-500-9148

-- J.

--

[Jens]

Other option is to rewrite a++ as (a = a+1, a-1) which preserves the semantics and probably as less of a performance impact.

Also an option but one of the main problems I had while trying to rewrite a++ was that I couldn't figure out how to change operator precedence.

The first thing I have done was modifying the JsVisitor so that it also visits JsUnaryOperation. Once I had done this I could replace the JsUnaryOperation with a comma JsBinaryOperation. This worked fine but after taken a look at the compiled JS code I realized that all these comma operators have been written out without braces. This could create trouble as the comma operator has least precedence and in fact code like "var x = a++;" that probably would have been rewritten to "var x = a, a++;" would break as its invalid JS. So I thought I need to increase the operator precedence to the maximum but I couldn't figure out how to do that and if its correct in all cases. If its not correct in all cases I would also have to detect all cases were I can't increase the precedence (or the other way around: detect cases when I need to increase it).

Thats when I stopped working on it and switched to the try/catch/throws approach to get a working solution for our app first.

-- J.

[John A. Tamplin]

On Fri, Feb 8, 2013 at 3:02 PM, Jens <jens.ne...@gmail.com> wrote:

Also an option but one of the main problems I had while trying to rewrite a++ was that I couldn't figure out how to change operator precedence.

 

You are dealing with an AST at this point, so precedence is irrelevant.

 

The first thing I have done was modifying the JsVisitor so that it also visits JsUnaryOperation. Once I had done this I could replace the JsUnaryOperation with a comma JsBinaryOperation. This worked fine but after taken a look at the compiled JS code I realized that all these comma operators have been written out without braces.

If the generated JS doesn't reflect operator precedence, then it would be a bug there.  However, given how much code has run through it at this point, I would be very surprised if such a bug existed at this point.  Do you have a concrete example where incorrect JS is generated?

--
John A. Tamplin

[Jens]

Am Freitag, 8. Februar 2013 21:21:00 UTC+1 schrieb John A. Tamplin:

On Fri, Feb 8, 2013 at 3:02 PM, Jens <jens.ne...@gmail.com> wrote:

Also an option but one of the main problems I had while trying to rewrite a++ was that I couldn't figure out how to change operator precedence.

 

You are dealing with an AST at this point, so precedence is irrelevant.

Ah! Ok makes sense, somehow missed that part.

 

The first thing I have done was modifying the JsVisitor so that it also visits JsUnaryOperation. Once I had done this I could replace the JsUnaryOperation with a comma JsBinaryOperation. This worked fine but after taken a look at the compiled JS code I realized that all these comma operators have been written out without braces.

If the generated JS doesn't reflect operator precedence, then it would be a bug there.  However, given how much code has run through it at this point, I would be very surprised if such a bug existed at this point.  Do you have a concrete example where incorrect JS is generated?

It was just an assumption because some code constructs are not available in our app's javascript, e.g. we don't have var x = a++, and I missed the AST thingy. So I thought this could maybe rewritten wrong.

Unfortunately the visitor that replaces a++ with (a, a++) did not solve all issues in our app while the visitor using try/catch/throw does solve everything (at the cost of performance). So it seems like there are still other bugs in Nitro beside the a++ thing. For example our HashMaps with String keys work with the a++ replacement but on the other hand if we reload the app at a given Place the we are still redirected to the default place randomly.

When I have time I will try the alternative replacement (a = a + 1, a - 1) mentioned by Roberto to check if it makes any difference.

-- J.

[John A. Tamplin]

I'm not sure why (a,a++) would do anything -- you still have the a++, and any decent compiler should be able to throw away any expression without side effects on the left of a comma expression.

--
John A. Tamplin

[Jens]

I'm not sure why (a,a++) would do anything -- you still have the a++, and any decent compiler should be able to throw away any expression without side effects on the left of a comma expression.

Hm you are probably right. The linked stackoverflow article states that changing line 29 in http://jsfiddle.net/yGuy/L6t5G/ from b.$f = a++; to b.$f = a, a++; will solve the issue with a++. While this is true (try it yourself on any Safari 6) I think when using GWT's visitor a replacement from a++ to a, a++ would result in JS code b.$f = (a, a++); because I am working on an AST and I am replacing the a++ node. 

In fact when you use the jsfiddle link and change line 29 to  b.$f = (a, a++); the issue is still present. Seems somewhat logical that b.$f = (a, a++); is optimized back to b.$f = a++; while b.$f = a, a++; won't be optimized.

If you use Roberto's alternative in jsfiddle and change the line to b.$f = (a = a + a, a - 1); everything works as well. So I will definitely try this one again.

-- J.

[Jens]

 b.$f = (a = a + a, a - 1);

Typo: b.$f = (a = a + 1, a - 1); 

[Jens]

Ok I have tried it again but replacing a++ with Roberto's version only solves some issues. Also tried to replace ++a, a-- and --a using the corresponding replacement strategy but some issues still remain. So its not a viable solution.

To make our app run reliably on Safari 6 we have to stick to the try/catch/throw approach. I guess we have to tweak things a bit so the visitor only executes for an iOS6 Safari permutation and then hope that these issues will be fixed in WebKit/Nitro someday.

Thanks anyways :-)

-- J.

[Roberto Lublinerman]

Just bear in mind that a++ == (a = a + 1, a -1) only if a does not have side effects (which I suspect is always the case but I am not sure).

Roberto Lublinerman | Software Engineer | rlu...@google.com | 408-500-9148

On Wed, Feb 13, 2013 at 3:23 AM, Jens <jens.ne...@gmail.com> wrote:

 b.$f = (a = a + a, a - 1);

Typo: b.$f = (a = a + 1, a - 1); 

--