viz: Make SharedBitmap tracing consistent and more clear

Make SharedBitmap return an UnguessableToken as the shared bitmap-based
tracing id, if the SharedBitmap is in fact backed by shared memory for
cross-process use. This means ClientSharedBitmapManager always does
and ServerSharedBitmapManager returns null if the bitmap was allocated
inside the ServerSharedBitmapManager.

In ServerSharedBitmapManager memory dumps, if a bitmap is shared memory
backed, it dumps the |mapped_id()| which is the UnguessableToken.
Otherwise, it dumps the SharedBitmapGUID from the SharedBitmapId.

In ResourceProvider, if the SharedBitmap has an UnguessableToken for
its shared memory backing, it dumps that, else uses the SharedBitmapID
to make a GUID.

In ResourcePool, the same thing is done as ResourceProvider.

Thus, all child-process bitmaps will be dumped via the shared memory
mapped_id() which is an UnguessableToken. And all service-process
bitmaps are dumped via the SharedBitmapGUID.

R=ericrk@chromium.org

Bug: 730660
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Id4b1c20350a139d36df13743d012756181937c6e
Reviewed-on: https://chromium-review.googlesource.com/935428
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541275}

cc/resources/resource_pool.cc

@@ -570,11 +570,11 @@ void ResourcePool::PoolResource::OnMemoryDump(
   base::UnguessableToken shm_guid;
   base::trace_event::MemoryAllocatorDumpGuid backing_guid;
   if (shared_bitmap_) {
-    // Software resources are in shared memory but are kept closed to avoid
-    // holding an fd open. So there is no SharedMemoryHandle to get a guid
-    // from.
-    DCHECK(!shared_bitmap_->GetSharedMemoryHandle().IsValid());
-    backing_guid = viz::GetSharedBitmapGUIDForTracing(shared_bitmap_->id());
+    // If the SharedBitmap was allocated for cross-process use, then it has this
+    // |shm_guid|, else we fallback to the SharedBitmapGUID.
+    shm_guid = shared_bitmap_->GetCrossProcessGUID();
+    if (shm_guid.is_empty())
+      backing_guid = viz::GetSharedBitmapGUIDForTracing(shared_bitmap_->id());
   } else if (gpu_backing_) {
     // We prefer the SharedMemoryGuid() if it exists, if the resource is backed
     // by shared memory.

cc/resources/resource_provider.cc

@@ -207,7 +207,7 @@ bool ResourceProvider::OnMemoryDump(
         backing_memory_allocated = !!resource.gl_id;
         break;
       case viz::ResourceType::kBitmap:
-        backing_memory_allocated = resource.has_shared_bitmap_id;
+        backing_memory_allocated = !!resource.shared_bitmap;
         break;
     }
 
@@ -236,10 +236,16 @@ bool ResourceProvider::OnMemoryDump(
     base::UnguessableToken shared_memory_guid;
     switch (resource.type) {
       case viz::ResourceType::kGpuMemoryBuffer:
-        guid =
-            resource.gpu_memory_buffer->GetGUIDForTracing(tracing_process_id);
+        // GpuMemoryBuffers may be backed by shared memory, and in that case we
+        // use the guid from there to attribute for the global shared memory
+        // dumps. Otherwise, they may be backed by native structures, and we
+        // fall back to that with GetGUIDForTracing.
         shared_memory_guid =
             resource.gpu_memory_buffer->GetHandle().handle.GetGUID();
+        if (shared_memory_guid.is_empty()) {
+          guid =
+              resource.gpu_memory_buffer->GetGUIDForTracing(tracing_process_id);
+        }
         break;
       case viz::ResourceType::kTexture:
         DCHECK(resource.gl_id);
@@ -249,24 +255,28 @@ bool ResourceProvider::OnMemoryDump(
             resource.gl_id);
         break;
       case viz::ResourceType::kBitmap:
-        DCHECK(resource.has_shared_bitmap_id);
-        guid = viz::GetSharedBitmapGUIDForTracing(resource.shared_bitmap_id);
-        if (resource.shared_bitmap) {
-          shared_memory_guid =
-              resource.shared_bitmap->GetSharedMemoryHandle().GetGUID();
-        }
+        // If the resource comes from out of process, it will have this id,
+        // which we prefer. Otherwise, we fall back to the SharedBitmapGUID
+        // which can be generated for in-process bitmaps.
+        shared_memory_guid = resource.shared_bitmap->GetCrossProcessGUID();
+        if (shared_memory_guid.is_empty())
+          guid = viz::GetSharedBitmapGUIDForTracing(resource.shared_bitmap_id);
         break;
     }
 
-    DCHECK(!guid.empty());
+    DCHECK(!shared_memory_guid.is_empty() || !guid.empty());
 
-    const int kImportance = 2;
+    const int kImportanceForInteral = 2;
+    const int kImportanceForExternal = 1;
+    int importance = resource.origin == viz::internal::Resource::INTERNAL
+                         ? kImportanceForInteral
+                         : kImportanceForExternal;
     if (!shared_memory_guid.is_empty()) {
       pmd->CreateSharedMemoryOwnershipEdge(dump->guid(), shared_memory_guid,
-                                           kImportance);
+                                           importance);
     } else {
       pmd->CreateSharedGlobalAllocatorDump(guid);
-      pmd->AddOwnershipEdge(dump->guid(), guid, kImportance);
+      pmd->AddOwnershipEdge(dump->guid(), guid, importance);
     }
   }
 

components/viz/client/client_shared_bitmap_manager.cc

@@ -32,7 +32,8 @@ class ClientSharedBitmap : public SharedBitmap {
                      id,
                      sequence_number),
         shared_bitmap_allocation_notifier_(
-            std::move(shared_bitmap_allocation_notifier)) {}
+            std::move(shared_bitmap_allocation_notifier)),
+        tracing_id_(shared_memory->mapped_id()) {}
 
   ClientSharedBitmap(
       scoped_refptr<mojom::ThreadSafeSharedBitmapAllocationNotifierPtr>
@@ -51,17 +52,16 @@ class ClientSharedBitmap : public SharedBitmap {
     (*shared_bitmap_allocation_notifier_)->DidDeleteSharedBitmap(id());
   }
 
-  // SharedBitmap:
-  base::SharedMemoryHandle GetSharedMemoryHandle() const override {
-    if (!shared_memory_holder_)
-      return base::SharedMemoryHandle();
-    return shared_memory_holder_->handle();
+  // SharedBitmap implementation.
+  base::UnguessableToken GetCrossProcessGUID() const override {
+    return tracing_id_;
   }
 
  private:
   scoped_refptr<mojom::ThreadSafeSharedBitmapAllocationNotifierPtr>
       shared_bitmap_allocation_notifier_;
   std::unique_ptr<base::SharedMemory> shared_memory_holder_;
+  base::UnguessableToken tracing_id_;
 };
 
 // Collect extra information for debugging bitmap creation failures.

components/viz/common/quads/shared_bitmap.h

@@ -16,7 +16,7 @@
 #include "ui/gfx/geometry/size.h"
 
 namespace base {
-class SharedMemoryHandle;
+class UnguessableToken;
 }
 
 namespace viz {
@@ -40,16 +40,15 @@ class VIZ_COMMON_EXPORT SharedBitmap {
   virtual ~SharedBitmap();
 
   uint8_t* pixels() { return pixels_; }
-
   const SharedBitmapId& id() { return id_; }
 
   // The sequence number that ClientSharedBitmapManager assigned to this
   // SharedBitmap.
   uint32_t sequence_number() const { return sequence_number_; }
 
-  // Returns the shared memory's handle when the back end is base::SharedMemory.
-  // Otherwise, this returns an invalid handle.
-  virtual base::SharedMemoryHandle GetSharedMemoryHandle() const = 0;
+  // Returns the GUID for tracing when the SharedBitmap supports cross-process
+  // use via shared memory. Otherwise, this returns empty.
+  virtual base::UnguessableToken GetCrossProcessGUID() const = 0;
 
   // Returns true if the size is valid and false otherwise.
   static bool SizeInBytes(const gfx::Size& size, size_t* size_in_bytes);

components/viz/service/display_embedder/server_shared_bitmap_manager.cc

@@ -22,7 +22,9 @@ namespace viz {
 class BitmapData : public base::RefCountedThreadSafe<BitmapData> {
  public:
   explicit BitmapData(size_t buffer_size) : buffer_size(buffer_size) {}
+  // For shm allocated and shared from a client out-of-process.
   std::unique_ptr<base::SharedMemory> memory;
+  // For memory allocated by the ServerSharedBitmapManager for in-process.
   std::unique_ptr<uint8_t[]> pixels;
   size_t buffer_size;
 
@@ -49,11 +51,13 @@ class ServerSharedBitmap : public SharedBitmap {
       manager_->FreeSharedMemoryFromMap(id());
   }
 
-  // SharedBitmap:
-  base::SharedMemoryHandle GetSharedMemoryHandle() const override {
-    if (!bitmap_data_->memory)
-      return base::SharedMemoryHandle();
-    return bitmap_data_->memory->handle();
+  // SharedBitmap implementation.
+  base::UnguessableToken GetCrossProcessGUID() const override {
+    if (!bitmap_data_->memory) {
+      // Locally allocated for in-process use.
+      return {};
+    }
+    return bitmap_data_->memory->mapped_id();
   }
 
  private:
@@ -151,7 +155,8 @@ bool ServerSharedBitmapManager::ChildAllocatedSharedBitmapForTest(
     const SharedBitmapId& id) {
   auto data = base::MakeRefCounted<BitmapData>(buffer_size);
   data->memory = std::make_unique<base::SharedMemory>(memory_handle, false);
-  data->memory->Map(data->buffer_size);
+  if (!data->memory->Map(data->buffer_size))
+    return false;
   data->memory->Close();
 
   base::AutoLock lock(lock_);
@@ -172,30 +177,32 @@ bool ServerSharedBitmapManager::OnMemoryDump(
     base::trace_event::ProcessMemoryDump* pmd) {
   base::AutoLock lock(lock_);
 
-  for (const auto& bitmap : handle_map_) {
+  for (const auto& pair : handle_map_) {
+    const SharedBitmapId& id = pair.first;
+    BitmapData* data = pair.second.get();
+
+    std::string dump_str = base::StringPrintf(
+        "sharedbitmap/%s", base::HexEncode(id.name, sizeof(id.name)).c_str());
     base::trace_event::MemoryAllocatorDump* dump =
-        pmd->CreateAllocatorDump(base::StringPrintf(
-            "sharedbitmap/%s",
-            base::HexEncode(bitmap.first.name, sizeof(bitmap.first.name))
-                .c_str()));
+        pmd->CreateAllocatorDump(dump_str);
     if (!dump)
       return false;
 
     dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
                     base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                    bitmap.second->buffer_size);
-
-    if (bitmap.second->memory) {
-      base::UnguessableToken shared_memory_guid =
-          bitmap.second->memory->mapped_id();
-      if (!shared_memory_guid.is_empty()) {
-        pmd->CreateSharedMemoryOwnershipEdge(dump->guid(), shared_memory_guid,
-                                             0 /* importance*/);
-      }
+                    data->buffer_size);
+
+    if (data->memory) {
+      // Resources from a client have shared memory, and we use the guid from
+      // that.
+      base::UnguessableToken shared_memory_guid = data->memory->mapped_id();
+      DCHECK(!shared_memory_guid.is_empty());
+      pmd->CreateSharedMemoryOwnershipEdge(dump->guid(), shared_memory_guid,
+                                           0 /* importance*/);
     } else {
-      // Generate a global GUID used to share this allocation with renderer
-      // processes.
-      auto guid = GetSharedBitmapGUIDForTracing(bitmap.first);
+      // Otherwise, resources were allocated locally for in-process use, and
+      // there is no shared memory. Instead make up a GUID for them.
+      auto guid = GetSharedBitmapGUIDForTracing(id);
       pmd->CreateSharedGlobalAllocatorDump(guid);
       pmd->AddOwnershipEdge(dump->guid(), guid);
     }

components/viz/service/display_embedder/server_shared_bitmap_manager_unittest.cc

@@ -160,13 +160,14 @@ TEST_F(ServerSharedBitmapManagerTest, SharedMemoryHandle) {
   size_t size_in_bytes;
   EXPECT_TRUE(SharedBitmap::SizeInBytes(bitmap_size, &size_in_bytes));
   std::unique_ptr<base::SharedMemory> bitmap(new base::SharedMemory());
-  auto shared_memory_guid = bitmap->handle().GetGUID();
   bitmap->CreateAndMapAnonymous(size_in_bytes);
   memset(bitmap->memory(), 0xff, size_in_bytes);
-  SharedBitmapId id = SharedBitmap::GenerateId();
+  base::UnguessableToken shared_memory_guid = bitmap->handle().GetGUID();
+  EXPECT_FALSE(shared_memory_guid.is_empty());
 
   SharedBitmapAllocationNotifierImpl notifier(manager());
 
+  SharedBitmapId id = SharedBitmap::GenerateId();
   base::SharedMemoryHandle handle = bitmap->handle().Duplicate();
   mojo::ScopedSharedBufferHandle buffer_handle = mojo::WrapSharedMemoryHandle(
       handle, size_in_bytes,
@@ -175,8 +176,7 @@ TEST_F(ServerSharedBitmapManagerTest, SharedMemoryHandle) {
 
   std::unique_ptr<SharedBitmap> shared_bitmap;
   shared_bitmap = manager()->GetSharedBitmapFromId(gfx::Size(1, 1), id);
-  EXPECT_EQ(shared_bitmap->GetSharedMemoryHandle().GetGUID(),
-            shared_memory_guid);
+  EXPECT_EQ(shared_bitmap->GetCrossProcessGUID(), shared_memory_guid);
 
   notifier.DidDeleteSharedBitmap(id);
 }

components/viz/test/test_shared_bitmap_manager.cc

@@ -27,8 +27,8 @@ class OwnedSharedBitmap : public SharedBitmap {
   ~OwnedSharedBitmap() override = default;
 
   // SharedBitmap:
-  base::SharedMemoryHandle GetSharedMemoryHandle() const override {
-    return shared_memory_->handle();
+  base::UnguessableToken GetCrossProcessGUID() const override {
+    return shared_memory_->mapped_id();
   }
 
  private:
@@ -37,13 +37,19 @@ class OwnedSharedBitmap : public SharedBitmap {
 
 class UnownedSharedBitmap : public SharedBitmap {
  public:
-  UnownedSharedBitmap(uint8_t* pixels, const SharedBitmapId& id)
-      : SharedBitmap(pixels, id, g_next_sequence_number++) {}
+  UnownedSharedBitmap(uint8_t* pixels,
+                      const SharedBitmapId& id,
+                      const base::UnguessableToken& tracing_id)
+      : SharedBitmap(pixels, id, g_next_sequence_number++),
+        tracing_id_(tracing_id) {}
 
   // SharedBitmap:
-  base::SharedMemoryHandle GetSharedMemoryHandle() const override {
-    return base::SharedMemoryHandle();
+  base::UnguessableToken GetCrossProcessGUID() const override {
+    return tracing_id_;
   }
+
+ private:
+  base::UnguessableToken tracing_id_;
 };
 
 }  // namespace
@@ -69,7 +75,8 @@ std::unique_ptr<SharedBitmap> TestSharedBitmapManager::GetSharedBitmapFromId(
   if (bitmap_map_.find(id) == bitmap_map_.end())
     return nullptr;
   uint8_t* pixels = static_cast<uint8_t*>(bitmap_map_[id]->memory());
-  return std::make_unique<UnownedSharedBitmap>(pixels, id);
+  const base::UnguessableToken& tracing_id = bitmap_map_[id]->mapped_id();
+  return std::make_unique<UnownedSharedBitmap>(pixels, id, tracing_id);
 }
 
 bool TestSharedBitmapManager::ChildAllocatedSharedBitmap(