Specifically, this bit: 6163 if (evalWithBindings) { 6164 /* TODO - This should probably be a Call object, like ES5 strict eval. */ 6165 RootedPlainObject nenv(cx, NewObjectWithGivenProto<PlainObject>(cx, NullPtr(), env)); This object then ends up on function scope chains, etc. You could probably use a With object here, the way the DOM does for the scope chains it tries to build up... or yes, a Call object. This is one of the few remaining things that are depending on object parents, as far as I can see.

Comment on attachment 8576386 [details] [diff] [review] part 4. Add some assertions about what enclosingScope can return for non-scope objects Review of attachment 8576386 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/ScopeObject.cpp @@ +2725,5 @@ > +JSObject * > +JSObject::enclosingScopeSlow() > +{ > + MOZ_ASSERT_IF(is<JSFunction>(), as<JSFunction>().isInterpreted()); > + MOZ_ASSERT(getParent() == &global()); assertParentIs maybe? Not that it matters. ::: js/src/vm/ScopeObject.h @@ +1042,5 @@ > > + if (is<js::GlobalObject>()) > + return nullptr; > + > + return enclosingScopeSlow(); Rather than this, I think you can just move this method definition into ScopeObject-inl.h, then add #includes of that file where needed.

Comment on attachment 8576380 [details] [diff] [review] part 1. Make it possible to CreateScopeObjectsForScopeChain with a given non-global scope chain terminator Review of attachment 8576380 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsapi.cpp @@ +3195,5 @@ > } > > static bool > CreateScopeObjectsForScopeChain(JSContext *cx, AutoObjectVector &scopeChain, > + HandleObject enclosingScope, nit: dynamicTerminatingScope? I'll leave the name up to you, but I'd like "dynamic" in there to avoid confusion.

Comment on attachment 8576383 [details] [diff] [review] part 3. Use CreateScopeObjectsForScopeChain in DebuggerFrame_evalWithBindings Review of attachment 8576383 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. r=me with comments addressed. ::: js/src/vm/Debugger.cpp @@ +763,5 @@ > * from GetDebugScopeFor(Frame|Function). > */ > + MOZ_ASSERT(!env->is<ScopeObject>() || > + (env->is<DynamicWithObject>() && > + !env->as<DynamicWithObject>().isSyntactic())); This condition is used in various places now (here and below in this patch, in AssertDynamicScopeMatchesStaticScope, and in ExecuteKernel off the top of my head). Do you mind refactoring it to something like IsValidTerminatingScope? @@ +6024,5 @@ > * ScopeIter should stop at any non-ScopeObject boundaries, and we are > * putting a DebugScopeProxy on the scope chain. > + * > + * XXXbz But maybe in the case when env is a DynamicWithObject we should in > + * fact pass the corresponding StaticWithObject for the static scope? No, we shouldn't make a StaticWithObject. With your changes the notion of ScopeObject chains terminated by non-ScopeObject really means ScopeObject chains terminated by non-ScopeObject or non-syntactic Withs. Could you update the comment accordingly?

> nit: dynamicTerminatingScope? Done. > Do you mind refactoring it to something like IsValidTerminatingScope? No, was considering that myself. Done. > Could you update the comment accordingly? Done. > assertParentIs maybe? Yes, done. > I think you can just move this method definition into ScopeObject-inl.h Done. https://hg.mozilla.org/integration/mozilla-inbound/rev/5b09f6900fe8 https://hg.mozilla.org/integration/mozilla-inbound/rev/455577de172e https://hg.mozilla.org/integration/mozilla-inbound/rev/16723f5b0307 https://hg.mozilla.org/integration/mozilla-inbound/rev/37d8d0362318

https://hg.mozilla.org/mozilla-central/rev/5b09f6900fe8 https://hg.mozilla.org/mozilla-central/rev/455577de172e https://hg.mozilla.org/mozilla-central/rev/16723f5b0307 https://hg.mozilla.org/mozilla-central/rev/37d8d0362318