Bug 156228 – Web Inspector: Include "thisValue" internal property when logging an arrow function

• Component: Web Inspector
• Status: NEW

[Joseph Pecoraro]

* SUMMARY
Include "thisValue" internal property when logging an arrow function.

* TEST
(function() {
    window.arrowWithThis = () => this;
    window.arrowWithoutThis = () => 123;
}).call([1,2,3]);

console.dir(window.arrowWithThis) // Expanding object in console shows: thisValue of [1,2,3]
console.dir(window.arrowWithoutThis) // Expanding object in console shows: thisValue of undefined

[Joseph Pecoraro]

Created attachment 275637 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Err, test example should have been:

(function() {
    window.arrowWithThis = () => this;
}).call([1,2,3]);
(function() {
    window.arrowWithoutThis = () => 123;
})();

Screenshot to come.

[Joseph Pecoraro]

Created attachment 275638 [details]
[IMAGE] "thisValue" internal property

[Radar WebKit Bug Importer]

<rdar://problem/25552300>

[Blaze Burg]

Comment on attachment 275637 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=275637&action=review

> LayoutTests/inspector/model/remote-object.html:54
> +        {expression: "() => 123"}, // arrow function

Tests LGTM.

> Source/JavaScriptCore/ChangeLog:10
> +        If the value is an arrow function, return a "thisValue" internal

Is it called 'thisValue' for bound functions too?

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:278
> +            return array;

I would like Tim to double check this code, I'm not up to date on JSC bindings.

[Blaze Burg]

Comment on attachment 275638 [details]
[IMAGE] "thisValue" internal property

It would be nice if we somehow visually distinguished special/builtin properties. Then it could say 'this' instead of 'thisValue' or it could be clear that 'thisValue' is not actually a property of the function object.

[Blaze Burg]

(In reply to comment #6)
> Comment on attachment 275638 [details]
> [IMAGE] "thisValue" internal property
> 
> It would be nice if we somehow visually distinguished special/builtin
> properties. Then it could say 'this' instead of 'thisValue' or it could be
> clear that 'thisValue' is not actually a property of the function object.

A tooltip that explains builtin properties would be fine I think.

[Saam Barati]

Comment on attachment 275637 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=275637&action=review

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:268
> +            if (scope->getPropertySlot(exec, exec->propertyNames().thisIdentifier, slot)) {

I don't think this is quite right. For example:
function foo() {
    let arr1 = () => {
        this; // "this" from foo
        function bar() {
            let arr2 = () => { 
                ...; // code that doesn't contain a use of "this"
                // when you resolve for "this" from arr2's scope
                // I believe you will get foo's "this" instead of bar's "this"
            }
        }
}


If this is indeed a problem, one suction is for arrow functions 
to always close over "this" when generating debugger op codes. 
(Same with closing over arguments/new.target, etc.)

[Joseph Pecoraro]

(In reply to comment #8)
> Comment on attachment 275637 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=275637&action=review
> 
> > Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:268
> > +            if (scope->getPropertySlot(exec, exec->propertyNames().thisIdentifier, slot)) {
> 
> I don't think this is quite right. For example:
> function foo() {
>     let arr1 = () => {
>         this; // "this" from foo
>         function bar() {
>             let arr2 = () => { 
>                 ...; // code that doesn't contain a use of "this"
>                 // when you resolve for "this" from arr2's scope
>                 // I believe you will get foo's "this" instead of bar's
> "this"
>             }
>         }
> }
> 
> 
> If this is indeed a problem, one suction is for arrow functions 
> to always close over "this" when generating debugger op codes. 
> (Same with closing over arguments/new.target, etc.)

Alternatively, because arr2 doesn't capture `this` we could store the "usesThis" somewhere, like on the Executable in m_features, and avoid showing a `thisValue` unless the arrow function uses it.

[Saam Barati]

(In reply to comment #9)
> (In reply to comment #8)
> > Comment on attachment 275637 [details]
> > [PATCH] Proposed Fix
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=275637&action=review
> > 
> > > Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:268
> > > +            if (scope->getPropertySlot(exec, exec->propertyNames().thisIdentifier, slot)) {
> > 
> > I don't think this is quite right. For example:
> > function foo() {
> >     let arr1 = () => {
> >         this; // "this" from foo
> >         function bar() {
> >             let arr2 = () => { 
> >                 ...; // code that doesn't contain a use of "this"
> >                 // when you resolve for "this" from arr2's scope
> >                 // I believe you will get foo's "this" instead of bar's
> > "this"
> >             }
> >         }
> > }
> > 
> > 
> > If this is indeed a problem, one suction is for arrow functions 
> > to always close over "this" when generating debugger op codes. 
> > (Same with closing over arguments/new.target, etc.)
> 
> Alternatively, because arr2 doesn't capture `this` we could store the
> "usesThis" somewhere, like on the Executable in m_features, and avoid
> showing a `thisValue` unless the arrow function uses it.
Yeah. I like this idea better.