Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

gpu/ipc/service/pass_through_image_transport_surface.cc

 // Copyright 2016 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.
 
 #include "gpu/ipc/service/pass_through_image_transport_surface.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #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"
 
 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;
 }
 
 void PassThroughImageTransportSurface::Destroy() {
   GLSurfaceAdapter::Destroy();
 }
 
 gfx::SwapResult PassThroughImageTransportSurface::SwapBuffers() {
@@ skipping 81 matching lines @@
             switches::kDisableGpuVsync))
       context->ForceSwapIntervalZero(true);
     else
       context->SetSwapInterval(1);
     did_set_swap_interval_ = true;
   }
   return true;
 }
 
 PassThroughImageTransportSurface::~PassThroughImageTransportSurface() {
-  if (stub_.get()) {
-    stub_->SetLatencyInfoCallback(
+  if (delegate_) {
+    delegate_->SetLatencyInfoCallback(
         base::Callback<void(const std::vector<ui::LatencyInfo>&)>());
   }
 }
 
 void PassThroughImageTransportSurface::SetLatencyInfo(
     const std::vector<ui::LatencyInfo>& latency_info) {
   latency_info_.insert(latency_info_.end(), latency_info.begin(),
                        latency_info.end());
 }
 
 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.

   }
 }
 
 std::unique_ptr<std::vector<ui::LatencyInfo>>
 PassThroughImageTransportSurface::StartSwapBuffers() {
   // GetVsyncValues before SwapBuffers to work around Mali driver bug:
   // crbug.com/223558.
   SendVSyncUpdateIfAvailable();
 
   base::TimeTicks swap_time = base::TimeTicks::Now();
@@ skipping 12 matching lines @@
 void PassThroughImageTransportSurface::FinishSwapBuffers(
     std::unique_ptr<std::vector<ui::LatencyInfo>> latency_info,
     gfx::SwapResult result) {
   base::TimeTicks swap_ack_time = base::TimeTicks::Now();
   for (auto& latency : *latency_info) {
     latency.AddLatencyNumberWithTimestamp(
         ui::INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT, 0, 0,
         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(
     std::unique_ptr<std::vector<ui::LatencyInfo>> latency_info,
     GLSurface::SwapCompletionCallback callback,
     gfx::SwapResult result) {
   FinishSwapBuffers(std::move(latency_info), result);
   callback.Run(result);
 }
 
+void PassThroughImageTransportSurface::UpdateVSyncParameters(
+    base::TimeTicks timebase,
+    base::TimeDelta interval) {
+  if (delegate_)
+    delegate_->UpdateVSyncParameters(timebase, interval);
+}
+
 }  // namespace gpu