567301 - Require that scope chain only contains Call/Block/With/DeclEnvClass or global object

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

It seems the scope chain can contain non-global DOM objects (event handlers). That's ugly. It would be a nice invariant to have that the scope chain is purely native and only of Call/Block/With/DeclEnvClass until we hit the global object. The handler should be wrapped with a with object.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Blocking Jaeger on this because BINDNAME wants to track its value as an "object" type, and not have to peek at js_FunctionClass. Fat values requires this distinction, like tracing.

Comment 2

[Andreas Gal :gal]

The parent of some functions is Function, which is js_FunctionClass. I am trying to make a shell test case that exposes that on the scope chain.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So how would the with-wrapping affect event handler performance?

Comment 4

[Andreas Gal :gal]

I think we should carefully scope this bug if JM blocks on it and strive for a minimal change. Long term I would like to sanitize the scope chain. Short term we might aim lower:
- all objects on the scope chain must be native (almost there)
- all objects on the scope chain must be objects (not functions)

If we end up doing the with trick, I don't expect a performance impact. The property caches the lookup using X/Y coordinates, so its just one proto over off the with object. Not a big deal.

Comment 5

[Boris Zbarsky [:bzbarsky]]

I was more worried about tracing inside event handlers, but maybe that's not an issue.

Comment 6

[Andreas Gal :gal]

Yeah, I am treading carefully here. Still trying to understand what exactly we are doing right now.

Comment 7

[Brendan Eich [:brendan]]

(In reply to comment #5)
> I was more worried about tracing inside event handlers, but maybe that's not an
> issue.

We don't trace in any code that has non-native objects on the scope chain. See ScopeChainCheck in jstracer.cpp.

FWIW, v8 in chromium/chrome wraps event receiver and scope-ancestors in with somehow, at least from the decompilation results.

/be

Comment 8

[Igor Bukanov]

(In reply to comment #5)
> I was more worried about tracing inside event handlers, but maybe that's not an
> issue.

This is only relevant for inline event handlers defined using onsomething="code". Only those code fragments would have DOM objects on the scope chain. I can imagine that those handlers could be bottleneck in some synthetic benchmarks, but in practice they are mostly one-liners calling some functions.

(In reply to comment #7)
> We don't trace in any code that has non-native objects on the scope chain. See
> ScopeChainCheck in jstracer.cpp.

That function uses js_IsCacheableNonGlobalScope. That means that only a scope chain with Block, Call and DeclEnv instances as non-globals is traced.