Bug 100664 – Web Inspector: adds isOwnProperty to remote protocol

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

[Andrey Lushnikov]

downstream: http://code.google.com/p/chromium/issues/detail?id=157676

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.16 (KHTML, like Gecko) Chrome/24.0.1305.3 Safari/537.16

Steps to reproduce the problem:
I'm writing an automated testing tool, and I'd like to fetch all the properties of an object, and then display the properties that come from the object's prototype with a different format.

This can be achieved by calling Runtime.getProperties [1] with ownProperties set to true, and then call it again with ownProperties set to false. However, that's more inefficient, and a bit harder to implement than a single call to Runtime.getProperties.

What is the expected behavior?
I would like to see a boolean ownProperty in PropertyDescriptor[2].

What went wrong?
I currently need two calls to Runtime.getProperties [1], and I need to merge the results in them. PropertyDescriptor [2] does not have an ownProperty attribute, which would make my life super-easy.

Did this work before? No 

Chrome version: 24.0.1305.3  Channel: dev
OS Version: OS X 10.8.2

[1] https://developers.google.com/chrome-developer-tools/docs/protocol/1.0/runtime#command-getProperties
[2] https://developers.google.com/chrome-developer-tools/docs/protocol/1.0/runtime#type-PropertyDescriptor

I'm guessing that other developers would like this change too. The extra code is not a big deal in my case, but I think changing the Webkit remote debugging protocol would make it more useful.

[Andrey Lushnikov]

Created attachment 171223 [details]
Patch

[Pavel Feldman]

Comment on attachment 171223 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [Dev Tools] Slight enhancement of remote protocol

drop these two lines

> Source/WebCore/ChangeLog:12
> +        * inspector/InjectedScriptSource.js:

Please explain what has changed and why.

> Source/WebCore/inspector/InjectedScriptSource.js:355
> +

please revert

> Source/WebCore/inspector/InjectedScriptSource.js:369
> +                            descriptors.push({ name: name, value: object[name], writable: false, configurable: false, enumerable: false, isOwnProperty: firstIteration});

isOwnProperty: o === object,

also, since this property is optional, you should only add it when it is true. I.e. create an object and optionally add isOwnProperty there.

> LayoutTests/ChangeLog:3
> +        [Dev Tools] Slight enhancement of remote protocol

drop these 2 lines

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty-expected.txt:3
> +property.name=bar isOwnProperty=true

==, surround with " "
property.name == bar

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:8
> +window.b = {bar: "cde"};

{ bar: "cde" };

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:14
> +    var fail = function(msg)

function fail(message)
{
    ....
}

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:18
> +        throw new Error("Error: " + msg);

drop this

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:25
> +        if (error) {

Do not use {} around single line blocks

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:26
> +            fail("Error on getting window.b property");

return fail() ...

... also, you don't need this branch. Let the test time out in case it fails.

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:28
> +            RuntimeAgent.getProperties(result.objectId, false, step2);

/* isOwnProperty */ false

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:32
> +    function step2(error, properties) {

{ on the next line

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:33
> +        if (error) {

ditto

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:36
> +        if (!properties) {

ditto

> LayoutTests/inspector/runtime/runtime-getProperties-isOwnProperty.html:39
> +        for (var i = 0; i < properties.length; i++) {

++i

[Andrey Lushnikov]

Created attachment 171247 [details]
Patch

[Build Bot]

Comment on attachment 171247 [details]
Patch

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

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

[Andrey Lushnikov]

Created attachment 171257 [details]
Patch

[Build Bot]

Comment on attachment 171257 [details]
Patch

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

New failing tests:
inspector/runtime/runtime-getProperties-isOwnProperty.html

[Andrey Lushnikov]

Created attachment 171417 [details]
Patch

[WebKit Review Bot]

Comment on attachment 171417 [details]
Patch

Clearing flags on attachment: 171417

Committed r132902: <http://trac.webkit.org/changeset/132902>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.