[patch][cla] Existing setTimeout implementation overwritten by hostenv_rhino.js

[guest]

When running within Rhino, the hostenv_rhino.js file defines implementations of setTimeout and clearTimeout. However, the code doesn't check if those functions have already been defined. So those implementations will shadow prior definitions.

Other frameworks used in Rhino may define their own implementation of setTimeout, clearTimeout, setInterval, etc. It would be great if Dojo did not overwrite any existing implementation.

Examples of other implementations of setTimeout for Rhino are:

• ​OpenAnzo's asyncTestRunner
• ​John Resig's env.js

The main difference being whether they use Java threads or an event pump to behave more like the browsers which are single threaded.

• Jordi Albornoz Mulligan
  

[email protected]…

[guest]

Suggested patch - CLA submitted and acknowledged.

[guest]

Patch was attached by Jordi Albornoz Mulligan <[email protected]…>.

[dylan]

(In [14299]) fixes #6393, prevent rhino from overriding any existing [set|clear]Timeout implementations, thanks Jordi Albornoz Mulligan for the patch, \!strict

[Mark Wubben]

In my current setup, resolving the setTimeout and clearTimeout variables does not throw an exception. Running Rhino 1.7.

I can't quite figure out why this is the case, but it wouldn't hurt adding:

	if(typeof(setTimeout) == 'undefined' || typeof(clearTimeout) == 'undefined'){ throw new Error; }

at the end of the try{} block.

[Mark Wubben]

In fact, this could replace the existing check, since the variables are referenced anyway.

[dylan]

Mark,

You mean like so:

Index: hostenv_rhino.js
===================================================================
--- hostenv_rhino.js    (revision 15698)
+++ hostenv_rhino.js    (working copy)
@@ -223,6 +223,7 @@
                                }
                                try{
                                        func();
+                                       if(typeof(setTimeout) == 'undefined' || typeof(clearTimeout) == 'undefined'){ throw new Error; }
                                }catch(e){
                                        console.debug("Error running setTimeout thread:" + e);
                                }

[Mark Wubben]

Nope, meant the first block, to check for existing implementations:

try{
	setTimeout;
	clearTimeout;
}catch(e){

However, if I run this directly in a Rhino 1.7 shell (release 2, 11/05/2008) referencing setTimeout does throw an error. Don't quite recall what was happening earlier.

[dylan]

Ok, marking as fixed then for now, as I can't reproduce either.

[Mark Wubben]

We found a reproduction by running util/doh/runner.sh. Peller fixed the detection in r16312, see #8159.