exo::CompositorFrameSinkHolder's release callbacks hold ref

components/exo/compositor_frame_sink_holder.cc

Index: components/exo/compositor_frame_sink_holder.cc
diff --git a/components/exo/compositor_frame_sink_holder.cc b/components/exo/compositor_frame_sink_holder.cc
index dd82aff4dc1dc915afc79434ccbb091dc0c6d476..e2a730ad4e746eea1924261ea7167ac95e9a0059 100644
--- a/components/exo/compositor_frame_sink_holder.cc
+++ b/components/exo/compositor_frame_sink_holder.cc
@@ -4,6 +4,7 @@
 
 #include "components/exo/compositor_frame_sink_holder.h"
 
+#include "base/callback_helpers.h"
 #include "cc/resources/returned_resource.h"
 #include "components/exo/surface.h"
 
@@ -32,7 +33,7 @@ bool CompositorFrameSinkHolder::HasReleaseCallbackForResource(
 void CompositorFrameSinkHolder::AddResourceReleaseCallback(
     cc::ResourceId id,
     std::unique_ptr<cc::SingleReleaseCallback> callback) {
-  release_callbacks_[id] = std::make_pair(this, std::move(callback));
+  release_callbacks_[id] = std::move(callback);
 }
 
 void CompositorFrameSinkHolder::ActivateFrameCallbacks(
@@ -83,10 +84,10 @@ void CompositorFrameSinkHolder::ReclaimResources(
   for (auto& resource : resources) {
     auto it = release_callbacks_.find(resource.id);
     DCHECK(it != release_callbacks_.end());
-    std::unique_ptr<cc::SingleReleaseCallback> callback =
-        std::move(it->second.second);
+    base::ScopedClosureRunner runner(base::Bind(

[reveman, 2016/12/22 17:00:53]

You're still not using PostTask here so this sink holder instance might be
destroyed before this function returns. That it might be destroyed in the middle
of a loop seems especially bad. Please use PostTask or at least move all
callbacks to a temporary variable and run them after the loop that access
release_callbacks_.

[Alex Z., 2016/12/22 18:08:48]

I thought you wanted the ScopedClosureRunner to protect the callback against the
message loop shutting down before the callback gets to run?

[reveman, 2016/12/22 20:29:49]

Yes, the only way that can happen is if you post a task to the message loop.
Without a PostTask you're still letting the callback run here and the sink
holder instance to be destroyed in the middle of this loop inside a member
function of the sink holder.

[Alex Z., 2016/12/23 22:45:38]

Done.

+        &cc::SingleReleaseCallback::Run, base::Passed(std::move(it->second)),
+        resource.sync_token, resource.lost));
     release_callbacks_.erase(it);
-    callback->Run(resource.sync_token, resource.lost);
   }
 }