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

services/ui/ws/server_window_compositor_frame_sink_manager.cc

Index: services/ui/ws/server_window_compositor_frame_sink_manager.cc
diff --git a/services/ui/ws/server_window_compositor_frame_sink_manager.cc b/services/ui/ws/server_window_compositor_frame_sink_manager.cc
index 2b9b57223cd71efff366c0dd628cf7df333ceaeb..a78302ffb5fa680fa329b241ed9cb791c39be347 100644
--- a/services/ui/ws/server_window_compositor_frame_sink_manager.cc
+++ b/services/ui/ws/server_window_compositor_frame_sink_manager.cc
@@ -35,36 +35,22 @@ bool ServerWindowCompositorFrameSinkManager::ShouldDraw() {
   return !waiting_for_initial_frames_;
 }
 
-void ServerWindowCompositorFrameSinkManager::CreateCompositorFrameSink(
-    mojom::CompositorFrameSinkType compositor_frame_sink_type,
+void ServerWindowCompositorFrameSinkManager::CreateDisplayCompositorFrameSink(
     gfx::AcceleratedWidget widget,
     cc::mojom::MojoCompositorFrameSinkRequest request,
     cc::mojom::MojoCompositorFrameSinkClientPtr client) {
-  cc::FrameSinkId frame_sink_id(
-      WindowIdToTransportId(window_->id()),
-      static_cast<uint32_t>(compositor_frame_sink_type));
-  CompositorFrameSinkData& data =
-      type_to_compositor_frame_sink_map_[compositor_frame_sink_type];
-  cc::mojom::MojoCompositorFrameSinkPrivateRequest private_request;
-  if (data.pending_compositor_frame_sink_request.is_pending()) {
-    private_request = std::move(data.pending_compositor_frame_sink_request);
-  } else {
-    private_request = mojo::GetProxy(&data.compositor_frame_sink);
-  }
-
-  // TODO(fsamuel): AcceleratedWidget cannot be transported over IPC for Mac or
-  // Android. We should instead use GpuSurfaceTracker here on those platforms.
-  window_->delegate()->GetDisplayCompositor()->CreateCompositorFrameSink(
-      frame_sink_id, widget, std::move(request), std::move(private_request),
-      std::move(client));
+  CreateCompositorFrameSinkInternal(mojom::CompositorFrameSinkType::DEFAULT,
+                                    widget, std::move(request),
+                                    std::move(client));
+}
 
-  if (window_->parent()) {
-    window_->delegate()
-        ->GetRootWindow(window_)
-        ->GetOrCreateCompositorFrameSinkManager()
-        ->AddChildFrameSinkId(mojom::CompositorFrameSinkType::DEFAULT,
-                              frame_sink_id);
-  }
+void ServerWindowCompositorFrameSinkManager::CreateOffscreenCompositorFrameSink(
+    mojom::CompositorFrameSinkType compositor_frame_sink_type,
+    cc::mojom::MojoCompositorFrameSinkRequest request,
+    cc::mojom::MojoCompositorFrameSinkClientPtr client) {
+  CreateCompositorFrameSinkInternal(compositor_frame_sink_type,
+                                    gfx::kNullAcceleratedWidget,
+                                    std::move(request), std::move(client));
 }
 
 void ServerWindowCompositorFrameSinkManager::AddChildFrameSinkId(
@@ -142,6 +128,49 @@ bool ServerWindowCompositorFrameSinkManager::
          latest_submitted_frame_size.height() >= window_->bounds().height();
 }
 
+void ServerWindowCompositorFrameSinkManager::CreateCompositorFrameSinkInternal(
+    mojom::CompositorFrameSinkType compositor_frame_sink_type,
+    gfx::AcceleratedWidget widget,
+    cc::mojom::MojoCompositorFrameSinkRequest request,
+    cc::mojom::MojoCompositorFrameSinkClientPtr client) {
+  cc::FrameSinkId frame_sink_id(
+      WindowIdToTransportId(window_->id()),
+      static_cast<uint32_t>(compositor_frame_sink_type));
+  CompositorFrameSinkData& data =
+      type_to_compositor_frame_sink_map_[compositor_frame_sink_type];
+  cc::mojom::MojoCompositorFrameSinkPrivateRequest private_request;
+  if (data.pending_compositor_frame_sink_request.is_pending()) {
+    private_request = std::move(data.pending_compositor_frame_sink_request);
+  } else {
+    private_request = mojo::GetProxy(&data.compositor_frame_sink);
+  }
+
+  if (widget != gfx::kNullAcceleratedWidget) {

[msw, 2016/12/14 20:07:44]

Why does this conditional help? It looks like it could just always supply
|widget| regardless of whether the value is gfx::kNullAcceleratedWidget or not,
right?

[Fady Samuel, 2016/12/14 20:17:33]

Ohh I missed this. Thanks Mike We should be calling
CreateOffscreenCompositorFrameSink if widget is gfx::kNullAcceleratedWidget

[Alex Z., 2016/12/14 20:20:02]

The widget vs gfx::kNullAcceleratedWidget is the only difference, for now.

In the short term future, CreateDisplayCompositorFrameSink() would also take a
DisplayPrivate mojo interface pointer while CreateOffscreenCompositorFrameSink()
remains the same.

[Alex Z., 2016/12/14 20:26:21]

Done.

+    // TODO(fsamuel): AcceleratedWidget cannot be transported over IPC for Mac
+    // or Android. We should instead use GpuSurfaceTracker here on those
+    // platforms.
+    window_->delegate()
+        ->GetDisplayCompositor()
+        ->CreateDisplayCompositorFrameSink(
+            frame_sink_id, widget, std::move(request),
+            std::move(private_request), std::move(client));
+  } else {
+    window_->delegate()
+        ->GetDisplayCompositor()
+        ->CreateDisplayCompositorFrameSink(
+            frame_sink_id, gfx::kNullAcceleratedWidget, std::move(request),
+            std::move(private_request), std::move(client));
+  }
+
+  if (window_->parent()) {
+    window_->delegate()
+        ->GetRootWindow(window_)
+        ->GetOrCreateCompositorFrameSinkManager()
+        ->AddChildFrameSinkId(mojom::CompositorFrameSinkType::DEFAULT,
+                              frame_sink_id);
+  }
+}
+
 CompositorFrameSinkData::CompositorFrameSinkData() {}
 
 CompositorFrameSinkData::~CompositorFrameSinkData() {}