[tracing] Adding Skia memory dump provider with cache totals

[tracing] Adding Skia memory dump provider with cache totals

This CL adds a dump provider to dump skia memory statistics to
chrome://tracing. The dump provider dumps only the totals of
SkGlyphCache and SkResourceCache.

BUG=503168
Committed: https://crrev.com/59c9691634783108308bd54f3f1ea39f795235cb
Cr-Commit-Position: refs/heads/master@{#340674}

[ssid, 2015-07-27 15:35:59]

ssid@chromium.org changed reviewers:
+ petrcermak@chromium.org

[ssid, 2015-07-27 15:36:00]

PTAL

[petrcermak, 2015-07-27 15:45:39]

LGTM with two comments.

Thanks,
Petr

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
File skia/ext/skia_memory_dump_provider.cc (right):

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
skia/ext/skia_memory_dump_provider.cc:36: // TODO(ssid): crbug.com/503168. Add
sub-allocation edge from discardable or
nit: Probably "edgeS" (because you say "dumpS" later)

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
File skia/ext/skia_memory_dump_provider.h (right):

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
skia/ext/skia_memory_dump_provider.h:14: class SK_API SkiaMemoryDumpProvider
It might be worth adding a short unittest (not sure if necessary).

[petrcermak, 2015-07-27 15:47:34]

nit: Please remove fullstop from the end of the title and the first line.

Thanks,
Petr

[ssid, 2015-07-27 16:21:57]

Thanks for the review. Made changes suggested.

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
File skia/ext/skia_memory_dump_provider.cc (right):

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
skia/ext/skia_memory_dump_provider.cc:36: // TODO(ssid): crbug.com/503168. Add
sub-allocation edge from discardable or
On 2015/07/27 15:45:39, petrcermak wrote:
> nit: Probably "edgeS" (because you say "dumpS" later)

Done.

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
File skia/ext/skia_memory_dump_provider.h (right):

https://codereview.chromium.org/1253403002/diff/20001/skia/ext/skia_memory_du...
skia/ext/skia_memory_dump_provider.h:14: class SK_API SkiaMemoryDumpProvider
On 2015/07/27 15:45:39, petrcermak wrote:
> It might be worth adding a short unittest (not sure if necessary).

Done.

[ssid, 2015-07-27 16:24:14]

ssid@chromium.org changed reviewers:
+ reed@google.com

[ssid, 2015-07-27 16:24:14]

+reed
This is the first version of the skia memory dump in tracing. PTAL.

Thanks.

[reed1, 2015-07-27 19:11:47]

lgtm

https://codereview.chromium.org/1253403002/diff/60001/skia/ext/skia_memory_du...
File skia/ext/skia_memory_dump_provider.h (right):

https://codereview.chromium.org/1253403002/diff/60001/skia/ext/skia_memory_du...
skia/ext/skia_memory_dump_provider.h:8: #include "SkPreConfig.h"
Hmmm, why do you need this? In general, we think PreConfig and PostConfig are
sort of "private" headers to skia.

[ssid, 2015-07-27 19:17:21]

Thanks, please see comment inline.

https://codereview.chromium.org/1253403002/diff/60001/skia/ext/skia_memory_du...
File skia/ext/skia_memory_dump_provider.h (right):

https://codereview.chromium.org/1253403002/diff/60001/skia/ext/skia_memory_du...
skia/ext/skia_memory_dump_provider.h:8: #include "SkPreConfig.h"
On 2015/07/27 19:11:47, reed1 wrote:
> Hmmm, why do you need this? In general, we think PreConfig and PostConfig are
> sort of "private" headers to skia.

This is needed for the SK_API define. Is that not required?

[ssid, 2015-07-27 19:27:56]

ssid@chromium.org changed reviewers:
+ sievers@chromium.org

[ssid, 2015-07-27 19:27:56]

+sievers
PTAL at 4 lines content/ changes. This adds initialization in browser and
renderer.
Thanks.

[reed1, 2015-07-27 20:13:49]

https://codereview.chromium.org/1253403002/diff/60001/skia/ext/skia_memory_du...
File skia/ext/skia_memory_dump_provider.h (right):

https://codereview.chromium.org/1253403002/diff/60001/skia/ext/skia_memory_du...
skia/ext/skia_memory_dump_provider.h:8: #include "SkPreConfig.h"
On 2015/07/27 19:17:21, ssid wrote:
> On 2015/07/27 19:11:47, reed1 wrote:
> > Hmmm, why do you need this? In general, we think PreConfig and PostConfig
are
> > sort of "private" headers to skia.
> 
> This is needed for the SK_API define. Is that not required?

#include "SkTypes.h" is our "base" header. You should include that if you just
need access to something like SK_API.

[no sievers, 2015-07-27 21:36:54]

content lgtm

[ssid, 2015-07-28 10:52:27]

ssid@chromium.org changed reviewers:
+ picksi@chromium.org, primiano@chromium.org

[ssid, 2015-07-28 12:29:02]

The CQ bit was checked by ssid@chromium.org

[ssid, 2015-07-28 12:29:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
petrcermak@chromium.org, sievers@chromium.org, reed@google.com
Link to the patchset: https://codereview.chromium.org/1253403002/#ps80001
(title: "Including correct header.")

[commit-bot: I haz the power, 2015-07-28 12:29:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1253403002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1253403002/80001

[commit-bot: I haz the power, 2015-07-28 13:03:07]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-07-28 13:03:39]

Patchset 5 (id:??) landed as
https://crrev.com/59c9691634783108308bd54f3f1ea39f795235cb
Cr-Commit-Position: refs/heads/master@{#340674}