DevTools: expose layer compositing reasons and paint count via layer tree agent

DevTools: expose layer compositing reasons and paint count via layer tree agent

This part only includes back-end and model changes.

- keep compositing reasons in GraphicsLayer;
- fix paint count calculation in GraphicsLayer (was always 0);
- drop {setShow,isShowing}RepaintCounter, they're unused;
- expose GraphicsLayer::replicatedLayer(), pretend it's a parent for replica layers.

BUG=
R=pfeldman@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156701

[shawnsingh, 2013-08-21 20:08:18]

It should be mentioned that another approach may be to get the
CompositingReasons from the RenderLayer instead.

Depending on how you plan to present/visualize this information in the
inspector, that option might be better.  I'm gradually trying to evolve the
blink compositing code so that multiple RenderLayer subtrees may contribute to
one composited GraphicsLayer.  So if you are planning to add these annotations
to the existing element tree in inspector, it may be better to use the
RenderLayer version of compositingReasons instead.  Or, are we planning to give
developers a view into our composited layer tree in the inspector?

Also, CompositingReasons will likely change and evolve a lot over time.  There
is already a disappointing number of places where compositingReasons enum /
logic is duplicated.  Is there a way we can make it so that the inspector
automatically knows the most updated list of compositing reasons?   Perhaps we
can keep a statically defined table of reasons + UI description strings in
core/rendering/CompositingReasons.h which the inspector parses at runtime?

I'm happy to discuss more offline, as I don't really know the plans and
constraints for the inspector =)

[caseq, 2013-08-22 08:56:45]

> It should be mentioned that another approach may be to get the
> CompositingReasons from the RenderLayer instead.

From what I can see, these are almost identical -- ones in RenderLayer get
propagated as is to GraphicsLayer by RenderLayerBacking, plus there's a bunch of
reasons that RenderLayerBacking introduces for layer it creates.

> Depending on how you plan to present/visualize this information in the
> inspector, that option might be better.  I'm gradually trying to evolve the
> blink compositing code so that multiple RenderLayer subtrees may contribute to
> one composited GraphicsLayer.  So if you are planning to add these annotations
> to the existing element tree in inspector, it may be better to use the
> RenderLayer version of compositingReasons instead.  Or, are we planning to
give
> developers a view into our composited layer tree in the inspector?

The main reason we expose layers is for performance analysis, i.e. to be able to
refer layers from timeline's layer-related events like paint/rasterize and
otherwise expose performance metrics from compositor. So we ultimately want
references to CC layers, which is the reason why I switched
InspectorLayerTreeAgent from exposing properties of RenderLayers and using
synthetic ids to exposing GraphicsLayers and using WebLayer id's. We still want
traceability to DOM nodes, so currently we walk render layer hierarchy, keep the
ids of owner nodes from there and then grab GraphicsLayer via RenderLayerBacking
and associate them with owning nodes.

So I guess the short answer to your question is that we wont both -- actual
layers used by compositor and references to DOM nodes.

> Also, CompositingReasons will likely change and evolve a lot over time.  There
> is already a disappointing number of places where compositingReasons enum /
> logic is duplicated.  Is there a way we can make it so that the inspector
> automatically knows the most updated list of compositing reasons?   Perhaps we
> can keep a statically defined table of reasons + UI description strings in
> core/rendering/CompositingReasons.h which the inspector parses at runtime?

I would like something like this, but last time we discussed this with Pavel he
seemed to favor strong typing in protocol.json. I don't think compositing
reasons would be used for anything beyond debugging and I can imagine them being
volatile, so I guess having something like compositingReasonsToStringVector()
somewhere in core/rendering would be way more convenient than plumbing a large
strongly typed object around. Pavel, wdyt?

[pfeldman, 2013-08-22 16:49:53]

https://codereview.chromium.org/23050018/diff/1/LayoutTests/inspector/layer-c...
File LayoutTests/inspector/layer-compositing-reasons.html (right):

