The new Google Maps patch, dissected

We released a new browser.js file yesterday. It's a pretty interesting release actually..

The Amazon patch is a pretty radical step – we have never launched any patch that embeds a third-party JS library on every page of a global web presence like Amazon. We know it solves some of our problems, and we know it doesn't solve all problems – we'll keep working with Amazon to have more of their server-side browser sniffing fixed. It would be nice if this patch were short-lived πŸ˜‰


But the most interesting patch – or at least the hardest one to come up with – is a workaround for a bug in the Carakan ECMAScript engine that a new Google Maps feature hit. The feature is being rolled out, so some users have seen this problem and some haven't yet. As you see from the screenshot, they are very excited about the new feature and announce it with a small popup. In Opera, that popup refuses to go away again. Using the close button just brings up the small "loading" indication at the top of the screenshot. Using Google Maps with this stubborn box staying on top of everything else is more than a little annoying..

My colleague KΓ₯re did the hard work of reading obfuscated JS to figure out where the problem was. I don't want to know how many hours it took him to boil it down to this compact demo:

var passed = 0;
(function (){
	var foo = 1;
	this.runtest = function (){
		eval('window.ev=function(a){eval(a)};');
		try{
			window.ev('(function(){passed=foo})()')
			}
		catch(e){}
		document.getElementsByTagName('p')[0].firstChild.data = passed?'PASS':'FAIL';
		};
	})();
runtest();

To understand this issue, including why finding a patch at first seemed impossible, we need to know about an esoteric quirk of the eval() function.

Normally, if you're naughty enough to call eval() (which it is better not to use, by the way..) you invoke the window.eval() built-in function. It will evaluate the code you pass to it in the local scope. So, for example doing this:

function(){
  var ok=true;
  eval('alert(ok?"sees local variable":"does not see local variable")');
}

the eval'ed code sees the local variable 'ok'.

However, eval by any other name doesn't smell as sweet – if the name you use to refer to the eval function is not 'eval' the method will change behaviour entirely, and run code in the global scope instead! Doing this:

var ok=false;
function(){
  var ok=true, ev=window.eval;
  ev('alert(ok?"sees local variable":"does not see local variable")');
}

we see that if the reference to the eval method is different, it no longer sees local variables!

This odd behaviour is a known eval quirk which is now standardised in ECMAScript 5.

Regarding the problem, Google Maps seem to have found a way to do indirect eval in a local scope. Sort of. I have no idea why they are doing it this way..

The window.ev('(function(){passed=foo})()') just references another method that will do the actual eval'ing. Doing an indirect eval here would run code in global scope. Hence it should not actually have accesss to the local 'foo' variable.

However, the earlier eval call is a direct eval – so it will run in the local scope:
eval('window.ev=function(a){eval(a)};');
meaning that the function expression here should have closure data that would include the local variable foo.

Hence, when a string of code that refers to 'foo' is passed to the direct eval() inside that ev function, this eval call will run in the local scope of that function expression, and the variable foo should be found in its closure data. And this is where Carakan stumbles: it fails somehow to get the closure data right for a function expression created in an eval() inside another function.

Now, why was this hard to patch? Normally when some method Opera defines has a bug, we can overwrite that method with a fixed one from browser.js. For example, if we find a problem in the DOM appendChild() method we'd just overwrite Node.prototype.appendChild with a working method. However, we can not overwrite window.eval() like that – because we would need to cache a reference to it, and any calls to that reference would be an "indirect" eval and run in the global scope.. This would violate most of Google Maps assumptions and make the site fail entirely.

Here's the patch I eventually came up with:

addPreprocessHandler(  /function(a){eval(a)}/g, '(function(){return function(a){eval(a)}})());' );

This does a search/replace on Google Maps' source code while we load it, replacing any instances of

function(a){eval(a)}

with

(function(){return function(a){eval(a)}})());

which adds an extra function expression – one more chance for Carakan to record closure data. This double-take does, by a lucky stroke, save the required context.

So, here's hoping most users get the new browser.js before the Google Maps update. I like the speeeed of patching problems before users even get them πŸ™‚

Advertisements

14 thoughts on “The new Google Maps patch, dissected

  1. Menno writes:Any hypotheses why Google Maps goes about calling window.eval in such a roundabout way? I'm guessing "window.ev(a)" is subsequently used throughout the rest of the code, so it should either be:- an extreme javascript minifying measure, or- making things work right in another browser

  2. Got a javascript error just now on <draft.blogger.com>ERROR: Possible problem with your *.gwt.xml module file.The compile time user.agent value (opera) does not match the runtime user.agent value (unknown). Expect more errors.Its the new blogger dashboard (google ofcourse)

  3. I am grateful that your team put so much effort into ensuring that users' browsing experience is efficient, and even somewhat pleasurable. Thank you. Thank you to you all.

  4. Anonymous writes:any news on why google maps die after some time of dragging them with middle click? this is still in and this is still highly annoying :/

  5. Originally posted by anonymous:

    Any hypotheses why Google Maps goes about calling window.eval in such a roundabout way?

    The bigger question – to me at least – is why they are even using window.eval in the first place. Once they have decided to use eval in the first place I guess the above is a clever-ish trick to make code in part A of their architecture refer to local variables in part B? It would be really interesting to ask someone on the Maps team why they are using eval() to the extent they do.Originally posted by geeneeyes:

    Got a javascript error just now on <draft.blogger.com>

    Yes, we do a bit of trickery for blogger.com to make sure Opera gets access to the rich text editor. You don't notice any problems, do you? I think this error is just the code being confused enough to emit a warning but not confused enough to actually break in any way. That's what I hope anyway ;-)Originally posted by anonymous:

    any news on why google maps die after some time

    Yes, I believe that a fix to handle this was added to Opera's core code during this week. 😎 It's a significant rewrite so we'll bake it carefully and test it extensively. I haven't tested it on Maps myself yet, but I will.

  6. What is strange for me is that I can't create a working custom search from the google maps search boxWhen I try to do a search using the custom search it gives me a box and thats it

  7. I guess there is so much JavaScript involved when submitting the Google Maps search that creating a search from it won't work. Perhaps somebody can give us a hint about how to do this?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s