Implement aura::WindowPortMus::CreateCompositorFrameSink()

This CL implements aura::WindowPortMus::CreateCompositorFrameSink().
Exo will use it in Mushrome and Mustash to create a frame sink
and use it to submit frames.

BUG=720600

Review-Url: https://codereview.chromium.org/2875753002
Cr-Original-Commit-Position: refs/heads/master@{#478287}
Committed: https://chromium.googlesource.com/chromium/src/+/810bd6f44ea3be0e7ff4522190cf2e4924769e17
Review-Url: https://codereview.chromium.org/2875753002
Cr-Commit-Position: refs/heads/master@{#478355}

components/exo/surface.cc

@@ -185,10 +185,6 @@ class CustomWindowTargeter : public aura::WindowTargeter {
 ////////////////////////////////////////////////////////////////////////////////
 // Surface, public:
 
-// TODO(fsamuel): exo should not use context_factory_private. Instead, we should
-// request a CompositorFrameSink from the aura::Window. Setting up the
-// BeginFrame hierarchy should be an internal implementation detail of aura or
-// mus in aura-mus.
 Surface::Surface() : window_(new aura::Window(new CustomWindowDelegate(this))) {
   window_->SetType(aura::client::WINDOW_TYPE_CONTROL);
   window_->SetName("ExoSurface");
@@ -808,6 +804,10 @@ void Surface::UpdateSurface(bool full_damage) {
   }
 
   content_size_ = layer_size;
+  // We need update window_'s bounds with content size, because the
+  // CompositorFrameSink may not update the window's size base the size of
+  // the lastest submitted CompositorFrame.
+  window_->SetBounds(gfx::Rect(window_->bounds().origin(), content_size_));
   // TODO(jbauman): Figure out how this interacts with the pixel size of
   // CopyOutputRequests on the layer.
   gfx::Size contents_surface_size = layer_size;

components/viz/client/client_compositor_frame_sink.cc

@@ -32,7 +32,8 @@ ClientCompositorFrameSink::ClientCompositorFrameSink(
       compositor_frame_sink_info_(std::move(compositor_frame_sink_info)),
       client_request_(std::move(client_request)),
       client_binding_(this),
-      enable_surface_synchronization_(enable_surface_synchronization) {
+      enable_surface_synchronization_(enable_surface_synchronization),
+      weak_factory_(this) {
   DETACH_FROM_THREAD(thread_checker_);
 }
 
@@ -49,12 +50,19 @@ ClientCompositorFrameSink::ClientCompositorFrameSink(
       compositor_frame_sink_info_(std::move(compositor_frame_sink_info)),
       client_request_(std::move(client_request)),
       client_binding_(this),
-      enable_surface_synchronization_(enable_surface_synchronization) {
+      enable_surface_synchronization_(enable_surface_synchronization),
+      weak_factory_(this) {
   DETACH_FROM_THREAD(thread_checker_);
 }
 
 ClientCompositorFrameSink::~ClientCompositorFrameSink() {}
 
+base::WeakPtr<ClientCompositorFrameSink>
+ClientCompositorFrameSink::GetWeakPtr() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  return weak_factory_.GetWeakPtr();
+}
+
 bool ClientCompositorFrameSink::BindToClient(
     cc::CompositorFrameSinkClient* client) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
@@ -89,6 +97,7 @@ void ClientCompositorFrameSink::DetachFromClient() {
 
 void ClientCompositorFrameSink::SetLocalSurfaceId(
     const cc::LocalSurfaceId& local_surface_id) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(local_surface_id.is_valid());
   DCHECK(enable_surface_synchronization_);
   local_surface_id_ = local_surface_id;

components/viz/client/client_compositor_frame_sink.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_VIZ_CLIENT_CLIENT_COMPOSITOR_FRAME_SINK_H_
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "cc/ipc/mojo_compositor_frame_sink.mojom.h"
 #include "cc/output/compositor_frame_sink.h"
 #include "cc/output/context_provider.h"
@@ -46,6 +47,8 @@ class ClientCompositorFrameSink
 
   ~ClientCompositorFrameSink() override;
 
+  base::WeakPtr<ClientCompositorFrameSink> GetWeakPtr();
+
   // cc::CompositorFrameSink implementation.
   bool BindToClient(cc::CompositorFrameSinkClient* client) override;
   void DetachFromClient() override;
@@ -77,6 +80,8 @@ class ClientCompositorFrameSink
   THREAD_CHECKER(thread_checker_);
   const bool enable_surface_synchronization_;
 
+  base::WeakPtrFactory<ClientCompositorFrameSink> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ClientCompositorFrameSink);
 };
 

services/ui/ws/window_server.cc

@@ -882,10 +882,9 @@ void WindowServer::OnSurfaceCreated(const cc::SurfaceInfo& surface_info) {
 
   HandleTemporaryReferenceForNewSurface(surface_info.id(), window);
 
-  if (!window->parent())
-    return;
-
-  WindowTree* window_tree = GetTreeWithId(window->parent()->id().client_id);
+  // We always use the owner of the window's id (even for an embedded window),
+  // because an embedded window's id is allocated by the parent's window tree.
+  WindowTree* window_tree = GetTreeWithId(window->id().client_id);
   if (window_tree)
     window_tree->ProcessWindowSurfaceChanged(window, surface_info);
 }

services/ui/ws/window_tree.cc

@@ -963,14 +963,9 @@ void WindowTree::ProcessTransientWindowRemoved(
 void WindowTree::ProcessWindowSurfaceChanged(
     ServerWindow* window,
     const cc::SurfaceInfo& surface_info) {
-  ServerWindow* parent_window = window->parent();
-  ClientWindowId client_window_id, parent_client_window_id;
-  if (!IsWindowKnown(window, &client_window_id) ||
-      !IsWindowKnown(parent_window, &parent_client_window_id) ||
-      !created_window_map_.count(parent_window->id())) {
+  ClientWindowId client_window_id;
+  if (!IsWindowKnown(window, &client_window_id))
     return;
-  }
-
   client()->OnWindowSurfaceChanged(client_window_id.id, surface_info);
 }
 

services/ui/ws/window_tree_client_unittest.cc

@@ -2187,46 +2187,78 @@ TEST_F(WindowTreeClientTest, SurfaceIdPropagation) {
 
   // Establish the second client at 1,100.
   ASSERT_NO_FATAL_FAILURE(EstablishSecondClientWithRoot(window_1_100));
+  changes2()->clear();
 
   // 1,100 is the id in the wt_client1's id space. The new client should see
   // 2,1 (the server id).
   const Id window_1_100_in_ws2 = BuildWindowId(client_id_1(), 1);
   EXPECT_EQ(window_1_100_in_ws2, wt_client2()->root_window_id());
 
+  // Submit a CompositorFrame to window_1_100_in_ws2 (the embedded window in
+  // wt2) and make sure the server gets it.
+  {
+    cc::mojom::MojoCompositorFrameSinkPtr surface_ptr;
+    cc::mojom::MojoCompositorFrameSinkClientRequest client_request;
+    cc::mojom::MojoCompositorFrameSinkClientPtr surface_client_ptr;
+    client_request = mojo::MakeRequest(&surface_client_ptr);
+    wt2()->AttachCompositorFrameSink(window_1_100_in_ws2,
+                                     mojo::MakeRequest(&surface_ptr),
+                                     std::move(surface_client_ptr));
+    cc::CompositorFrame compositor_frame;
+    std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
+    gfx::Rect frame_rect(0, 0, 100, 100);
+    render_pass->SetNew(1, frame_rect, frame_rect, gfx::Transform());
+    compositor_frame.render_pass_list.push_back(std::move(render_pass));
+    compositor_frame.metadata.device_scale_factor = 1.f;
+    compositor_frame.metadata.begin_frame_ack =
+        cc::BeginFrameAck(0, 1, 1, true);
+    cc::LocalSurfaceId local_surface_id(1, base::UnguessableToken::Create());
+    surface_ptr->SubmitCompositorFrame(local_surface_id,
+                                       std::move(compositor_frame));
+  }
+  // Make sure the parent connection gets the surface ID.
+  wt_client1()->WaitForChangeCount(1);
+  // Verify that the submitted frame is for |window_2_101|.
+  EXPECT_EQ(window_1_100_in_ws2,
+            changes1()->back().surface_id.frame_sink_id().client_id());
+  changes1()->clear();
+
   // The first window created in the second client gets a server id of 2,1
   // regardless of the id the client uses.
   const Id window_2_101 = wt_client2()->NewWindow(101);
   ASSERT_TRUE(wt_client2()->AddWindow(window_1_100_in_ws2, window_2_101));
-  const Id window_2_101_in_ws1 = BuildWindowId(client_id_2(), 1);
+  const Id window_2_101_in_ws2 = BuildWindowId(client_id_2(), 1);
   wt_client1()->WaitForChangeCount(1);
-  EXPECT_EQ("HierarchyChanged window=" + IdToString(window_2_101_in_ws1) +
+  EXPECT_EQ("HierarchyChanged window=" + IdToString(window_2_101_in_ws2) +
                 " old_parent=null new_parent=" + IdToString(window_1_100),
             SingleChangeToDescription(*changes1()));
-  changes1()->clear();
-
-  // Submit a CompositorFrame to window_2_101 and make sure server gets it.
-  cc::mojom::MojoCompositorFrameSinkPtr surface_ptr;
-  cc::mojom::MojoCompositorFrameSinkClientRequest client_request;
-  cc::mojom::MojoCompositorFrameSinkClientPtr surface_client_ptr;
-  client_request = mojo::MakeRequest(&surface_client_ptr);
-  wt2()->AttachCompositorFrameSink(window_2_101,
-                                   mojo::MakeRequest(&surface_ptr),
-                                   std::move(surface_client_ptr));
-  cc::CompositorFrame compositor_frame;
-  std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
-  gfx::Rect frame_rect(0, 0, 100, 100);
-  render_pass->SetNew(1, frame_rect, frame_rect, gfx::Transform());
-  compositor_frame.render_pass_list.push_back(std::move(render_pass));
-  compositor_frame.metadata.device_scale_factor = 1.f;
-  compositor_frame.metadata.begin_frame_ack = cc::BeginFrameAck(0, 1, 1, true);
-  cc::LocalSurfaceId local_surface_id(1, base::UnguessableToken::Create());
-  surface_ptr->SubmitCompositorFrame(local_surface_id,
-                                     std::move(compositor_frame));
+  // Submit a CompositorFrame to window_2_101_in_ws2 (a regular window in
+  // wt2) and make sure client gets it.
+  {
+    cc::mojom::MojoCompositorFrameSinkPtr surface_ptr;
+    cc::mojom::MojoCompositorFrameSinkClientRequest client_request;
+    cc::mojom::MojoCompositorFrameSinkClientPtr surface_client_ptr;
+    client_request = mojo::MakeRequest(&surface_client_ptr);
+    wt2()->AttachCompositorFrameSink(window_2_101,
+                                     mojo::MakeRequest(&surface_ptr),
+                                     std::move(surface_client_ptr));
+    cc::CompositorFrame compositor_frame;
+    std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
+    gfx::Rect frame_rect(0, 0, 100, 100);
+    render_pass->SetNew(1, frame_rect, frame_rect, gfx::Transform());
+    compositor_frame.render_pass_list.push_back(std::move(render_pass));
+    compositor_frame.metadata.device_scale_factor = 1.f;
+    compositor_frame.metadata.begin_frame_ack =
+        cc::BeginFrameAck(0, 1, 1, true);
+    cc::LocalSurfaceId local_surface_id(2, base::UnguessableToken::Create());
+    surface_ptr->SubmitCompositorFrame(local_surface_id,
+                                       std::move(compositor_frame));
+  }
   // Make sure the parent connection gets the surface ID.
-  wt_client1()->WaitForChangeCount(1);
+  wt_client2()->WaitForChangeCount(1);
   // Verify that the submitted frame is for |window_2_101|.
-  EXPECT_EQ(window_2_101_in_ws1,
-            changes1()->back().surface_id.frame_sink_id().client_id());
+  EXPECT_EQ(window_2_101_in_ws2,
+            changes2()->back().surface_id.frame_sink_id().client_id());
 }
 
 // Verifies when an unknown window with a known child is added to a hierarchy

ui/aura/local/window_port_local.cc

@@ -129,11 +129,6 @@ void WindowPortLocal::OnSurfaceChanged(const cc::SurfaceId& surface_id,
                                        const gfx::Size& surface_size) {
   DCHECK_EQ(surface_id.frame_sink_id(), frame_sink_id_);
   local_surface_id_ = surface_id.local_surface_id();
-  // The bounds must be updated before switching to the new surface, because
-  // the layer may be mirrored, in which case a surface change causes the
-  // mirror layer to update its surface using the latest bounds.
-  window_->layer()->SetBounds(
-      gfx::Rect(window_->layer()->bounds().origin(), surface_size));
   cc::SurfaceInfo surface_info(surface_id, 1.0f, surface_size);
   scoped_refptr<cc::SurfaceReferenceFactory> reference_factory =
       aura::Env::GetInstance()

ui/aura/mus/window_mus.h

@@ -90,7 +90,6 @@ class AURA_EXPORT WindowMus {
       const cc::FrameSinkId& frame_sink_id) = 0;
   virtual const cc::LocalSurfaceId& GetOrAllocateLocalSurfaceId(
       const gfx::Size& new_size) = 0;
-  virtual void SetPrimarySurfaceInfo(const cc::SurfaceInfo& surface_info) = 0;
   virtual void SetFallbackSurfaceInfo(const cc::SurfaceInfo& surface_info) = 0;
   // The window was deleted on the server side. DestroyFromServer() should
   // result in deleting |this|.
@@ -128,6 +127,8 @@ class AURA_EXPORT WindowMus {
 
   virtual void NotifyEmbeddedAppDisconnected() = 0;
 
+  virtual bool HasLocalCompositorFrameSink() = 0;
+
  private:
   // Just for set_server_id(), which other places should not call.
   friend class WindowTreeClient;

ui/aura/mus/window_port_mus.cc

@@ -8,6 +8,7 @@
 #include "components/viz/client/local_surface_id_provider.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"
@@ -95,7 +96,7 @@ void WindowPortMus::Embed(
   window_tree_client_->Embed(window_, std::move(client), flags, callback);
 }
 
-std::unique_ptr<cc::CompositorFrameSink>
+std::unique_ptr<viz::ClientCompositorFrameSink>
 WindowPortMus::RequestCompositorFrameSink(
     scoped_refptr<cc::ContextProvider> context_provider,
     gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager) {
@@ -115,7 +116,7 @@ WindowPortMus::RequestCompositorFrameSink(
       enable_surface_synchronization);
   window_tree_client_->AttachCompositorFrameSink(
       server_id(), std::move(sink_request), std::move(client));
-  return std::move(compositor_frame_sink);
+  return compositor_frame_sink;
 }
 
 WindowPortMus::ServerChangeIdType WindowPortMus::ScheduleChange(
@@ -306,18 +307,25 @@ const cc::LocalSurfaceId& WindowPortMus::GetOrAllocateLocalSurfaceId(
   if (frame_sink_id_.is_valid())
     UpdatePrimarySurfaceInfo();
 
-  return local_surface_id_;
-}
+  if (local_compositor_frame_sink_)
+    local_compositor_frame_sink_->SetLocalSurfaceId(local_surface_id_);
 
-void WindowPortMus::SetPrimarySurfaceInfo(const cc::SurfaceInfo& surface_info) {
-  primary_surface_info_ = surface_info;
-  UpdateClientSurfaceEmbedder();
-  if (window_->delegate())
-    window_->delegate()->OnWindowSurfaceChanged(surface_info);
+  return local_surface_id_;
 }
 
 void WindowPortMus::SetFallbackSurfaceInfo(
     const cc::SurfaceInfo& surface_info) {
+  if (!frame_sink_id_.is_valid()) {
+    // |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.
+  DCHECK_EQ(surface_info.id().frame_sink_id(), frame_sink_id_);
+
   fallback_surface_info_ = surface_info;
   UpdateClientSurfaceEmbedder();
 }
@@ -426,6 +434,10 @@ void WindowPortMus::NotifyEmbeddedAppDisconnected() {
     observer.OnEmbeddedAppDisconnected(window_);
 }
 
+bool WindowPortMus::HasLocalCompositorFrameSink() {
+  return !!local_compositor_frame_sink_;
+}
+
 void WindowPortMus::OnPreInit(Window* window) {
   window_ = window;
   window_tree_client_->OnWindowMusCreated(this);
@@ -521,38 +533,46 @@ 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);
+  DCHECK(!local_compositor_frame_sink_);
+  auto frame_sink = RequestCompositorFrameSink(
+      nullptr,
+      aura::Env::GetInstance()->context_factory()->GetGpuMemoryBufferManager());
+  local_compositor_frame_sink_ = frame_sink->GetWeakPtr();
+  return std::move(frame_sink);
 }
 
 cc::SurfaceId WindowPortMus::GetSurfaceId() const {
-  // TODO(penghuang): Implement it for Mus.
+  // This is only used by WindowPortLocal in unit tests.
   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(cc::SurfaceInfo(
+  primary_surface_info_ = cc::SurfaceInfo(
       cc::SurfaceId(frame_sink_id_, local_surface_id_),
-      ScaleFactorForDisplay(window_), last_surface_size_in_pixels_));
+      ScaleFactorForDisplay(window_), last_surface_size_in_pixels_);
+  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)
+  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 (!client_surface_embedder_) {
     client_surface_embedder_ = base::MakeUnique<ClientSurfaceEmbedder>(

ui/aura/mus/window_port_mus.h

@@ -24,6 +24,10 @@
 #include "ui/gfx/geometry/rect.h"
 #include "ui/platform_window/mojo/text_input_state.mojom.h"
 
+namespace viz {
+class ClientCompositorFrameSink;
+}
+
 namespace aura {
 
 class ClientSurfaceEmbedder;
@@ -76,7 +80,7 @@ class AURA_EXPORT WindowPortMus : public WindowPort, public WindowMus {
              uint32_t flags,
              const ui::mojom::WindowTree::EmbedCallback& callback);
 
-  std::unique_ptr<cc::CompositorFrameSink> RequestCompositorFrameSink(
+  std::unique_ptr<viz::ClientCompositorFrameSink> RequestCompositorFrameSink(
       scoped_refptr<cc::ContextProvider> context_provider,
       gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager);
 
@@ -223,7 +227,6 @@ class AURA_EXPORT WindowPortMus : public WindowPort, public WindowMus {
   void SetFrameSinkIdFromServer(const cc::FrameSinkId& frame_sink_id) override;
   const cc::LocalSurfaceId& GetOrAllocateLocalSurfaceId(
       const gfx::Size& surface_size_in_pixels) override;
-  void SetPrimarySurfaceInfo(const cc::SurfaceInfo& surface_info) override;
   void SetFallbackSurfaceInfo(const cc::SurfaceInfo& surface_info) override;
   void DestroyFromServer() override;
   void AddTransientChildFromServer(WindowMus* child) override;
@@ -239,6 +242,7 @@ class AURA_EXPORT WindowPortMus : public WindowPort, public WindowMus {
   void PrepareForTransientRestack(WindowMus* window) override;
   void OnTransientRestackDone(WindowMus* window) override;
   void NotifyEmbeddedAppDisconnected() override;
+  bool HasLocalCompositorFrameSink() override;
 
   // WindowPort:
   void OnPreInit(Window* window) override;
@@ -285,6 +289,11 @@ class AURA_EXPORT WindowPortMus : public WindowPort, public WindowMus {
 
   ui::CursorData cursor_;
 
+  // When a frame sink is created
+  // for a local aura::Window, we need keep a weak ptr of it, so we can update
+  // the local surface id when necessary.
+  base::WeakPtr<viz::ClientCompositorFrameSink> local_compositor_frame_sink_;
+
   DISALLOW_COPY_AND_ASSIGN(WindowPortMus);
 };
 

ui/aura/mus/window_tree_client.cc

@@ -699,7 +699,8 @@ void WindowTreeClient::ScheduleInFlightBoundsChange(
   base::Optional<cc::LocalSurfaceId> local_surface_id;
   if (window->window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM ||
       window->window_mus_type() == WindowMusType::EMBED_IN_OWNER ||
-      window->window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED) {
+      window->window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED ||
+      window->HasLocalCompositorFrameSink()) {
     local_surface_id = window->GetOrAllocateLocalSurfaceId(new_bounds.size());
     synchronizing_with_child_on_next_frame_ = true;
   }