Hide Surface windows before removing them from the hierarchy.

This makes sure that the HIDDEN occlusion update is delivered when
hiding the window, not when showing it later.

Subsurface additions are processed at any time (not only commit), which
means hidden windows can get added to the hierarchy. Their occlusion
will be computed as hidden until the next CommitSurfaceHierarchy where
they are made visible. This change moves that HIDDEN occlusion update to
the time of hiding, rather than the time of showing.

sent upon entering PIP, not leaving.

Bug: 140563748
Test: Alt-tab to trigger PIP on Youtube. Occlusion fraction of 1.0 is
Change-Id: I9a4a8f74bdaa0a55a65a6152b9df1232bd0e6e5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792041
Commit-Queue: Eliot Courtney <edcourtney@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695077}

components/exo/surface.cc

@@ -368,9 +368,9 @@ void Surface::RemoveSubSurface(Surface* sub_surface) {
   TRACE_EVENT1("exo", "Surface::RemoveSubSurface", "sub_surface",
                sub_surface->AsTracedValue());
 
-  window_->RemoveChild(sub_surface->window());
   if (sub_surface->window()->IsVisible())
     sub_surface->window()->Hide();
+  window_->RemoveChild(sub_surface->window());
 
   DCHECK(ListContainsEntry(pending_sub_surfaces_, sub_surface));
   pending_sub_surfaces_.erase(

components/exo/surface_unittest.cc

@@ -42,6 +42,24 @@ std::unique_ptr<std::vector<gfx::Rect>> GetHitTestShapeRects(Surface* surface) {
   return rects;
 }
 
+class SurfaceObserverForTest : public SurfaceObserver {
+ public:
+  SurfaceObserverForTest() = default;
+
+  void OnSurfaceDestroying(Surface* surface) override {}
+
+  void OnWindowOcclusionChanged(Surface* surface) override {
+    num_occlusion_changes_++;
+  }
+
+  int num_occlusion_changes() const { return num_occlusion_changes_; }
+
+ private:
+  int num_occlusion_changes_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(SurfaceObserverForTest);
+};
+
 class SurfaceTest : public test::ExoTestBase,
                     public ::testing::WithParamInterface<float> {
  public:
@@ -945,5 +963,36 @@ TEST_P(SurfaceTest, AcquireFence) {
   EXPECT_FALSE(surface->HasPendingAcquireFence());
 }
 
+TEST_P(SurfaceTest, UpdatesOcclusionOnDestroyingSubsurface) {
+  gfx::Size buffer_size(256, 512);
+  auto buffer = std::make_unique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = std::make_unique<Surface>();
+  auto shell_surface = std::make_unique<ShellSurface>(surface.get());
+  surface->Attach(buffer.get());
+  surface->Commit();
+
+  gfx::Size child_buffer_size(64, 128);
+  auto child_buffer = std::make_unique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(child_buffer_size));
+  auto child_surface = std::make_unique<Surface>();
+  auto sub_surface =
+      std::make_unique<SubSurface>(child_surface.get(), surface.get());
+  child_surface->Attach(child_buffer.get());
+  child_surface->Commit();
+  surface->Commit();
+
+  // Turn on occlusion tracking.
+  child_surface->SetOcclusionTracking(true);
+  SurfaceObserverForTest observer;
+  ScopedSurface scoped_child_surface(child_surface.get(), &observer);
+
+  // Destroy the subsurface and expect to get an occlusion update.
+  sub_surface.reset();
+  EXPECT_EQ(1, observer.num_occlusion_changes());
+  EXPECT_EQ(aura::Window::OcclusionState::HIDDEN,
+            child_surface->window()->occlusion_state());
+}
+
 }  // namespace
 }  // namespace exo