Bug 94397 – Web Inspector: expose object internal properties such as PrimitiveValue or BoundThis

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Peter Rybin]

Some JavaScript objects holds inner state that cannot be seen as a regular properties. However the state is well defined in spec as internal properties. Make them visible to user, since they might be important for user data exploring.

[Peter Rybin]

Created attachment 159242 [details]
Patch

[Peter Rybin]

Created attachment 159243 [details]
Patch

[WebKit Review Bot]

Comment on attachment 159243 [details]
Patch

Attachment 159243 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13520603

[Peter Rybin]

Created attachment 159248 [details]
Patch

[Build Bot]

Comment on attachment 159248 [details]
Patch

Attachment 159248 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13517960

[Timothy Hatcher]

Comment on attachment 159248 [details]
Patch

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

> Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:194
> +    return jsUndefined();

Please file a JSC bug about adding this and put in a FIXME.

[Peter Rybin]

Created attachment 165154 [details]
Patch

[Build Bot]

Comment on attachment 165154 [details]
Patch

Attachment 165154 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13951777

New failing tests:
inspector/console/command-line-api.html

[WebKit Review Bot]

Comment on attachment 165154 [details]
Patch

Attachment 165154 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13980006

New failing tests:
WebFilterOperationsTest.saveAndRestore
inspector/console/command-line-api.html

[Yury Semikhatsky]

Comment on attachment 165154 [details]
Patch

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

> Source/WebCore/inspector/Inspector.json:498
> +                    { "name": "internal", "type": "boolean", "optional": true, "description": "True if property is only visible in debugger and its name is conventional." }

r- for this part as we agreed off the bug to return internal properties in a separate array instead.

[Peter Rybin]

Created attachment 165655 [details]
Patch

[WebKit Review Bot]

Attachment 165655 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1
Source/WebCore/inspector/InjectedScript.cpp:145:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:82:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Peter Rybin]

(In reply to comment #11)
> Created an attachment (id=165655) [details]
> Patch

Since this patch this bug assumes that frontend needs explicit support for this feature (with previous version it got internal properties among others).

[Build Bot]

Comment on attachment 165655 [details]
Patch

Attachment 165655 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14019240

[Build Bot]

Comment on attachment 165655 [details]
Patch

Attachment 165655 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14032197

New failing tests:
inspector-protocol/runtime-getProperties.html
http/tests/security/sandboxed-iframe-origin-add.html

[Yury Semikhatsky]

Comment on attachment 165655 [details]
Patch

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

> LayoutTests/inspector-protocol/runtime-getProperties-expected.txt:1
> +Properties of Object(5)

You will also need to provide JSC-specific test expectations.

> LayoutTests/inspector-protocol/runtime-getProperties.html:50
> +                processStep(next);

Consider moving this into the try block.

> LayoutTests/inspector-protocol/runtime-getProperties.html:90
> +        if (id === undefined)

if (!id) would work

[Peter Rybin]

Comment on attachment 165655 [details]
Patch

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

>> LayoutTests/inspector-protocol/runtime-getProperties.html:50
>> +                processStep(next);
> 
> Consider moving this into the try block.

processStep is a one big try itself.

>> LayoutTests/inspector-protocol/runtime-getProperties.html:90
>> +        if (id === undefined)
> 
> if (!id) would work

Technically yes, but I'd like not think about whether id can legitimately 0 (no) or empty string (no).

[Peter Rybin]

Created attachment 166959 [details]
Patch

[WebKit Review Bot]

Attachment 166959 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1
Source/WebCore/inspector/InjectedScript.cpp:145:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:82:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 166959 [details]
Patch

Attachment 166959 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14131786

New failing tests:
inspector/console/command-line-api.html

[Peter Rybin]

Created attachment 166970 [details]
Patch

[WebKit Review Bot]

Attachment 166970 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1
Source/WebCore/inspector/InjectedScript.cpp:145:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:82:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Peter Rybin]

(In reply to comment #16)
> (From update of attachment 165655 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165655&action=review
> 
> > LayoutTests/inspector-protocol/runtime-getProperties-expected.txt:1
> > +Properties of Object(5)
> 
> You will also need to provide JSC-specific test expectations.

Done

[WebKit Review Bot]

Comment on attachment 166970 [details]
Patch

Clearing flags on attachment: 166970

Committed r130398: <http://trac.webkit.org/changeset/130398>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.