CanvasMemoryDumpProvider should be thread-safe.

RegisterClient and UnregisterClient methods will be invoked in different threads at the same time. This causes ASAN crashes.

Bug: 1092385
Change-Id: I67ed7fd1925bde72770c8c5b4f737258e9564de5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324689
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Reviewed-by: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792654}

third_party/blink/renderer/platform/instrumentation/canvas_memory_dump_provider.cc

@@ -21,14 +21,20 @@ bool CanvasMemoryDumpProvider::OnMemoryDump(
     base::trace_event::ProcessMemoryDump* memory_dump) {
   if (args.level_of_detail ==
       base::trace_event::MemoryDumpLevelOfDetail::DETAILED) {
+    base::AutoLock auto_lock(lock_);
     for (auto* it : clients_)
       it->OnMemoryDump(memory_dump);
     return true;
   }
 
   size_t total_size = 0;
-  for (auto* it : clients_)
-    total_size += it->GetSize();
+  size_t clients_size = 0;
+  {
+    base::AutoLock auto_lock(lock_);
+    for (auto* it : clients_)
+      total_size += it->GetSize();
+    clients_size = clients_.size();
+  }
 
   auto* dump =
       memory_dump->CreateAllocatorDump("canvas/ResourceProvider/SkSurface");
@@ -37,7 +43,7 @@ bool CanvasMemoryDumpProvider::OnMemoryDump(
                   total_size);
   dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount,
                   base::trace_event::MemoryAllocatorDump::kUnitsObjects,
-                  clients_.size());
+                  clients_size);
 
   // SkiaMemoryDumpProvider reports only sk_glyph_cache and sk_resource_cache.
   // So the SkSurface is suballocation of malloc, not SkiaDumpProvider.
@@ -50,11 +56,13 @@ bool CanvasMemoryDumpProvider::OnMemoryDump(
 }
 
 void CanvasMemoryDumpProvider::RegisterClient(CanvasMemoryDumpClient* client) {
+  base::AutoLock auto_lock(lock_);
   clients_.insert(client);
 }
 
 void CanvasMemoryDumpProvider::UnregisterClient(
     CanvasMemoryDumpClient* client) {
+  base::AutoLock auto_lock(lock_);
   DCHECK(clients_.Contains(client));
   clients_.erase(client);
 }

third_party/blink/renderer/platform/instrumentation/canvas_memory_dump_provider.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_INSTRUMENTATION_CANVAS_MEMORY_DUMP_PROVIDER_H_
 
 #include "base/macros.h"
+#include "base/synchronization/lock.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
@@ -39,7 +40,8 @@ class PLATFORM_EXPORT CanvasMemoryDumpProvider final
  private:
   CanvasMemoryDumpProvider() = default;
 
-  WTF::HashSet<CanvasMemoryDumpClient*> clients_;
+  base::Lock lock_;
+  WTF::HashSet<CanvasMemoryDumpClient*> clients_ GUARDED_BY(lock_);
 
   DISALLOW_COPY_AND_ASSIGN(CanvasMemoryDumpProvider);
 };