Implement aura::WindowPortMus::CreateCompositorFrameSink()

ui/aura/mus/window_port_mus.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/aura/mus/window_port_mus.h"
 
 #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"
 #include "ui/aura/mus/window_tree_client_delegate.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_delegate.h"
 #include "ui/aura/window_observer.h"
 #include "ui/base/class_property.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
@@ skipping 14 matching lines @@
 
 WindowPortMus::WindowMusChangeDataImpl::~WindowMusChangeDataImpl() = default;
 
 // static
 WindowMus* WindowMus::Get(Window* window) {
   return WindowPortMus::Get(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();
 
   // DESTROY is only scheduled from DestroyFromServer(), meaning if DESTROY is
   // present then the server originated the change.
   const WindowTreeClient::Origin origin =
       RemoveChangeByTypeAndData(ServerChangeType::DESTROY, ServerChangeData())
           ? WindowTreeClient::Origin::SERVER
           : WindowTreeClient::Origin::CLIENT;
@@ skipping 32 matching lines @@
 }
 
 void WindowPortMus::Embed(
     ui::mojom::WindowTreeClientPtr client,
     uint32_t flags,
     const ui::mojom::WindowTree::EmbedCallback& callback) {
   window_tree_client_->Embed(window_, std::move(client), flags, callback);
 }
 
 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;
   cc::mojom::MojoCompositorFrameSinkRequest sink_request =
       mojo::MakeRequest(&sink_info);
   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);
 }
 
 WindowPortMus::ServerChangeIdType WindowPortMus::ScheduleChange(
     const ServerChangeType type,
     const ServerChangeData& data) {
   ServerChange change;
   change.type = type;
@@ skipping 161 matching lines @@
   // If the FrameSinkId is available, then immediately embed the SurfaceId.
   // The newly generated frame by the embedder will block in the display
   // compositor until the child submits a corresponding CompositorFrame or a
   // deadline hits.
   if (frame_sink_id_.is_valid())
     UpdatePrimarySurfaceInfo();
 
   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::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();
+  }
 
-void WindowPortMus::SetFallbackSurfaceInfo(
-    const cc::SurfaceInfo& surface_info) {
+  // 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();
 }
 
 void WindowPortMus::DestroyFromServer() {
   std::unique_ptr<ScopedServerChange> remove_from_parent_change;
   if (window_->parent()) {
     ServerChangeData data;
     data.child_id = server_id();
     WindowPortMus* parent = Get(window_->parent());
@@ skipping 173 matching lines @@
   // TODO(sky): investigate to see if we need to compare data. In particular do
   // we ever have a case where changing a property cascades into changing the
   // same property?
   if (!RemoveChangeByTypeAndData(ServerChangeType::PROPERTY, change_data))
     window_tree_client_->OnWindowMusPropertyChanged(this, key, old_value,
                                                     std::move(data));
 }
 
 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_);
   }
 
   client_surface_embedder_->SetPrimarySurfaceInfo(primary_surface_info_);
   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