[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 #12 from Andrey Kosyakov <caseq at chromium.org>  2012-11-28 12:34:53 PST ---
(From update of attachment 176477)
View in context: https://bugs.webkit.org/attachment.cgi?id=176477&action=review

> Source/WebCore/CMakeLists.txt:1582
> +    inspector/InspectorLayerTreeAgent.cpp

please keep the entries sorted

> Source/WebCore/GNUmakefile.list.am:3699
> +	Source/WebCore/inspector/InspectorLayerTreeAgent.cpp \
> +	Source/WebCore/inspector/InspectorLayerTreeAgent.h \

ditto

> Source/WebCore/Target.pri:759
> +    inspector/InspectorLayerTreeAgent.cpp \

ditto

> Source/WebCore/Target.pri:1901
> +    inspector/InspectorLayerTreeAgent.h \

ditto

> Source/WebCore/WebCore.gypi:2852
> +            'inspector/InspectorLayerTreeAgent.cpp',
> +            'inspector/InspectorLayerTreeAgent.h',

ditto

>> Source/WebCore/inspector/Inspector.json:3266
>> +                    { "name": "isRoot", "type": "boolean", "description": "Indicates whether this layer is the root." },
> 
> You might want to make it optional and imply that it is not root when omitted.

Is it necessary at all? It appears that parentId could be used to check for root instead. Actually, it looks like with current implementation you don't need either isRoot or parent, as these may be easily deduced from the tree that you return.

>> Source/WebCore/inspector/InspectorLayerTreeAgent.cpp:53
>> +};
> 
> Is it typical to have a namespace for this strings?

yes, at least for the rest of inspector.

> Source/WebCore/inspector/InspectorLayerTreeAgent.cpp:137
> +PassRefPtr<TypeBuilder::LayerTree::Layer> InspectorLayerTreeAgent::buildObjectForLayer(RenderLayer* renderLayer, bool deep)

is deep ever false?

>> Source/WebCore/inspector/InspectorLayerTreeAgent.cpp:149
>> +        .setIsComposited(isComposited);
> 
> I know this is pretty common in inspector code, but I'm not a huge fan of chaining like this. It looks too much like JavaScript/JQuery code :)
> 
> But more importantly, it's a pain to debug because you have to step in and out lots of times (I should check if lldb is smart about line numbers there).

It's the only practical way to do it -- the intermediate types that each setFoo() returns are not expected to appear explicitly in the client code (it would be an ugly template with an integer parameter set to a bitmask of not-yet-set fields -- we use this to assure at compile time that all fields are set before we send the object)

> Source/WebCore/inspector/InstrumentingAgents.h:43
> +class InspectorLayerTreeAgent;

sorting problem

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