Implement aura::WindowPortMus::CreateCompositorFrameSink()

ui/aura/mus/window_port_mus.cc

Index: ui/aura/mus/window_port_mus.cc
diff --git a/ui/aura/mus/window_port_mus.cc b/ui/aura/mus/window_port_mus.cc
index b946380ee01b382faa7ee45716b51182c7f698b6..f77e54dc9d53e34c4e2702cc97276a89355434f1 100644
--- a/ui/aura/mus/window_port_mus.cc
+++ b/ui/aura/mus/window_port_mus.cc
@@ -7,6 +7,7 @@
 #include "base/memory/ptr_util.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/transient_window_client.h"
+#include "ui/aura/env.h"
 #include "ui/aura/mus/client_surface_embedder.h"
 #include "ui/aura/mus/property_converter.h"
 #include "ui/aura/mus/window_tree_client.h"
@@ -496,12 +497,28 @@ void WindowPortMus::OnPropertyChanged(const void* key,
 
 std::unique_ptr<cc::CompositorFrameSink>
 WindowPortMus::CreateCompositorFrameSink() {

[Fady Samuel, 2017/05/18 20:02:02]

This is super confusing. We have a RequestCompositorFrameSink, and a
CreateCompositorFrameSink. Could we simply use RequestCompositorFrameSink with
two optional parameters?

[Peng, 2017/05/18 20:51:00]

Done

-  // TODO(penghuang): Implement it for Mus.
-  return nullptr;
+  // We only allow creating a compositor frame sink for a local window.
+  if (window_mus_type() != WindowMusType::LOCAL)
+    return nullptr;
+
+  ui::ContextFactory* context_factory =
+      aura::Env::GetInstance()->context_factory();
+  std::unique_ptr<ui::ClientCompositorFrameSinkBinding> frame_sink_binding;
+  // The compositor frame sink is for a local window, so we don't need surface
+  // synchronization.
+  const bool enable_surface_synchronization = false;

[Fady Samuel, 2017/05/18 20:02:02]

Surface synchronization is actually a requirement here. Each CompositorFrameSink
is a separate messagepipe but you want to update all of them atomically, so this
is incorrect.

[Peng, 2017/05/18 20:50:59]

As my understanding, when the the CompositorFrameSink allocates a new local
surface id, and sends a frame with the id, the viz will notify ws a new surface
is created, and then WindowTreeClient::OnWindowSurfaceChanged() will be called,
and then we will use the new surface id for submitting up level compositor
frame. So I don't see why we need enable surface synchronization. thought?

[Fady Samuel, 2017/05/18 20:56:22]

Without surface synchronization, there is no guarantee that all the
CompositorFrames from all the exo subsurfaces will be received and aggregated in
time for the next display frame.

[Peng, 2017/05/23 18:00:42]

Done.

+  std::unique_ptr<ui::ClientCompositorFrameSink> frame_sink =
+      ui::ClientCompositorFrameSink::Create(
+          nullptr /* context_provider */,
+          context_factory->GetGpuMemoryBufferManager(), &frame_sink_binding,
+          enable_surface_synchronization);
+  AttachCompositorFrameSink(std::move(frame_sink_binding));
+  return std::move(frame_sink);
 }
 
 cc::SurfaceId WindowPortMus::GetSurfaceId() const {
-  // TODO(penghuang): Implement it for Mus.
+  // This method is only used by exo unittests which are not running against
+  // mus, so don't implement it now.
   return cc::SurfaceId();
 }
 
@@ -520,11 +537,6 @@ void WindowPortMus::UpdatePrimarySurfaceInfo() {
 }
 
 void WindowPortMus::UpdateClientSurfaceEmbedder() {
-  bool embeds_surface = window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM ||
-                        window_mus_type() == WindowMusType::EMBED_IN_OWNER;
-  if (!embeds_surface)
-    return;
-
   if (!client_surface_embedder_) {
     client_surface_embedder_ = base::MakeUnique<ClientSurfaceEmbedder>(
         window_, window_tree_client_->normal_client_area_insets_);