Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

gpu/ipc/service/pass_through_image_transport_surface.cc

Index: gpu/ipc/service/pass_through_image_transport_surface.cc
diff --git a/gpu/ipc/service/pass_through_image_transport_surface.cc b/gpu/ipc/service/pass_through_image_transport_surface.cc
index 8e11cad097db652d4cde4f94dc0c1fd1669597cd..416d6f483fb664a42c35faa61c9d782fe86cb1d7 100644
--- a/gpu/ipc/service/pass_through_image_transport_surface.cc
+++ b/gpu/ipc/service/pass_through_image_transport_surface.cc
@@ -9,7 +9,6 @@
 #include "base/command_line.h"
 #include "build/build_config.h"
 #include "gpu/ipc/common/gpu_messages.h"

[piman, 2016/11/09 22:49:36]

nit: ditto, still needed?

[Fady Samuel, 2016/11/09 23:28:11]

Nope removed.

-#include "gpu/ipc/service/gpu_command_buffer_stub.h"
 #include "ui/gfx/vsync_provider.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_switches.h"
@@ -17,19 +16,19 @@
 namespace gpu {
 
 PassThroughImageTransportSurface::PassThroughImageTransportSurface(
-    GpuCommandBufferStub* stub,
+    base::WeakPtr<ImageTransportSurfaceDelegate> delegate,
     gl::GLSurface* surface)
     : GLSurfaceAdapter(surface),
-      stub_(stub->AsWeakPtr()),
+      delegate_(delegate),
       did_set_swap_interval_(false),
       weak_ptr_factory_(this) {}
 
 bool PassThroughImageTransportSurface::Initialize(
     gl::GLSurface::Format format) {
   // The surface is assumed to have already been initialized.
-  if (!stub_.get() || !stub_->decoder())
+  if (!delegate_)

[piman, 2016/11/09 22:49:36]

nit: I think you could DCHECK here, which means we always return true. And same
on the mac one. IOW, I think we might be able to remove a failure path.

[Fady Samuel, 2016/11/09 23:28:10]

done. I'll remove the bool in a separate CL.

     return false;
-  stub_->SetLatencyInfoCallback(
+  delegate_->SetLatencyInfoCallback(
       base::Bind(&PassThroughImageTransportSurface::SetLatencyInfo,
                  base::Unretained(this)));
   return true;
@@ -131,8 +130,8 @@ bool PassThroughImageTransportSurface::OnMakeCurrent(gl::GLContext* context) {
 }
 
 PassThroughImageTransportSurface::~PassThroughImageTransportSurface() {
-  if (stub_.get()) {
-    stub_->SetLatencyInfoCallback(
+  if (delegate_) {
+    delegate_->SetLatencyInfoCallback(
         base::Callback<void(const std::vector<ui::LatencyInfo>&)>());
   }
 }
@@ -146,8 +145,11 @@ void PassThroughImageTransportSurface::SetLatencyInfo(
 void PassThroughImageTransportSurface::SendVSyncUpdateIfAvailable() {
   gfx::VSyncProvider* vsync_provider = GetVSyncProvider();
   if (vsync_provider) {
-    vsync_provider->GetVSyncParameters(base::Bind(
-        &GpuCommandBufferStub::SendUpdateVSyncParameters, stub_->AsWeakPtr()));
+    // PassThroughImageTransportSurface owns the VSyncProvider and will
+    // outlive it. Thus, base::Unretained is safe here.
+    vsync_provider->GetVSyncParameters(
+        base::Bind(&PassThroughImageTransportSurface::UpdateVSyncParameters,
+                   base::Unretained(this)));

[piman, 2016/11/09 22:49:36]

nit: Any reason for the extra function, as opposed to simply
base::Bind(&ImageTransportSurfaceDelegate::UpdateVSyncParameters, delegate_) ?

[Fady Samuel, 2016/11/09 23:28:10]

Done.

   }
 }
 
@@ -180,10 +182,12 @@ void PassThroughImageTransportSurface::FinishSwapBuffers(
         swap_ack_time, 1);
   }
 
-  GpuCommandBufferMsg_SwapBuffersCompleted_Params params;
-  params.latency_info = *latency_info;
-  params.result = result;
-  stub_->SendSwapBuffersCompleted(params);
+  if (delegate_) {
+    SwapBuffersCompleteParams params;
+    params.latency_info = std::move(*latency_info);
+    params.result = result;
+    delegate_->DidSwapBuffersComplete(std::move(params));
+  }
 }
 
 void PassThroughImageTransportSurface::FinishSwapBuffersAsync(
@@ -194,4 +198,11 @@ void PassThroughImageTransportSurface::FinishSwapBuffersAsync(
   callback.Run(result);
 }
 
+void PassThroughImageTransportSurface::UpdateVSyncParameters(
+    base::TimeTicks timebase,
+    base::TimeDelta interval) {
+  if (delegate_)
+    delegate_->UpdateVSyncParameters(timebase, interval);
+}
+
 }  // namespace gpu