viz: Print FrameSink debug label on orphaned temporary references

FrameSink debug labels are useful to identify the client associated with a
particular FrameSink or Surface. This CL introduces the appropriate plumbing
to allow printing the FrameSink debug label given a FrameSinkId from the
surfaces layer. The debug label will now be printed if a temporary
reference is orphand and will be garbage collected.

Bug: 655231
Change-Id: I75abd9b6aff484fee946e2ee584bd9927a0b843c
Reviewed-on: https://chromium-review.googlesource.com/c/1320018Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605846}

components/viz/common/surfaces/frame_sink_id.cc

@@ -12,6 +12,12 @@ std::string FrameSinkId::ToString() const {
   return base::StringPrintf("FrameSinkId(%u, %u)", client_id_, sink_id_);
 }
 
+std::string FrameSinkId::ToString(base::StringPiece debug_label) const {
+  return base::StringPrintf("FrameSinkId[%s](%u, %u)",
+                            debug_label.as_string().c_str(), client_id_,
+                            sink_id_);
+}
+
 std::ostream& operator<<(std::ostream& out, const FrameSinkId& frame_sink_id) {
   return out << frame_sink_id.ToString();
 }

components/viz/common/surfaces/frame_sink_id.h

@@ -12,6 +12,7 @@
 #include <tuple>
 
 #include "base/hash.h"
