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 14ae20a368daca4d74ce4909e70545b361551f90..67a161df8a1050cb1d92a5c2083e6ce92d6a3ba6 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"
@@ -41,7 +42,9 @@ WindowMus* WindowMus::Get(Window* window) {
 
 WindowPortMus::WindowPortMus(WindowTreeClient* client,
                              WindowMusType window_mus_type)
-    : WindowMus(window_mus_type), window_tree_client_(client) {}
+    : WindowMus(window_mus_type),
+      window_tree_client_(client),
+      weak_factory_(this) {}
 
 WindowPortMus::~WindowPortMus() {
   client_surface_embedder_.reset();
@@ -94,7 +97,7 @@ void WindowPortMus::Embed(
 }
 
 std::unique_ptr<cc::CompositorFrameSink>
-WindowPortMus::RequestCompositorFrameSink(
+WindowPortMus::CreateCompositorFrameSinkWithContextAndBufferManager(
     scoped_refptr<cc::ContextProvider> context_provider,
     gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager) {
   cc::mojom::MojoCompositorFrameSinkPtrInfo sink_info;
@@ -103,11 +106,13 @@ WindowPortMus::RequestCompositorFrameSink(
   cc::mojom::MojoCompositorFrameSinkClientPtr client;
   cc::mojom::MojoCompositorFrameSinkClientRequest client_request =
       mojo::MakeRequest(&client);
-  constexpr bool enable_surface_synchronization = true;
+  // For a "local window", we need ClientCompositorFrameSink to generate local
+  // surface id.
+  bool generate_local_surface_id = window_mus_type() == WindowMusType::LOCAL;

[Fady Samuel, 2017/05/30 18:36:42]

The parent should allocate the LocalSurfaceId. Let's not do this?

[sky, 2017/05/30 19:37:29]

One comment here. Currently the client that initiates a bounds change on a
window does *not* get OnWindowBoundsChanged(). We shouldn't change that as it'll
mean the client will start getting unexpected OnWindowBoundsChanged(). The only
way the client could deal with that is if we added extra information to
OnWindowBoundsChanged() so that the client could know it initiated the change. I
think this would effectively mean we need a OnWindowBoundsChangeCompleted()
function is called in this case instead of OnChangeCompleted().

[Peng, 2017/05/30 19:46:28]

For a local window, actually we don't have the concept parent and child. 
I think you mean we should generate local surface id in WindowPortMus instead of
ClientCompositorFrameSink, right?

[Peng, 2017/05/31 20:12:25]

Done. By Allocating local surface id in WindowPortMus.

   auto compositor_frame_sink = base::MakeUnique<viz::ClientCompositorFrameSink>(
       std::move(context_provider), gpu_memory_buffer_manager,
       std::move(sink_info), std::move(client_request),
-      enable_surface_synchronization);
+      generate_local_surface_id);
   window_tree_client_->AttachCompositorFrameSink(
       server_id(), std::move(sink_request), std::move(client));
   return std::move(compositor_frame_sink);
@@ -289,15 +294,26 @@ const cc::LocalSurfaceId& WindowPortMus::GetOrAllocateLocalSurfaceId(
   return local_surface_id_;
 }
 
-void WindowPortMus::SetPrimarySurfaceInfo(const cc::SurfaceInfo& surface_info) {

[Fady Samuel, 2017/05/30 20:35:30]

Please don't delete this either.

-  primary_surface_info_ = surface_info;
-  UpdateClientSurfaceEmbedder();
-  if (window_->delegate())
-    window_->delegate()->OnWindowSurfaceChanged(surface_info);
-}
-
-void WindowPortMus::SetFallbackSurfaceInfo(
+void WindowPortMus::SetSurfaceInfoFromServer(

[Fady Samuel, 2017/05/30 18:36:41]

This doesn't match our discussion. Let's keep the code path for local and
embedded windows the same.

[Peng, 2017/05/30 19:46:28]

I thought to generate in ClientCompositorFrameSink may not work with surface
synchronization. But after looking it carefully, I think it should work fine. So
I thought it is better to avoid the extra OnWindowBoundsChanged IPC, and with
the extra IPC, the exo cannot submit new frames immediately after resizing, it
has to want the extra IPC. It makes code difficult to write. So why not make
code simpler and more efficient? 

[Fady Samuel, 2017/05/30 19:51:31]

This introduces a special case for local windows so I'd argue this is
introducing more complexity. Let's have fewer code paths. 

     const cc::SurfaceInfo& surface_info) {
+  if (!frame_sink_id_.is_valid()) {
+    // This only happens for a "local" window, because the
+    // |SetFrameSinkIdFromServer| will not be called for a "local" window.
+    DCHECK_EQ(window_mus_type(), WindowMusType::LOCAL);
+    // |primary_surface_info_| shold not be valid, since we didn't know the
+    // |frame_sink_id_|.
+    DCHECK(!primary_surface_info_.is_valid());
+    frame_sink_id_ = surface_info.id().frame_sink_id();
+    UpdatePrimarySurfaceInfo();
+  }
+
+  // The frame sink id should never be changed.
+  DCECHK_EQ(surface_info.id().frame_sink_id(), frame_sink_id_);
+
+  // If the window is informed of a surface from server then that surface ID is
+  // guaranteed to be available in the display compositor so we set it as the
+  // fallback. For embedded window, the primary SurfaceInfo is created by the
+  // embedder, and the LocalSurfaceId is allocated by the embedder.
   fallback_surface_info_ = surface_info;
   UpdateClientSurfaceEmbedder();
 }
@@ -491,39 +507,43 @@ void WindowPortMus::OnPropertyChanged(const void* key,
 
 std::unique_ptr<cc::CompositorFrameSink>
 WindowPortMus::CreateCompositorFrameSink() {
-  // TODO(penghuang): Implement it for Mus.
-  return nullptr;
+  DCHECK_EQ(window_mus_type(), WindowMusType::LOCAL);
+  auto frame_sink = CreateCompositorFrameSinkWithContextAndBufferManager(
+      nullptr,
+      aura::Env::GetInstance()->context_factory()->GetGpuMemoryBufferManager());
+  auto* viz_frame_sink =
+      static_cast<viz::ClientCompositorFrameSink*>(frame_sink.get());
+  viz_frame_sink->SetSurfaceChangedCallback(
+      base::Bind(&WindowPortMus::OnSurfaceChangedForLocalWindow,
+                 weak_factory_.GetWeakPtr()));
+  return 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();
 }
 
 void WindowPortMus::UpdatePrimarySurfaceInfo() {
-  bool embeds_surface =
-      window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM ||
-      window_mus_type() == WindowMusType::EMBED_IN_OWNER ||
-      window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED;
-  if (!embeds_surface)
+  if (window_mus_type() != WindowMusType::TOP_LEVEL_IN_WM &&
+      window_mus_type() != WindowMusType::EMBED_IN_OWNER &&
+      window_mus_type() != WindowMusType::DISPLAY_MANUALLY_CREATED &&
+      window_mus_type() != WindowMusType::LOCAL)
     return;
 
   if (!frame_sink_id_.is_valid() || !local_surface_id_.is_valid())
     return;
 
-  SetPrimarySurfaceInfo(
+  primary_surface_info_ =
       cc::SurfaceInfo(cc::SurfaceId(frame_sink_id_, local_surface_id_),
-                      ScaleFactorForDisplay(window_), last_surface_size_));
+                      ScaleFactorForDisplay(window_), last_surface_size_);
+  UpdateClientSurfaceEmbedder();
+  if (window_->delegate())
+    window_->delegate()->OnWindowSurfaceChanged(primary_surface_info_);
 }
 
 void WindowPortMus::UpdateClientSurfaceEmbedder() {
-  bool embeds_surface =
-      window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM ||
-      window_mus_type() == WindowMusType::EMBED_IN_OWNER ||
-      window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED;
-  if (!embeds_surface)
-    return;
-
   if (!client_surface_embedder_) {
     client_surface_embedder_ = base::MakeUnique<ClientSurfaceEmbedder>(
         window_, window_tree_client_->normal_client_area_insets_);
@@ -533,4 +553,18 @@ void WindowPortMus::UpdateClientSurfaceEmbedder() {
   client_surface_embedder_->SetFallbackSurfaceInfo(fallback_surface_info_);
 }
 
+void WindowPortMus::OnSurfaceChangedForLocalWindow(

[Fady Samuel, 2017/05/30 18:36:42]

Please don't special case local windows?

[Peng, 2017/05/31 20:12:25]

Done. By allocating local_surface_id in GetOrAllocateLocalSurfaceId().

+    const cc::LocalSurfaceId& local_surface_id,
+    const gfx::Size& size) {
+  DCHECK_EQ(window_mus_type(), WindowMusType::LOCAL);
+  local_surface_id_ = local_surface_id;
+  last_surface_size_ = size;
+
+  // For the first frame, the |frame_sink_id_| will be null, and then the
+  // |UpdatePrimarySurfaceInfo| will not update the |primary_surface_info_|,
+  // instead the |primary_surface_info_| will be updated until we receive the
+  // surface info via |SetSurfaceInfoFromServer|.
+  UpdatePrimarySurfaceInfo();
+}
+
 }  // namespace aura