cc: Stop calling DetachFromClient if bind failed.

blimp/client/core/compositor/blimp_compositor_frame_sink.cc

Index: blimp/client/core/compositor/blimp_compositor_frame_sink.cc
diff --git a/blimp/client/core/compositor/blimp_compositor_frame_sink.cc b/blimp/client/core/compositor/blimp_compositor_frame_sink.cc
index 439585555ca623d76e610b9733fc66f830b4a2e4..c5ed1af061a0d5a4f5926b31c7e899c66305822e 100644
--- a/blimp/client/core/compositor/blimp_compositor_frame_sink.cc
+++ b/blimp/client/core/compositor/blimp_compositor_frame_sink.cc
@@ -27,7 +27,6 @@ BlimpCompositorFrameSink::BlimpCompositorFrameSink(
                               std::move(worker_context_provider)),
       main_task_runner_(std::move(main_task_runner)),
       main_thread_proxy_(main_thread_proxy),
-      bound_to_client_(false),
       weak_factory_(this) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 }
@@ -46,30 +45,26 @@ void BlimpCompositorFrameSink::SwapCompositorFrameAck() {
 
 bool BlimpCompositorFrameSink::BindToClient(
     cc::CompositorFrameSinkClient* client) {
-  bool success = cc::CompositorFrameSink::BindToClient(client);
-  if (success) {
-    begin_frame_source_ = base::MakeUnique<cc::DelayBasedBeginFrameSource>(
-        base::MakeUnique<cc::DelayBasedTimeSource>(
-            base::ThreadTaskRunnerHandle::Get().get()));
-    client->SetBeginFrameSource(begin_frame_source_.get());
+  if (!cc::CompositorFrameSink::BindToClient(client))
+    return false;
 
-    main_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&BlimpCompositorFrameSinkProxy::BindToProxyClient,
-                              main_thread_proxy_, weak_factory_.GetWeakPtr()));
-  }
-  return success;
+  begin_frame_source_ = base::MakeUnique<cc::DelayBasedBeginFrameSource>(
+      base::MakeUnique<cc::DelayBasedTimeSource>(
+          base::ThreadTaskRunnerHandle::Get().get()));
+  client->SetBeginFrameSource(begin_frame_source_.get());
+
+  main_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&BlimpCompositorFrameSinkProxy::BindToProxyClient,
+                            main_thread_proxy_, weak_factory_.GetWeakPtr()));
+  return true;
 }
 
 void BlimpCompositorFrameSink::DetachFromClient() {
   cc::CompositorFrameSink::DetachFromClient();
 
-  if (bound_to_client_ == true) {
-    bound_to_client_ = false;
-    main_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&BlimpCompositorFrameSinkProxy::UnbindProxyClient,
-                              main_thread_proxy_));
-  }
-
+  main_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&BlimpCompositorFrameSinkProxy::UnbindProxyClient,

[enne (OOO), 2016/10/10 22:27:10]

Is this a change in behavior, where this wouldn't have been called previously?

[danakj, 2016/10/10 22:35:25]

Yeah because it was in a dead branch. But it was meant to be called. So.. I
think this is right. This cleans up stuff in the blimp display compositor, which
is also cleaned up in its destructor, but would have leaked on context
loss/recovry.

[Khushal, 2016/10/10 22:55:56]

Thanks for the cleanup. Now that we call Detach only when the bind is
successful, this simplifies it a lot.

+                            main_thread_proxy_));
   weak_factory_.InvalidateWeakPtrs();
 }