[tracing] Add SkResourceCache statistics to chrome://tracing (chrome side)

skia/ext/SkTraceMemoryDump_chrome.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "skia/ext/SkTraceMemoryDump_chrome.h"
+
+#include "base/strings/string_util.h"
+#include "base/trace_event/memory_allocator_dump.h"
+#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/process_memory_dump.h"
+#include "skia/ext/SkDiscardableMemory_chrome.h"
+
+namespace skia {
+
+namespace {
+const char kMallocBackingType[] = "malloc";
+}
+
+SkTraceMemoryDump_Chrome::SkTraceMemoryDump_Chrome(
+    base::trace_event::ProcessMemoryDump* process_memory_dump)
+    : process_memory_dump_(process_memory_dump) {}
+
+SkTraceMemoryDump_Chrome::~SkTraceMemoryDump_Chrome() {}
+
+void SkTraceMemoryDump_Chrome::dumpNumericValue(const char* dumpName,
+                                                const char* valueName,
+                                                const char* units,
+                                                uint64_t value) {
+  auto dump = GetOrCreateAllocatorDump(dumpName);
+  dump->AddScalar(valueName, units, value);
+}
+
+void SkTraceMemoryDump_Chrome::setMemoryBacking(const char* dumpName,
+                                                const char* backingType,
+                                                const char* backingObjectId) {
+  if (base::CompareCaseInsensitiveASCII(backingType, kMallocBackingType) == 0) {
+    auto dump = GetOrCreateAllocatorDump(dumpName);

[Primiano Tucci (use gerrit), 2015/08/30 18:28:36]

why do we allow case insensitive compare here? I'd really like to avoid that the
code on the skia side can say either "malloc" or "MAllOc" unless there is a very
strong reason fgor that. it is harder to maintain and spot bugs otherwise.

[ssid, 2015/08/30 23:11:23]

I mainly made it because skia could say malloc or Malloc. If you want to avoid
that I'm fine. I will change to sensitive compare.

+    process_memory_dump_->AddSuballocation(
+        dump->guid(), base::trace_event::MemoryDumpManager::GetInstance()
+                          ->system_allocator_pool_name());
+  }
+}

[Primiano Tucci (use gerrit), 2015/08/30 18:28:36]

also, please add a else{ NOTREACHED()} section at the end, so we are sure we
catch cases where skia code sets the backing to something which does not exist.

[ssid, 2015/09/02 12:59:52]

Done.

+
+void SkTraceMemoryDump_Chrome::setDiscardableMemoryBacking(
+    const char* dumpName,
+    const SkDiscardableMemory& discardableMemoryObject) {
+  DCHECK(!process_memory_dump_->GetAllocatorDump(dumpName));
+  const SkDiscardableMemoryChrome& discardable_memory_obj =
+      static_cast<const SkDiscardableMemoryChrome&>(discardableMemoryObject);
+  auto dump = discardable_memory_obj.CreateMemoryAllocatorDump(

[Primiano Tucci (use gerrit), 2015/08/30 18:28:36]

why are you not using the GetOrCreateAllocatorDump here?
Looking at this code, it seems that this will fail you first call
dumpNumericValue and then setDiscardableMemoryBacking (which, I know, probably
you will not do but would be nice if the api was supporting this, given the
name).
That is the reason why in the previous patchset I suggested to extract the
GetOrAllocate method.
Now you extracted the method but are ysing only in one place, which ironically
doesn't justify anymore the extra method.
Unless I am missing something, I'd put a call to GetOrCreate here.

[ssid, 2015/08/30 23:11:23]

Yes it will fail if skia first calls dump and then tells discardable backed.
This is a special case. And I and Eric are the ones writing code and we will
keep that in mind while making dump provides.
No I cannot use get or create here.
Reason: this api is calling into
base::DiscardableMemory::allocateMemoryAllocatorDump. This api exposed by
discardable dumps with sizes. This is the reason why I wanted to change the set
discardable backing api to dump discardable backed memory api. But, you had
agreed that we will make this change later and not now.
I have extracted the function because it's used at two places already.

If you would like to add support for get our create from discardable memory api,
then please file a bug and I will change the discardable api and implementations
to use getOrCreate.

+      dumpName, process_memory_dump_);
+  DCHECK(dump);
+}
+
+base::trace_event::MemoryAllocatorDump*
+SkTraceMemoryDump_Chrome::GetOrCreateAllocatorDump(const char* dumpName) {
+  auto dump = process_memory_dump_->GetAllocatorDump(dumpName);
+  if (!dump)
+    dump = process_memory_dump_->CreateAllocatorDump(dumpName);
+  return dump;
+}
+
+}  // namespace skia