Split cc::mojom::DisplayCompositor::CreateCompositorFrameSink() into CreateDisplayCompositorFrameSi…

services/ui/surfaces/display_compositor.cc

Index: services/ui/surfaces/display_compositor.cc
diff --git a/services/ui/surfaces/display_compositor.cc b/services/ui/surfaces/display_compositor.cc
index ffa022361709f1b6c81c83281b93532cbf8dbe79..dd99ea6ca77eadaae570877fb8da94d5c427d2b4 100644
--- a/services/ui/surfaces/display_compositor.cc
+++ b/services/ui/surfaces/display_compositor.cc
@@ -92,31 +92,32 @@ void DisplayCompositor::OnCompositorFrameSinkPrivateConnectionLost(
     compositor_frame_sinks_.erase(frame_sink_id);
 }
 
-void DisplayCompositor::CreateCompositorFrameSink(
+void DisplayCompositor::CreateDisplayCompositorFrameSink(
     const cc::FrameSinkId& frame_sink_id,
     gpu::SurfaceHandle surface_handle,
     cc::mojom::MojoCompositorFrameSinkRequest request,
     cc::mojom::MojoCompositorFrameSinkPrivateRequest private_request,
     cc::mojom::MojoCompositorFrameSinkClientPtr client) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // We cannot create more than one CompositorFrameSink with a given
-  // |frame_sink_id|.
-  DCHECK_EQ(0u, compositor_frame_sinks_.count(frame_sink_id));
-
-  std::unique_ptr<cc::Display> display;
-  std::unique_ptr<cc::SyntheticBeginFrameSource> begin_frame_source;
-  if (surface_handle != gpu::kNullSurfaceHandle) {
-    begin_frame_source.reset(new cc::DelayBasedBeginFrameSource(
-        base::MakeUnique<cc::DelayBasedTimeSource>(task_runner_.get())));
-    display =
-        CreateDisplay(frame_sink_id, surface_handle, begin_frame_source.get());
-  }
+  DCHECK_NE(surface_handle, gpu::kNullSurfaceHandle);
+  std::unique_ptr<cc::SyntheticBeginFrameSource> begin_frame_source(
+      new cc::DelayBasedBeginFrameSource(
+          base::MakeUnique<cc::DelayBasedTimeSource>(task_runner_.get())));
+  CreateCompositorFrameSinkInternal(
+      frame_sink_id, surface_handle,
+      CreateDisplay(frame_sink_id, surface_handle, begin_frame_source.get()),

[Fady Samuel, 2016/12/15 14:43:44]

store this in a local variable above to make sure this param is evaluated first.
The problem is Win64 build is evaluating std::move(begin_frame_source) before it
evaluates this statement and so begin_frame_source.get() is nullptr.

[Alex Z., 2016/12/15 15:17:51]

Done.

+      std::move(begin_frame_source), std::move(request),
+      std::move(private_request), std::move(client));
+}
 
-  compositor_frame_sinks_[frame_sink_id] =
-      base::MakeUnique<GpuCompositorFrameSink>(
-          this, frame_sink_id, std::move(display),
-          std::move(begin_frame_source), std::move(request),
-          std::move(private_request), std::move(client));
+void DisplayCompositor::CreateOffscreenCompositorFrameSink(
+    const cc::FrameSinkId& frame_sink_id,
+    cc::mojom::MojoCompositorFrameSinkRequest request,
+    cc::mojom::MojoCompositorFrameSinkPrivateRequest private_request,
+    cc::mojom::MojoCompositorFrameSinkClientPtr client) {
+  CreateCompositorFrameSinkInternal(
+      frame_sink_id, gpu::kNullSurfaceHandle, nullptr, nullptr,
+      std::move(request), std::move(private_request), std::move(client));
 }
 
 void DisplayCompositor::AddSurfaceReference(const cc::SurfaceReference& ref) {
@@ -218,6 +219,26 @@ std::unique_ptr<cc::Display> DisplayCompositor::CreateDisplay(
       base::MakeUnique<cc::TextureMailboxDeleter>(task_runner_.get()));
 }
 
+void DisplayCompositor::CreateCompositorFrameSinkInternal(
+    const cc::FrameSinkId& frame_sink_id,
+    gpu::SurfaceHandle surface_handle,
+    std::unique_ptr<cc::Display> display,
+    std::unique_ptr<cc::SyntheticBeginFrameSource> begin_frame_source,
+    cc::mojom::MojoCompositorFrameSinkRequest request,
+    cc::mojom::MojoCompositorFrameSinkPrivateRequest private_request,
+    cc::mojom::MojoCompositorFrameSinkClientPtr client) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // We cannot create more than one CompositorFrameSink with a given
+  // |frame_sink_id|.
+  DCHECK_EQ(0u, compositor_frame_sinks_.count(frame_sink_id));
+
+  compositor_frame_sinks_[frame_sink_id] =
+      base::MakeUnique<GpuCompositorFrameSink>(
+          this, frame_sink_id, std::move(display),
+          std::move(begin_frame_source), std::move(request),
+          std::move(private_request), std::move(client));
+}
+
 const cc::SurfaceId& DisplayCompositor::GetRootSurfaceId() const {
   return reference_manager_->GetRootSurfaceId();
 }