[Webkit-unassigned] [Bug 100664] Web Inspector: adds isOwnProperty to remote protocol

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=100664


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #171223|review?                     |review-
               Flag|                            |




--- Comment #2 from Pavel Feldman <pfeldman at chromium.org>  2012-10-29 06:42:04 PST ---
(From update of attachment 171223)
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

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.