https://codereview.chromium.org/23050018/diff/1/LayoutTests/inspector/layer-c...
LayoutTests/inspector/layer-compositing-reasons.html:4: @-webkit-keyframes
rotate {
This should be a protocol test.

https://codereview.chromium.org/23050018/diff/1/Source/core/platform/graphics...
File Source/core/platform/graphics/GraphicsLayer.h (left):

https://codereview.chromium.org/23050018/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/GraphicsLayer.h:377: void
setShowRepaintCounter(bool show) { m_showRepaintCounter = show; }
Why is this going away?

https://codereview.chromium.org/23050018/diff/1/Source/devtools/protocol.json
File Source/devtools/protocol.json (right):

https://codereview.chromium.org/23050018/diff/1/Source/devtools/protocol.json...
Source/devtools/protocol.json:3818: { "name": "transform3D", "type": "boolean",
"optional": true, "description": "Composition due to association with an element
with a CSS 3D transform." },
No! Please make this a dictionary!

https://codereview.chromium.org/23050018/diff/1/Source/devtools/protocol.json...
Source/devtools/protocol.json:3874: "name": "reasonsForCompositingLayer",
compositingReasons

[trchen, 2013-08-22 23:02:35]

https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
File Source/core/inspector/InspectorLayerTreeAgent.cpp (right):

https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorLayerTreeAgent.cpp:70: // root of graphics layer
hierarchy, as platform adds containing layers (e.g. for overflosw scroll).
I knew you didn't add this, but this comment is full of typos. :(

s/renger layer/render layer/
s/overflosw scroll/frame scrolling/

https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorLayerTreeAgent.cpp:161: *errorString = "Not in
compositing mode";
errorString is already set.

https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorLayerTreeAgent.cpp:184: RenderLayerCompositor*
compositor = renderLayerCompositor(errorString);
Unused. Can we remove this variable?

https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorLayerTreeAgent.cpp:252:
Vector<std::pair<GraphicsLayer*, size_t> > stack;
Any reason why not use recursion here?

[caseq, 2013-08-23 08:51:25]

On 2013/08/22 16:49:53, pfeldman wrote:
>
https://codereview.chromium.org/23050018/diff/1/LayoutTests/inspector/layer-c...
> File LayoutTests/inspector/layer-compositing-reasons.html (right):
> 
>
https://codereview.chromium.org/23050018/diff/1/LayoutTests/inspector/layer-c...
> LayoutTests/inspector/layer-compositing-reasons.html:4: @-webkit-keyframes
> rotate {
> This should be a protocol test.

As discussed offline -- while it would really be more appropriate as a protocol
test, it currently relies heavily on DOMAgent (mostly to use hard-coded element
ids instead of variable layer ids). Re-implementing it as a protocol test would
require quite some logic to replace the DOMAgent.

>
https://codereview.chromium.org/23050018/diff/1/Source/core/platform/graphics...
> File Source/core/platform/graphics/GraphicsLayer.h (left):
> 
>
https://codereview.chromium.org/23050018/diff/1/Source/core/platform/graphics...
> Source/core/platform/graphics/GraphicsLayer.h:377: void
> setShowRepaintCounter(bool show) { m_showRepaintCounter = show; }
> Why is this going away?

Because we can! (this seems to be unused)

> https://codereview.chromium.org/23050018/diff/1/Source/devtools/protocol.json
> File Source/devtools/protocol.json (right):
> 
>
https://codereview.chromium.org/23050018/diff/1/Source/devtools/protocol.json...
> Source/devtools/protocol.json:3818: { "name": "transform3D", "type":
"boolean",
> "optional": true, "description": "Composition due to association with an
element
> with a CSS 3D transform." },
> No! Please make this a dictionary!

I made it a list of strings.

>
https://codereview.chromium.org/23050018/diff/1/Source/devtools/protocol.json...
> Source/devtools/protocol.json:3874: "name": "reasonsForCompositingLayer",
> compositingReasons

Done (this was actually the original name from previous LayerTreeAgent
implementation).

[caseq, 2013-08-23 08:52:52]

On 2013/08/22 23:02:35, trchen wrote:
>
https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
> File Source/core/inspector/InspectorLayerTreeAgent.cpp (right):
> 
>
https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
> Source/core/inspector/InspectorLayerTreeAgent.cpp:70: // root of graphics
layer
> hierarchy, as platform adds containing layers (e.g. for overflosw scroll).
> I knew you didn't add this, but this comment is full of typos. :(

Actually it's mine :) Thanks, fixed!

>
https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
> Source/core/inspector/InspectorLayerTreeAgent.cpp:161: *errorString = "Not in
> compositing mode";
> errorString is already set.

Fixed.

>
https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
> Source/core/inspector/InspectorLayerTreeAgent.cpp:184: RenderLayerCompositor*
> compositor = renderLayerCompositor(errorString);
> Unused. Can we remove this variable?

Fixed.

>
https://codereview.chromium.org/23050018/diff/1/Source/core/inspector/Inspect...
> Source/core/inspector/InspectorLayerTreeAgent.cpp:252:
> Vector<std::pair<GraphicsLayer*, size_t> > stack;
> Any reason why not use recursion here?

Re-implemented using recursion.

[pfeldman, 2013-08-23 11:08:25]

lgtm

[caseq, 2013-08-26 14:46:57]

Committed patchset #4 manually as r156701 (presubmit successful).