[Fuchsia] Make ScenicOverlayView re-usable with a new ScenicSurface

Previously ScenicOverlayView was attaching itself to ScenicSurface and
would never re-attach itself if ScenicSurface is replaced with a new
one. This CL makes ScenicSurface responsible for attaching
ScenicOverlayView when necessary. This ensures that ScenicOverlayView
can be re-used for new ScenicSurface instances.

(cherry picked from commit dfd84fc)

Bug: 1300385
Change-Id: Ie94e502246c6b6287b7ec915710489d441d9bf3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3487147
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#974953}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3490582
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/4692@{#1531}
Cr-Branched-From: 038cd96-refs/heads/main@{#938553}

ui/ozone/platform/scenic/overlay_manager_scenic.cc

@@ -13,21 +13,17 @@ namespace {
 
 class OverlayCandidatesScenic : public OverlayCandidatesOzone {
  public:
-  OverlayCandidatesScenic(gfx::AcceleratedWidget widget) : widget_(widget) {}
+  OverlayCandidatesScenic() = default;
 
   void CheckOverlaySupport(OverlaySurfaceCandidateList* candidates) override {
     for (auto& candidate : *candidates) {
       if (!candidate.native_pixmap)
         continue;
       SysmemNativePixmap* sysmem_native_pixmap =
           reinterpret_cast<SysmemNativePixmap*>(candidate.native_pixmap.get());
-      candidate.overlay_handled =
-          sysmem_native_pixmap->SupportsOverlayPlane(widget_);
+      candidate.overlay_handled = sysmem_native_pixmap->SupportsOverlayPlane();
     }
   }
-
- private:
-  const gfx::AcceleratedWidget widget_;
 };
 
 }  // namespace
@@ -41,7 +37,7 @@ OverlayManagerScenic::~OverlayManagerScenic() = default;
 
 std::unique_ptr<OverlayCandidatesOzone>
 OverlayManagerScenic::CreateOverlayCandidates(gfx::AcceleratedWidget widget) {
-  return std::make_unique<OverlayCandidatesScenic>(widget);
+  return std::make_unique<OverlayCandidatesScenic>();
 }
 
 }  // namespace ui

ui/ozone/platform/scenic/scenic_overlay_view.cc

@@ -5,7 +5,6 @@
 #include "ui/ozone/platform/scenic/scenic_overlay_view.h"
 
 #include <lib/ui/scenic/cpp/commands.h>
-#include <lib/ui/scenic/cpp/view_token_pair.h>
 
 #include "base/fuchsia/fuchsia_logging.h"
 #include "ui/ozone/platform/scenic/scenic_surface_factory.h"
@@ -19,24 +18,13 @@ namespace {
 static const uint32_t kImagePipeBufferCollectionId = 1;
 static const std::string kSessionDebugName = "chromium scenic overlay";
 
-fuchsia::ui::views::ViewToken CreateViewToken(
-    fuchsia::ui::views::ViewHolderToken* holder_token) {
-  auto token_pair = scenic::ViewTokenPair::New();
-  *holder_token = std::move(token_pair.view_holder_token);
-  return std::move(token_pair.view_token);
-}
-
 }  // namespace
 
 ScenicOverlayView::ScenicOverlayView(
-    scenic::SessionPtrAndListenerRequest session_and_listener_request,
-    ScenicSurfaceFactory* scenic_surface_factory)
+    scenic::SessionPtrAndListenerRequest session_and_listener_request)
     : scenic_session_(std::move(session_and_listener_request)),
       safe_presenter_(&scenic_session_),
