LayerDebugInfo::AppendAsTraceFormat uses JsonWriter::Write directly on |out| which clears it. |
|||
Issue descriptionLayerDebugInfo::AppendAsTraceFormat uses JsonWrite::Write directly on |out| which clears it. All other AppendAsTraceFormat() impls correctly use a temp variable to interact with JsonWriter::Write (I admit that this is a bit silly but that's how it is right now). https://cs.chromium.org/chromium/src/ui/compositor/layer.cc?dr=CSs&q=LayerDebugInfo::AppendAsTraceFormat&sq=package:chromium&l=825 I made the same mistake in https://codereview.chromium.org/2374193002/ by copying LayerDebugInfo's approach. It borks the JSON and tracing chokes. I tried to trace with cc.debug + cc.debug.quads + devtools.timeline.layers enabled but I didn't manage to trigger this bug (nor get a "layer_name" arg appended) so this code doesn't seem to run right now..? I'll let owners (@piman from ui\compositor\OWNERS) figure out whether to fix it or remove it (if it was never noticed that chrome://tracing chokes when generating such a trace then it likely never ran..!).
,
Oct 10 2016
-> vollick who I believe wrote the code
,
Nov 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b44373a0917b40509990fa33bfde2ca24bdd15e3 commit b44373a0917b40509990fa33bfde2ca24bdd15e3 Author: vollick <vollick@chromium.org> Date: Wed Nov 02 16:19:14 2016 LayerDebugInfo should not clobber trace output The code previously used the JSON writer incorrectly (needed to use a temporary). BUG= 651455 Review-Url: https://codereview.chromium.org/2467953003 Cr-Commit-Position: refs/heads/master@{#429295} [modify] https://crrev.com/b44373a0917b40509990fa33bfde2ca24bdd15e3/ui/compositor/layer.cc [modify] https://crrev.com/b44373a0917b40509990fa33bfde2ca24bdd15e3/ui/compositor/layer_unittest.cc
,
Nov 2 2016
Thanks for noticing this! (The code is indeed not dead, but you have to build for cros in debug and turn on the usually-disabled cc.debug category). |
|||
►
Sign in to add a comment |
|||
Comment 1 by gab@chromium.org
, Sep 29 2016