Last Comment Bug 716765 - Disabling responseType for sync XHR (in window context) broke Mandreel based game apps
: Disabling responseType for sync XHR (in window context) broke Mandreel based ...
Status: NEW
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on:
Blocks: 701787
  Show dependency treegraph
 
Reported: 2012-01-09 18:47 PST by Masatoshi Kimura [:emk]
Modified: 2013-10-28 04:19 PDT (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Masatoshi Kimura [:emk] 2012-01-09 18:47:00 PST
Darin Fisher 2012-01-09 15:09:25 PST

This appears to break Mandreel WebGL-based apps:

I see the following error when loading chrome.monsterdashgame.com, which used to work in Firefox:

Error: uncaught exception: [Exception... "A parameter or an operation is not supported by the underlying object"  code: "15" nsresult: "0x8053000f (NS_ERROR_DOM_INVALID_ACCESS_ERR)"  location: "http://chrome.monsterdashgame.com/ZombieDash/mandreel.js Line: 1"]

Mandreel provides an emulation layer for binaries.  As such, it needs to emmulate fopen.  They appear to do so using synchronous XHR with responseType assigned to "ArrayBuffer".  They point the XHR at a local resource to provide decent performance it looks like.
Comment 1 Olli Pettay [:smaug] (reviewing overload) 2012-01-10 02:15:45 PST
I don't see any reason why Mandreel needs to use sync XHR.
Sure, sync XHR makes it perhaps easier to write the code, but it also makes user experience
worse.
Comment 2 Jarred Nicholls 2012-01-10 08:42:19 PST
https://bugs.webkit.org/show_bug.cgi?id=72154#c46
Comment #46 From Jarred Nicholls 2012-01-10 08:33:41 PST:

I don't know the internals of their compiler, but since they are in control of it, they could work around this by translating any sync call stack that invokes fopen() into an async pattern that doesn't continue execution until the async XHR request has completed.  I'm quite certain this is easier said than done, since their fopen() translation use to be self-contained but will now have to be context sensitive to where it is used...but it is definitely doable and would yield a better experience for fully web-based games that are loading large resources (e.g. textures) from remote sources.
Comment 3 Alon Zakai (:azakai) 2012-01-10 21:25:48 PST
(In reply to Olli Pettay [:smaug] from comment #1)
> I don't see any reason why Mandreel needs to use sync XHR.
> Sure, sync XHR makes it perhaps easier to write the code, but it also makes
> user experience
> worse.

Mandreel is compiling existing C and C++ codebases, that use fopen() and so forth in a sync manner. Emscripten does the same, and hits the same problem: It is very, very difficult or even impossible to automatically convert a large program using synchronous IO into one that is completely asynchronous.

I assume though that sync XHRs still work (otherwise a lot of the web would break), it's just that new features like ArrayBuffer don't in a sync XHR? Is this intentional or temporary?
Comment 4 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-01-10 23:42:27 PST
(In reply to Alon Zakai (:azakai) from comment #3)
> Is this intentional or temporary?

Intentional, but it seems the change needs to be undone. :-(
Comment 5 Olli Pettay [:smaug] (reviewing overload) 2012-01-11 01:22:01 PST
(In reply to Alon Zakai (:azakai) from comment #3)
> Mandreel is compiling existing C and C++ codebases, that use fopen() and so
> forth in a sync manner. Emscripten does the same, and hits the same problem:
> It is very, very difficult or even impossible to automatically convert a
> large program using synchronous IO into one that is completely asynchronous.
If they use sync XHR, I wouldn't recommend using them for any projects which want
good UX.

But I don't understand why it is hard to convert sync I/O to async if you control everything.
Just halt to program (i.e. don't handle any events) until xhr.onload fires.

> I assume though that sync XHRs still work (otherwise a lot of the web would
> break), it's just that new features like ArrayBuffer don't in a sync XHR? Is
> this intentional or temporary?
Very intentional. I'd like to remove sync XHR from Window context, and leave it to workers only.
But that may take time, or be even possible.
Comment 6 Masatoshi Kimura [:emk] 2012-01-11 06:17:07 PST
(In reply to Olli Pettay [:smaug] from comment #5)
> But I don't understand why it is hard to convert sync I/O to async if you
> control everything.
> Just halt to program (i.e. don't handle any events) until xhr.onload fires.

Could you rewrite test_XHR.html and test_xhr_progressevents.html without using JS 1.7 yield to prove rewriting is so easy?
Actual game apps will be much, much more complex. Moreover, converters such as Mandreel and Emscripten need to convert the app mechanically.
Comment 7 Alon Zakai (:azakai) 2012-01-11 09:58:05 PST
(In reply to Olli Pettay [:smaug] from comment #5)
> 
> But I don't understand why it is hard to convert sync I/O to async if you
> control everything.
> Just halt to program (i.e. don't handle any events) until xhr.onload fires.

Mandreel and Emscripten don't control the source code we compile. That code can have very large portions that are not written as event handling code, they might just load a file, process it, load another, process that, etc.

> 
> > I assume though that sync XHRs still work (otherwise a lot of the web would
> > break), it's just that new features like ArrayBuffer don't in a sync XHR? Is
> > this intentional or temporary?
> Very intentional. I'd like to remove sync XHR from Window context, and leave
> it to workers only.
> But that may take time, or be even possible.

Oh ok, so sync XHRs still work in workers, including with ArrayBuffers? In that case, I 100% agree with your position :)

We can compile existing C/C++ apps and run them in workers, that should be fine, they will receive input events and return pixel buffers and such. The only thing we would need to rewrite for that is the main event loop in the code we compile, but we need to do that even without workers.
Comment 8 Olli Pettay [:smaug] (reviewing overload) 2012-01-11 10:24:41 PST
Yes, the change applies only to mainthread sync XHR. If something blocks there, user can't
interact with the page. Very bad ux.
Comment 9 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-01-11 23:46:05 PST
Are the Mandreel folks OK with moving code to workers? Does crossing the worker boundary kill performance?

(I'm having doubts about the cost/benefit characteristics of banning sync XHR in the windows context when we don't ban sync XHR is the legacy responseType == '' case. That is, I'm inclined to think we should revert the ban and go through the trouble of making our task queues work right when sync XHR and alert() are involved.)
Comment 10 Olli Pettay [:smaug] (reviewing overload) 2012-01-12 03:22:37 PST
(In reply to Henri Sivonen (:hsivonen) from comment #9)
>  think we should revert the
> ban and go through the trouble of making our task queues work right when
> sync XHR and alert() are involved.)
our sync XHR or alert() handling have nothing to do with the "ban".

Blocking web page UI is bad, and web devs should have as few possibilities to do that as possible.
Comment 11 Jarred Nicholls 2012-01-12 07:17:26 PST
(In reply to Henri Sivonen (:hsivonen) from comment #9)
> Are the Mandreel folks OK with moving code to workers?

They need access to the WebGL and potentially also the Web Audio context.  With that, they'd prefer being in a worker.

> Does crossing the worker boundary kill performance?

I think all data would remain in the context of the worker if they have access to talk to the Web GL/Audio contexts.  Chrome's transferable objects is available though; would be nice to have that in all UAs.

> 
> (I'm having doubts about the cost/benefit characteristics of banning sync
> XHR in the windows context when we don't ban sync XHR is the legacy
> responseType == '' case. That is, I'm inclined to think we should revert the
> ban and go through the trouble of making our task queues work right when
> sync XHR and alert() are involved.)

Mandreel, Emscripten, and others like them are exceptions - generally speaking, no one should be writing *new* code that uses sync XHR in the window context.  This "ban" is a mere deterrent to using sync XHR by disabling a few of its newest features.  The foresight here is that, the more we steer away from sync XHR, eventually it might be possible to disable it in the window context completely.

This addition in the specs is a little strange, because it explicitly takes functionality away that was already shipped...however new that functionality happened to be.  But going forward, I would assume any new functionality that's added to XHR will require similar restrictions; and since it will be brand new functionality, there will be no grief for doing it.

If we are able to convert Mandreel and Emscripten to using workers or otherwise have them use the web platform in a "kosher" manner, and be successful, our initiative of a better web platform can chalk up a +1.
Comment 12 Alon Zakai (:azakai) 2012-01-12 10:09:12 PST
CC'ing the Mandreel guys (email from bug 678939). Miguel, are you aware of this issue? Is it problematic for Mandreel, and is moving to workers a reasonable solution (see discussion in the comments here)?
Comment 13 Jarred Nicholls 2012-01-12 10:16:54 PST
(In reply to Alon Zakai (:azakai) from comment #12)
> CC'ing the Mandreel guys (email from bug 678939). Miguel, are you aware of
> this issue? Is it problematic for Mandreel, and is moving to workers a
> reasonable solution (see discussion in the comments here)?

FYI there is a conversation happening on WebKit Bugzilla in parallel - Miguel has been a part of that conversation.

https://bugs.webkit.org/show_bug.cgi?id=72154
Comment 14 Gordon Williams 2012-03-20 08:02:04 PDT
Hi, I've just been hit with this on a WebGL app I develop (www.morphyre.com/scenedesign/go). I do a very similar thing to Mandreel and use an automated conversion from binary code to JavaScript, and 'just changing' to async code is definitely not easy.

I'm a single dev working on this, and this is an absolute disaster for me.

I think realistically what I will have to do as a workaround is to use a server-side script to create JSON, pass this back and then evaluate it with eval - which will result in SIGNIFICANTLY worse user experience than if this had just been left alone.
Comment 15 K. Gadd (:kael) 2013-10-28 00:43:19 PDT
FYI, JSIL has the same problem as Emscripten and Mandreel in this case. It's very difficult to mechanically transform code that relies on sync I/O to use async I/O without breaking things. (Possible, certainly, but there are more important issues I could address with the weeks to months it would probably take...)

Note You need to log in before you can comment on or make changes to this bug.