-      scenic_surface_factory_(scenic_surface_factory),
-      view_(&scenic_session_,
-            CreateViewToken(&view_holder_token_),
-            kSessionDebugName) {
+      image_material_(&scenic_session_) {
   scenic_session_.SetDebugName(kSessionDebugName);
   scenic_session_.set_error_handler(
       base::LogFidlErrorAndExitProcess(FROM_HERE, "ScenicSession"));
@@ -45,12 +33,6 @@ ScenicOverlayView::ScenicOverlayView(
 ScenicOverlayView::~ScenicOverlayView() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  ScenicSurface* surface = scenic_surface_factory_->GetSurface(widget_);
-  if (surface) {
-    surface->AssertBelongsToCurrentThread();
-    surface->RemoveOverlayView(buffer_collection_id_);
-  }
-
   // Releasing |image_pipe_| implicitly also enforces cleanup.
   image_pipe_->RemoveBufferCollection(kImagePipeBufferCollectionId);
 }
@@ -66,15 +48,7 @@ void ScenicOverlayView::Initialize(
   image_pipe_.set_error_handler(
       base::LogFidlErrorAndExitProcess(FROM_HERE, "ImagePipe"));
 
-  image_material_ = std::make_unique<scenic::Material>(&scenic_session_);
-  image_material_->SetTexture(image_pipe_id);
-
-  scenic::ShapeNode shape(&scenic_session_);
-  shape.SetShape(scenic::Rectangle(&scenic_session_, 1.f, 1.f));
-  shape.SetMaterial(*image_material_);
-
-  view_.AddChild(shape);
-  scenic_session_.ReleaseResource(image_pipe_id);
+  image_material_.SetTexture(image_pipe_id);
   safe_presenter_.QueuePresent();
 
   // Since there is one ImagePipe for each BufferCollection, it is ok to use a
@@ -118,35 +92,22 @@ void ScenicOverlayView::SetBlendMode(bool enable_blend) {
   enable_blend_ = enable_blend;
   // Setting alpha as |255| marks the image as opaque and no content below would
   // be seen. Anything lower than 255 allows blending.
-  image_material_->SetColor(255, 255, 255, enable_blend ? 254 : 255);
+  image_material_.SetColor(255, 255, 255, enable_blend ? 254 : 255);
   safe_presenter_.QueuePresent();
 }
 
-bool ScenicOverlayView::CanAttachToAcceleratedWidget(
-    gfx::AcceleratedWidget widget) {
-  return view_holder_token_.value.is_valid() || (widget_ == widget);
-}
-
-bool ScenicOverlayView::AttachToScenicSurface(
-    gfx::AcceleratedWidget widget,
-    gfx::SysmemBufferCollectionId id) {
+void ScenicOverlayView::AttachToScenicSurface(
+    fuchsia::ui::views::ViewToken view_token) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  if (widget_ != gfx::kNullAcceleratedWidget && widget_ == widget)
-    return true;
-
-  if (!view_holder_token_.value.is_valid()) {
-    DLOG(ERROR) << "ViewHolder is already attached.";
-    return false;
-  }
+  view_.emplace(&scenic_session_, std::move(view_token), kSessionDebugName);
 
-  buffer_collection_id_ = id;
-  widget_ = widget;
+  scenic::ShapeNode shape(&scenic_session_);
+  shape.SetShape(scenic::Rectangle(&scenic_session_, 1.f, 1.f));
+  shape.SetMaterial(image_material_);
 
-  ScenicSurface* surface = scenic_surface_factory_->GetSurface(widget_);
-  DCHECK(surface);
-  return surface->PresentOverlayView(buffer_collection_id_,
-                                     std::move(view_holder_token_));
+  view_->AddChild(shape);
+  safe_presenter_.QueuePresent();
 }
 
 }  // namespace ui

ui/ozone/platform/scenic/scenic_overlay_view.h

@@ -8,7 +8,6 @@
 #include <fuchsia/ui/scenic/cpp/fidl.h>
 #include <lib/ui/scenic/cpp/resources.h>
 #include <lib/ui/scenic/cpp/session.h>
-#include <memory>
 
 #include "base/threading/thread_checker.h"
 #include "ui/gfx/geometry/size.h"
@@ -24,9 +23,8 @@ namespace ui {
 // instances to display overlays.
 class ScenicOverlayView {
  public:
-  ScenicOverlayView(
-      scenic::SessionPtrAndListenerRequest session_and_listener_request,
-      ScenicSurfaceFactory* scenic_surface_factory);
+  explicit ScenicOverlayView(
+      scenic::SessionPtrAndListenerRequest session_and_listener_request);
   ~ScenicOverlayView();
   ScenicOverlayView(const ScenicOverlayView&) = delete;
   ScenicOverlayView& operator=(const ScenicOverlayView&) = delete;
@@ -50,28 +48,18 @@ class ScenicOverlayView {
   // If |enable_blend| is true, sets |image_pipe_| as non-opaque.
   void SetBlendMode(bool enable_blend);
 
-  // Return true if |view_holder_token_| can be attached to a surface from
-  // |widget|.
-  bool CanAttachToAcceleratedWidget(gfx::AcceleratedWidget widget);
-
-  // Return true if |view_holder_token_| is attached to the scene graph of
-  // surface corresponding to |widget|.
-  bool AttachToScenicSurface(gfx::AcceleratedWidget widget,
-                             gfx::SysmemBufferCollectionId id);
+  // Attaches the view using the specified token.
+  void AttachToScenicSurface(fuchsia::ui::views::ViewToken view_token);
 
  private:
   scenic::Session scenic_session_;
   // Used for safely queueing Present() operations on |scenic_session_|.
   SafePresenter safe_presenter_;
-  ScenicSurfaceFactory* const scenic_surface_factory_;
-  fuchsia::ui::views::ViewHolderToken view_holder_token_;
-  scenic::View view_;
+  absl::optional<scenic::View> view_;
   fuchsia::images::ImagePipe2Ptr image_pipe_;
-  std::unique_ptr<scenic::Material> image_material_;
+  scenic::Material image_material_;
 
   bool enable_blend_ = false;
-  gfx::AcceleratedWidget widget_ = gfx::kNullAcceleratedWidget;
-  gfx::SysmemBufferCollectionId buffer_collection_id_;
 
   THREAD_CHECKER(thread_checker_);
 };

ui/ozone/platform/scenic/scenic_surface.cc

@@ -117,9 +117,16 @@ ScenicSurface::~ScenicSurface() {
   scenic_surface_factory_->RemoveSurface(window_);
 }
 
-ScenicSurface::OverlayViewInfo::OverlayViewInfo(scenic::ViewHolder holder,
-                                                scenic::EntityNode node)
-    : view_holder(std::move(holder)), entity_node(std::move(node)) {}
+ScenicSurface::OverlayViewInfo::OverlayViewInfo(
+    scenic::Session* scenic_session,
+    fuchsia::ui::views::ViewHolderToken view_holder_token)
+    : view_holder(scenic_session,
+                  std::move(view_holder_token),
+                  "OverlayViewHolder"),
+      entity_node(scenic_session) {
+  view_holder.SetHitTestBehavior(fuchsia::ui::gfx::HitTestBehavior::kSuppress);
+  entity_node.AddChild(view_holder);
+}
 
 void ScenicSurface::OnScenicEvents(
     std::vector<fuchsia::ui::scenic::Event> events) {
@@ -158,7 +165,7 @@ void ScenicSurface::Present(
     BufferPresentedCallback presentation_callback) {
   if (!image_pipe_) {
     std::move(completion_callback)
-        .Run(gfx::SwapCompletionResult(gfx::SwapResult::SWAP_FAILED));
+        .Run(gfx::SwapCompletionResult(gfx::SwapResult::SWAP_SKIPPED));
     return;
   }
 
@@ -183,22 +190,33 @@ void ScenicSurface::Present(
                                          DuplicateGpuFences(acquire_fences),
                                          /*release_fences=*/{});
 
-    auto& handle =
-        static_cast<SysmemNativePixmap*>(overlay.pixmap.get())->PeekHandle();
+    auto* pixmap = static_cast<SysmemNativePixmap*>(overlay.pixmap.get());
+    auto& handle = pixmap->PeekHandle();
     gfx::SysmemBufferCollectionId overlay_id =
         handle.buffer_collection_id.value();
     auto it = overlay_views_.find(overlay_id);
-    CHECK(it != overlay_views_.end());
+
+    // If this is a new overlay then attach it.
+    if (it == overlay_views_.end()) {
+      auto token_pair = scenic::ViewTokenPair::New();
+      pixmap->GetScenicOverlayView()->AttachToScenicSurface(
+          std::move(token_pair.view_token));
+      auto emplace_result = overlay_views_.emplace(
+          std::piecewise_construct, std::forward_as_tuple(overlay_id),
+          std::forward_as_tuple(&scenic_session_,
+                                std::move(token_pair.view_holder_token)));
+      it = emplace_result.first;
+      layout_update_required = true;
+    }
+
     auto& overlay_view_info = it->second;
     overlay_view_info.should_be_visible = true;
 
     auto& overlay_data = overlay.overlay_plane_data;
-    if (!overlay_view_info.visible ||
-        overlay_view_info.plane_z_order != overlay_data.z_order ||
+    if (overlay_view_info.plane_z_order != overlay_data.z_order ||
         overlay_view_info.display_bounds != overlay_data.display_bounds ||
         overlay_view_info.crop_rect != overlay_data.crop_rect ||
         overlay_view_info.plane_transform != overlay_data.plane_transform) {
-      overlay_view_info.visible = true;
       overlay_view_info.plane_z_order = overlay_data.z_order;
       overlay_view_info.display_bounds = overlay_data.display_bounds;
       overlay_view_info.crop_rect = overlay_data.crop_rect;
@@ -208,12 +226,17 @@ void ScenicSurface::Present(
   }
 
   // Hide all overlays views that are not in `overlays_to_present`.
-  for (auto it = overlay_views_.begin(); it != overlay_views_.end(); ++it) {
+  auto it = overlay_views_.begin();
+  while (it != overlay_views_.end()) {
     auto& overlay_view = it->second;
-    if (overlay_view.visible && !overlay_view.should_be_visible) {
-      overlay_view.visible = false;
+    if (!overlay_view.should_be_visible) {
       layout_update_required = true;
+      parent_->DetachChild(overlay_view.entity_node);
+      it = overlay_views_.erase(it);
+      continue;
     }
+
+    it++;
   }
 
   if (layout_update_required) {
@@ -345,36 +368,6 @@ void ScenicSurface::SetTextureToImage(const scenic::Image& image) {
   main_shape_.SetMaterial(main_material_);
 }
 
-bool ScenicSurface::PresentOverlayView(
-    gfx::SysmemBufferCollectionId id,
-    fuchsia::ui::views::ViewHolderToken view_holder_token) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  scenic::ViewHolder view_holder(&scenic_session_, std::move(view_holder_token),
-                                 "OverlayViewHolder");
-  scenic::EntityNode entity_node(&scenic_session_);
-  view_holder.SetHitTestBehavior(fuchsia::ui::gfx::HitTestBehavior::kSuppress);
-
-  entity_node.AddChild(view_holder);
-
-  DCHECK(!overlay_views_.count(id));
-  overlay_views_.emplace(
-      std::piecewise_construct, std::forward_as_tuple(id),
-      std::forward_as_tuple(std::move(view_holder), std::move(entity_node)));
-
-  return true;
-}
-
-bool ScenicSurface::RemoveOverlayView(gfx::SysmemBufferCollectionId id) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  auto it = overlay_views_.find(id);
-  DCHECK(it != overlay_views_.end());
-  parent_->DetachChild(it->second.entity_node);
-  safe_presenter_.QueuePresent();
-  overlay_views_.erase(it);
-  return true;
-}
-
 mojo::PlatformHandle ScenicSurface::CreateView() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
@@ -446,6 +439,10 @@ void ScenicSurface::OnPresentComplete(
 void ScenicSurface::UpdateViewHolderScene() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  // Layout will be updated once we receive view size.
+  if (main_shape_size_.IsEmpty())
+    return;
+
   // |plane_z_order| for main surface is 0.
   int min_z_order = 0;
   for (auto& item : overlay_views_) {
@@ -456,12 +453,6 @@ void ScenicSurface::UpdateViewHolderScene() {
   for (auto& item : overlay_views_) {
     auto& overlay_view = item.second;
 
-    if (!overlay_view.visible) {
-      // `Detach()` is a no-op if the node is not attached.
-      overlay_view.entity_node.Detach();
-      continue;
-    }
-
     // No-op if the node is already attached.
     parent_->AddChild(overlay_view.entity_node);
 

ui/ozone/platform/scenic/scenic_surface.h

@@ -77,15 +77,6 @@ class ScenicSurface : public PlatformWindowSurface {
   // Sets the texture of the surface to an image resource.
   void SetTextureToImage(const scenic::Image& image);
 
-  // Presents a ViewHolder that is corresponding to the overlay content coming
-  // from BufferCollection specified by |id|.
-  bool PresentOverlayView(
-      gfx::SysmemBufferCollectionId id,
-      fuchsia::ui::views::ViewHolderToken view_holder_token);
-
-  // Remove ViewHolder specified by |id|.
-  bool RemoveOverlayView(gfx::SysmemBufferCollectionId id);
-
   // Creates a View for this surface, and returns a ViewHolderToken handle
   // that can be used to attach it into a scene graph.
   mojo::PlatformHandle CreateView();
@@ -186,12 +177,12 @@ class ScenicSurface : public PlatformWindowSurface {
   const gfx::AcceleratedWidget window_;
 
   struct OverlayViewInfo {
-    OverlayViewInfo(scenic::ViewHolder holder, scenic::EntityNode node);
+    OverlayViewInfo(scenic::Session* scenic_session,
+                    fuchsia::ui::views::ViewHolderToken view_holder_token);
 
     scenic::ViewHolder view_holder;
     scenic::EntityNode entity_node;
 
-    bool visible = false;
     int plane_z_order = 0;
     gfx::Rect display_bounds;
     gfx::RectF crop_rect;

ui/ozone/platform/scenic/sysmem_buffer_collection.cc

@@ -153,7 +153,7 @@ bool SysmemBufferCollection::Initialize(
   if (register_with_image_pipe) {
     overlay_view_task_runner_ = base::ThreadTaskRunnerHandle::Get();
     scenic_overlay_view_ = std::make_unique<ScenicOverlayView>(
-        scenic_surface_factory->CreateScenicSession(), scenic_surface_factory);
+        scenic_surface_factory->CreateScenicSession());
   }
 
   fuchsia::sysmem::BufferCollectionTokenSyncPtr collection_token;