Web Inspector: console.log fires getters for deep properties

• Component: Web Inspector
• Status: RESOLVED FIXED

[Brandon Dail]

With the following code the "ref" getter will be called. This is inconsistent with Firefox and Chrome, which do not fire the getter.

```
let inner = {};

Object.defineProperty(inner, "ref", {
  get: () => console.log("getter fired"),
});

let outer = {
  inner
};

console.log(outer);

```


This came up in the React repo: https://github.com/facebook/react/issues/13179

[Radar WebKit Bug Importer]

<rdar://problem/42873158>

[Joseph Pecoraro]

Interesting that this happens when logging `outer` but not `inner`. Seems then it might be an accident in the property preview path.

[Joseph Pecoraro]

Switching to `console.log` to `console.trace` gives us:

[Log] Trace: getter fired Console Evaluation (Console Evaluation 1:6)
    _isPreviewableObjectInternal
    _appendPropertyPreviews
    _generatePreview
    RemoteObject
    create
    log
    Console Evaluation (Console Evaluation 1:13)
    evaluateWithScopeExtension
    _evaluateOn
    _evaluateAndWrap

It looks like at the bottom of _isPreviewableObjectInternal we access properties `object[propertyName]` without regard to whether or not it may be a getter.

    _isPreviewableObjectInternal(object, knownObjects, depth)
    {
        ...
        // Objects are simple if they have 3 or less simple properties.
        let ownPropertyNames = Object.getOwnPropertyNames(object);
        if (ownPropertyNames.length > 3)
            return false;
        for (let i = 0; i < ownPropertyNames.length; ++i) {
            let propertyName = ownPropertyNames[i];
            if (!this._isPreviewableObjectInternal(object[propertyName], knownObjects, depth))
                return false;
        }

        return true;
    }

[Joseph Pecoraro]

Created attachment 346936 [details]
[PATCH] Proposed Fix

[Saam Barati]

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

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

> Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295
> +            let descriptor = Object.getOwnPropertyDescriptor(object, propertyName);

Might be worth verifying that we're ok w/ this doing effects? Like I believe this calls a trap in a proxy.

[Joseph Pecoraro]

(In reply to Saam Barati from comment #5)
> Comment on attachment 346936 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=346936&action=review
> 
> > Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295
> > +            let descriptor = Object.getOwnPropertyDescriptor(object, propertyName);
> 
> Might be worth verifying that we're ok w/ this doing effects? Like I believe
> this calls a trap in a proxy.

Is there a way to tell if its a getter without an observable effect (without being a built-in)?

[Saam Barati]

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

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

r=me

>>> Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295
>>> +            let descriptor = Object.getOwnPropertyDescriptor(object, propertyName);
>> 
>> Might be worth verifying that we're ok w/ this doing effects? Like I believe this calls a trap in a proxy.
> 
> Is there a way to tell if its a getter without an observable effect (without being a built-in)?

Nope, not in a general way.

[WebKit Commit Bot]

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

Clearing flags on attachment: 346936

Committed r234780: <https://trac.webkit.org/changeset/234780>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.