[Webkit-unassigned] [Bug 103513] Provide the backend for exposing the layer tree to the Web Inspector

[bugzilla-daemon at webkit.org]

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





--- Comment #44 from Andrey Kosyakov <caseq at chromium.org>  2012-12-03 10:58:48 PST ---
(From update of attachment 177147)
View in context: https://bugs.webkit.org/attachment.cgi?id=177147&action=review

> LayoutTests/inspector-protocol/layer-tree-expected.txt:322
> +          "memory": 4582080,

Do you expect memory to remain stable?

> LayoutTests/inspector-protocol/layer-tree-expected.txt:332
> +          "layerId": "0.11",

I wouldn't expect ids to remain stable.

> LayoutTests/inspector-protocol/layer-tree.html:8
> +    var element = document.createElement('div');

please use double quotes where possible.

> LayoutTests/inspector-protocol/layer-tree.html:16
> +    // Variables used throughout the test.

Here and below, please remove redundant/self-evident comments -- there's a convention in WebKit to avoid such.

> LayoutTests/inspector-protocol/layer-tree.html:94
> +        //

please remove

> LayoutTests/inspector-protocol/layer-tree.html:120
> +        if (!attributes.hasOwnProperty("id") || attributes.id != "last-element")

style: we use !==
hasOwnProperty looks redundant

> LayoutTests/inspector-protocol/layer-tree.html:144
> +        //

please remove

> LayoutTests/inspector-protocol/layer-tree.html:147
> +                return (oldKeys.indexOf(key) == -1);

style: we normally use ===

> LayoutTests/inspector-protocol/layer-tree.html:150
> +                return (newKeys.indexOf(key) == -1);

ditto

> LayoutTests/inspector-protocol/layer-tree.html:163
> +        })(layerTree);

please extract call into a separate statement

> LayoutTests/inspector-protocol/layer-tree.html:180
> +    function dumpObject(obj)
> +    {
> +        InspectorTest.log("\n" + JSON.stringify(obj, null, '  '));
> +    };

I think you can move this over to InspectorTest. Perhaps, we would eventually need to unify/merge it with the front-end test suite.

> LayoutTests/inspector-protocol/layer-tree.html:186
> +            if (checkFailure(test.name, messageObject))

I think it would be better readable if the logic of checkFailure() were inlined here.

> LayoutTests/inspector-protocol/layer-tree.html:222
> +  div {
> +    position: absolute;
> +    top: 0;
> +    left: 0;
> +  }

Here and below -- please fix indents to be a multiple of 4.

> LayoutTests/platform/chromium/TestExpectations:4107
> +# Layer tree inspection test only passes on Mac
> +webkit.org/b/103513 inspector-protocol/layer-tree.html [ Skip ]

Please file a bug for other platforms to support this and refer to this bug instead. BTW, could you please elaborate on why this is Mac-only?

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