+#include "base/strings/string_piece.h"
 #include "components/viz/common/viz_common_export.h"
 
 namespace viz {
@@ -63,6 +64,8 @@ class VIZ_COMMON_EXPORT FrameSinkId {
 
   std::string ToString() const;
 
+  std::string ToString(base::StringPiece debug_label) const;
+
  private:
   uint32_t client_id_;
   uint32_t sink_id_;

components/viz/common/surfaces/surface_id.cc

@@ -17,6 +17,14 @@ std::string SurfaceId::ToString() const {
                             local_surface_id_.ToString().c_str());
 }
 
+std::string SurfaceId::ToString(
+    base::StringPiece frame_sink_debug_label) const {
+  return base::StringPrintf(
+      "SurfaceId(%s, %s)",
+      frame_sink_id_.ToString(frame_sink_debug_label).c_str(),
+      local_surface_id_.ToString().c_str());
+}
+
 SurfaceId SurfaceId::ToSmallestId() const {
   return SurfaceId(frame_sink_id_, local_surface_id_.ToSmallestId());
 }

components/viz/common/surfaces/surface_id.h

@@ -59,6 +59,8 @@ class VIZ_COMMON_EXPORT SurfaceId {
 
   std::string ToString() const;
 
+  std::string ToString(base::StringPiece frame_sink_debug_label) const;
+
   // Returns whether this SurfaceId was generated after |other|.
   bool IsNewerThan(const SurfaceId& other) const;
 

components/viz/service/BUILD.gn

@@ -165,6 +165,7 @@ viz_component("service") {
     "surfaces/surface_hittest_delegate.h",
     "surfaces/surface_manager.cc",
     "surfaces/surface_manager.h",
+    "surfaces/surface_manager_delegate.h",
     "surfaces/surface_reference.cc",
     "surfaces/surface_reference.h",
     "viz_service_export.h",

components/viz/service/display/display_scheduler_unittest.cc

@@ -123,7 +123,7 @@ class DisplaySchedulerTest : public testing::Test {
   explicit DisplaySchedulerTest(bool wait_for_all_surfaces_before_draw = false)
       : fake_begin_frame_source_(0.f, false),
         task_runner_(new base::NullTaskRunner),
-        surface_manager_(4u),
+        surface_manager_(nullptr, 4u),
         scheduler_(&fake_begin_frame_source_,
                    &surface_manager_,
                    task_runner_.get(),

components/viz/service/frame_sinks/frame_sink_manager_impl.cc

@@ -48,7 +48,7 @@ FrameSinkManagerImpl::FrameSinkManagerImpl(
     DisplayProvider* display_provider)
     : shared_bitmap_manager_(shared_bitmap_manager),
       display_provider_(display_provider),
-      surface_manager_(activation_deadline_in_frames),
+      surface_manager_(this, activation_deadline_in_frames),
       hit_test_manager_(surface_manager()),
       binding_(this) {
   surface_manager_.AddObserver(&hit_test_manager_);
@@ -341,6 +341,14 @@ void FrameSinkManagerImpl::OnAggregatedHitTestRegionListUpdated(
   }
 }
 
+base::StringPiece FrameSinkManagerImpl::GetFrameSinkDebugLabel(
+    const FrameSinkId& frame_sink_id) const {
+  auto it = frame_sink_data_.find(frame_sink_id);
+  if (it != frame_sink_data_.end())
+    return it->second.debug_label;
+  return base::StringPiece();
+}
+
 void FrameSinkManagerImpl::RegisterCompositorFrameSinkSupport(
     const FrameSinkId& frame_sink_id,
     CompositorFrameSinkSupport* support) {
@@ -537,14 +545,6 @@ void FrameSinkManagerImpl::RemoveObserver(FrameSinkObserver* obs) {
   observer_list_.RemoveObserver(obs);
 }
 
-base::StringPiece FrameSinkManagerImpl::GetFrameSinkDebugLabel(
-    const FrameSinkId& frame_sink_id) const {
-  auto it = frame_sink_data_.find(frame_sink_id);
-  if (it != frame_sink_data_.end())
-    return it->second.debug_label;
-  return base::StringPiece();
-}
-
 std::vector<FrameSinkId> FrameSinkManagerImpl::GetCreatedFrameSinkIds() const {
   std::vector<FrameSinkId> frame_sink_ids;
   for (auto& map_entry : support_map_)

components/viz/service/frame_sinks/frame_sink_manager_impl.h

@@ -29,6 +29,7 @@
 #include "components/viz/service/hit_test/hit_test_aggregator_delegate.h"
 #include "components/viz/service/hit_test/hit_test_manager.h"
 #include "components/viz/service/surfaces/surface_manager.h"
+#include "components/viz/service/surfaces/surface_manager_delegate.h"
 #include "components/viz/service/surfaces/surface_observer.h"
 #include "components/viz/service/viz_service_export.h"
 #include "gpu/ipc/common/surface_handle.h"
@@ -50,7 +51,8 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl
     : public SurfaceObserver,
       public FrameSinkVideoCapturerManager,
       public mojom::FrameSinkManager,
-      public HitTestAggregatorDelegate {
+      public HitTestAggregatorDelegate,
+      public SurfaceManagerDelegate {
  public:
   explicit FrameSinkManagerImpl(
       SharedBitmapManager* shared_bitmap_manager,
@@ -123,6 +125,10 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl
       const FrameSinkId& frame_sink_id,
       const std::vector<AggregatedHitTestRegion>& hit_test_data) override;
 
+  // SurfaceManagerDelegate implementation:
+  base::StringPiece GetFrameSinkDebugLabel(
+      const FrameSinkId& frame_sink_id) const override;
+
   // CompositorFrameSinkSupport, hierarchy, and BeginFrameSource can be
   // registered and unregistered in any order with respect to each other.
   //
@@ -175,10 +181,6 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl
   void AddObserver(FrameSinkObserver* obs);
   void RemoveObserver(FrameSinkObserver* obs);
 
-  // Returns the debug label associated with |frame_sink_id| if any.
-  base::StringPiece GetFrameSinkDebugLabel(
-      const FrameSinkId& frame_sink_id) const;
-
   // Returns ids of all FrameSinks that were created.
   std::vector<FrameSinkId> GetCreatedFrameSinkIds() const;
   // Returns ids of all FrameSinks that were registered.

components/viz/service/surfaces/surface_manager.cc

@@ -20,6 +20,7 @@
 #include "components/viz/common/surfaces/surface_info.h"
 #include "components/viz/service/surfaces/surface.h"
 #include "components/viz/service/surfaces/surface_client.h"
+#include "components/viz/service/surfaces/surface_manager_delegate.h"
 
 #if DCHECK_IS_ON()
 #include <sstream>
@@ -41,8 +42,10 @@ const char kUmaRemovedTemporaryReference[] =
 }  // namespace
 
 SurfaceManager::SurfaceManager(
+    SurfaceManagerDelegate* delegate,
     base::Optional<uint32_t> activation_deadline_in_frames)
-    : activation_deadline_in_frames_(activation_deadline_in_frames),
+    : delegate_(delegate),
+      activation_deadline_in_frames_(activation_deadline_in_frames),
       dependency_tracker_(this),
       root_surface_id_(FrameSinkId(0u, 0u),
                        LocalSurfaceId(1u, base::UnguessableToken::Create())),
@@ -480,7 +483,13 @@ void SurfaceManager::ExpireOldTemporaryReferences() {
       // The temporary reference has existed for more than 10 seconds, a surface
       // reference should have replaced it by now. To avoid permanently leaking
       // memory delete the temporary reference.
-      DLOG(ERROR) << "Old/orphaned temporary reference to " << surface_id;
+      base::StringPiece frame_sink_debug_label;
+      if (delegate_) {
+        frame_sink_debug_label =
+            delegate_->GetFrameSinkDebugLabel(surface_id.frame_sink_id());
+      }
+      DLOG(ERROR) << "Old/orphaned temporary reference to "
+                  << surface_id.ToString(frame_sink_debug_label);
       temporary_references_to_delete.push_back(surface_id);
     } else if (IsMarkedForDestruction(surface_id)) {
       // Never mark live surfaces as old, they can't be garbage collected.

components/viz/service/surfaces/surface_manager.h

@@ -39,13 +39,14 @@ class TickClock;
 namespace viz {
 
 class Surface;
+class SurfaceManagerDelegate;
 struct BeginFrameAck;
 struct BeginFrameArgs;
 
 class VIZ_SERVICE_EXPORT SurfaceManager {
  public:
-  explicit SurfaceManager(
-      base::Optional<uint32_t> activation_deadline_in_frames);
+  SurfaceManager(SurfaceManagerDelegate* delegate,
+                 base::Optional<uint32_t> activation_deadline_in_frames);
   ~SurfaceManager();
 
 #if DCHECK_IS_ON()
@@ -270,6 +271,9 @@ class VIZ_SERVICE_EXPORT SurfaceManager {
   // Returns true if |surface_id| is in the garbage collector's queue.
   bool IsMarkedForDestruction(const SurfaceId& surface_id);
 
+  // Can be nullptr.
+  SurfaceManagerDelegate* const delegate_;
+
   base::Optional<uint32_t> activation_deadline_in_frames_;
 
   // SurfaceDependencyTracker needs to be destroyed after Surfaces are destroyed

components/viz/service/surfaces/surface_manager_delegate.h

+// Copyright 2018 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.
+
+#ifndef COMPONENTS_VIZ_SERVICE_SURFACES_SURFACE_MANAGER_DELEGATE_H_
+#define COMPONENTS_VIZ_SERVICE_SURFACES_SURFACE_MANAGER_DELEGATE_H_
+
+#include "base/strings/string_piece.h"
+#include "components/viz/service/viz_service_export.h"
+
+namespace viz {
+
+class VIZ_SERVICE_EXPORT SurfaceManagerDelegate {
+ public:
+  virtual ~SurfaceManagerDelegate() = default;
+
+  // Returns the debug label associated with |frame_sink_id| if any.
+  virtual base::StringPiece GetFrameSinkDebugLabel(
+      const FrameSinkId& frame_sink_id) const = 0;
+};
+
+}  // namespace viz
+
+#endif  // COMPONENTS_VIZ_SERVICE_SURFACES_SURFACE_MANAGER_DELEGATE_H_