Ted Campbell [:tcampbell] Assignee
	Description • 7 years ago

As a follow-up to Bug 1397049, there are also issues with computing JSOP_GLOBALTHIS, JSOP_FUNCTIONTHIS in debugger when non-syntactic environments are on the chain. These environments occur in browser code (JSMs, subscripts) but not content code.

	Ted Campbell [:tcampbell] Assignee
	Comment 2 • 7 years ago

Note to self, one test failed: https://treeherder.mozilla.org/logviewer.html#?job_id=145673246&repo=try&lineNumber=2665

This is due to Bug 1416070, which has since been fixed.

	Jason Orendorff [:jorendorff]
	Comment 3 • 7 years ago
mozreview-review

Comment on attachment 8929520 [details]
Bug 1418404 - Support non-syntactic |this| in debugger console

https://reviewboard.mozilla.org/r/200796/#review207020

We discussed this a lot on IRC and got a ton of help from shu, which is of course above and beyond the call of duty.

Clearing review for now with the following requests:

* There are definitely values of `env` for which it would be way wrong to call `GetExtensibleLexicalEnvironment(env)`. Whatever the rules are, I think it can and should enforce them with assertions.

* `GetExtensibleLexicalEnvironment` needs a comment, because the fact that it's even a sane question to ask, given how specific a category that is and that the spec contains no such thing, is extraordinary and unexplained.

::: js/src/vm/Interpreter.cpp:154
(Diff revision 1)
>  js::GetNonSyntacticGlobalThis(JSContext* cx, HandleObject envChain, MutableHandleValue res)
>  {
> -    RootedObject env(cx, envChain);
> -    while (true) {
> -        if (IsExtensibleLexicalEnvironment(env)) {
> -            res.set(GetThisValueOfLexical(env));
> +    // Find innermost extensible LexicalEnvironmentObject. This will hold the
> +    // current global |this| value for current script execution. Every global
> +    // has an associated LexicalEnvironmentObject so this is always defined.
> +    JSObject * lexicalEnv = &GetExtensibleLexicalEnvironment(envChain);

Nit: delete the space in `JSObject *`.

	Ursula Sarracini (:ursula)
	Comment 4 • 7 years ago

Steps to reproduce as asked for by Ted:

Open up the file ActivityStreamMessageChannel.jsm in the debugger panel.
Put a breakpoint on the line ```for (let port of this.channel.messagePorts) {```
Open a new tab
Try to see what ```this.channel``` is when breakpoint is hit

Expected Results:
It exists

Actual Results:
Says it's undefined but it's not actually undefined since I can see it exists in the Scopes sidebar

	Ted Campbell [:tcampbell] Assignee
	Comment 5 • 7 years ago

STR repeats for me. I see that this is actually covered by Bug 1397049, but I forgot I hadn't landed it yet. My local build version does fix this ActivityStreamMessageChannel case.