Bug 100664 - Web Inspector: adds isOwnProperty to remote protocol
Summary: Web Inspector: adds isOwnProperty to remote protocol
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-29 06:12 PDT by Andrey Lushnikov
Modified: 2012-10-30 06:13 PDT (History)
9 users (show)

See Also:


Attachments
Patch (7.24 KB, patch)
2012-10-29 06:24 PDT, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (6.79 KB, patch)
2012-10-29 07:35 PDT, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (7.55 KB, patch)
2012-10-29 09:10 PDT, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (7.58 KB, patch)
2012-10-30 04:26 PDT, Andrey Lushnikov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Lushnikov 2012-10-29 06:12:47 PDT
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.
Comment 1 Andrey Lushnikov 2012-10-29 06:24:16 PDT
Created attachment 171223 [details]
Patch
Comment 2 Pavel Feldman 2012-10-29 06:42:04 PDT
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
Comment 3 Andrey Lushnikov 2012-10-29 07:35:26 PDT
Created attachment 171247 [details]
Patch
Comment 4 Build Bot 2012-10-29 08:10:38 PDT
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
Comment 5 Andrey Lushnikov 2012-10-29 09:10:28 PDT
Created attachment 171257 [details]
Patch
Comment 6 Build Bot 2012-10-29 11:25:20 PDT
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
Comment 7 Andrey Lushnikov 2012-10-30 04:26:53 PDT
Created attachment 171417 [details]
Patch
Comment 8 WebKit Review Bot 2012-10-30 06:13:39 PDT
Comment on attachment 171417 [details]
Patch

Clearing flags on attachment: 171417

Committed r132902: <http://trac.webkit.org/changeset/132902>
Comment 9 WebKit Review Bot 2012-10-30 06:13:43 PDT
All reviewed patches have been landed.  Closing bug.