Add is_tracking_occlusion to the surface state

Fixes issue where occlusion tracking could start sending
updates before the surface state had been committed. In
ARC, this meant that occlusion updates were sent before
the surface bounds were first set, leading to incorrect
"full occlusion (1.0f) updates".

Bug: b:149229321
Test: exo_unittests
Change-Id: Ia885a44cf38658cc82d87273f70395c8bc2fcf72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212921
Commit-Queue: Jorge Gil <jorgegil@google.com>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Eliot Courtney <edcourtney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771850}

components/exo/surface.cc

@@ -610,7 +610,8 @@ void Surface::CommitSurfaceHierarchy(bool synchronized) {
             state_.only_visible_on_secure_output ||
         pending_state_.blend_mode != state_.blend_mode ||
         pending_state_.alpha != state_.alpha ||
-        pending_state_.color_space != state_.color_space;
+        pending_state_.color_space != state_.color_space ||
+        pending_state_.is_tracking_occlusion != state_.is_tracking_occlusion;
 
     bool needs_update_buffer_transform =
         pending_state_.buffer_scale != state_.buffer_scale ||
@@ -638,6 +639,13 @@ void Surface::CommitSurfaceHierarchy(bool synchronized) {
             ? aura::EventTargetingPolicy::kDescendantsOnly
             : aura::EventTargetingPolicy::kTargetAndDescendants);
 
+    if (state_.is_tracking_occlusion) {
+      // TODO(edcourtney): Currently, it doesn't seem to be possible to stop
+      // tracking the occlusion state once started, but it would be nice to stop
+      // if the tracked occlusion region becomes empty.
+      window_->TrackOcclusionState();
+    }
+
 #if defined(OS_CHROMEOS)
     if (needs_output_protection) {
       if (!output_protection_) {
@@ -859,12 +867,11 @@ bool Surface::FillsBoundsOpaquely() const {
 }
 
 void Surface::SetOcclusionTracking(bool tracking) {
-  is_tracking_occlusion_ = tracking;
-  // TODO(edcourtney): Currently, it doesn't seem to be possible to stop
-  // tracking the occlusion state once started, but it would be nice to stop if
-  // the tracked occlusion region becomes empty.
-  if (is_tracking_occlusion_)
-    window()->TrackOcclusionState();
+  pending_state_.is_tracking_occlusion = tracking;
+}
+
+bool Surface::IsTrackingOcclusion() {
+  return state_.is_tracking_occlusion;
 }
 
 void Surface::SetSurfaceHierarchyContentBoundsForTest(
@@ -1157,7 +1164,7 @@ void Surface::UpdateContentSize() {
 }
 
 void Surface::OnWindowOcclusionChanged() {
-  if (!is_tracking_occlusion_)
+  if (!state_.is_tracking_occlusion)
     return;
 
   for (SurfaceObserver& observer : observers_)

components/exo/surface.h

@@ -281,7 +281,7 @@ class Surface final : public ui::PropertyHandler {
   void OnWindowOcclusionChanged();
 
   // True if the window for this surface has its occlusion tracked.
-  bool is_tracking_occlusion() const { return is_tracking_occlusion_; }
+  bool IsTrackingOcclusion();
 
   // Sets the |surface_hierarchy_content_bounds_|.
   void SetSurfaceHierarchyContentBoundsForTest(const gfx::Rect& content_bounds);
@@ -309,6 +309,7 @@ class Surface final : public ui::PropertyHandler {
     float alpha = 1.0f;
     gfx::Vector2d offset;
     gfx::ColorSpace color_space;
+    bool is_tracking_occlusion = false;
   };
   class BufferAttachment {
    public:
@@ -451,9 +452,6 @@ class Surface final : public ui::PropertyHandler {
   // Surface observer list. Surface does not own the observers.
   base::ObserverList<SurfaceObserver, true>::Unchecked observers_;
 
-  // Whether this surface is tracking occlusion for the client.
-  bool is_tracking_occlusion_ = false;
-
 #if defined(OS_CHROMEOS)
   std::unique_ptr<ash::OutputProtectionDelegate> output_protection_;
 #endif  // defined(OS_CHROMEOS)

components/exo/surface_unittest.cc

@@ -1102,11 +1102,11 @@ TEST_P(SurfaceTest, UpdatesOcclusionOnDestroyingSubsurface) {
   auto sub_surface =
       std::make_unique<SubSurface>(child_surface.get(), surface.get());
   child_surface->Attach(child_buffer.get());
+  // Turn on occlusion tracking.
+  child_surface->SetOcclusionTracking(true);
   child_surface->Commit();
   surface->Commit();
 
-  // Turn on occlusion tracking.
-  child_surface->SetOcclusionTracking(true);
   SurfaceObserverForTest observer;
   ScopedSurface scoped_child_surface(child_surface.get(), &observer);
 

components/exo/wayland/zaura_shell.cc

@@ -224,7 +224,7 @@ void AuraSurface::OnSurfaceDestroying(Surface* surface) {
 }
 
 void AuraSurface::OnWindowOcclusionChanged(Surface* surface) {
-  if (!surface_ || !surface_->is_tracking_occlusion())
+  if (!surface_ || !surface_->IsTrackingOcclusion())
     return;
   auto* window = surface_->window();
   ComputeAndSendOcclusionFraction(window->occlusion_state(),
@@ -241,7 +241,7 @@ void AuraSurface::OnWindowActivating(ActivationReason reason,
   // Check if this surface is a child of a window that is losing focus.
   auto* widget = views::Widget::GetTopLevelWidgetForNativeView(window);
   if (!widget || losing_active != widget->GetNativeWindow() ||
-      !surface_->is_tracking_occlusion())
+      !surface_->IsTrackingOcclusion())
     return;
 
   // Result may be changed by animated windows, so compute it explicitly.

components/exo/wayland/zaura_shell_unittest.cc

@@ -11,6 +11,7 @@
 #include "ash/wm/desks/desks_util.h"
 #include "ash/wm/window_util.h"
 #include "base/time/time.h"
+#include "components/exo/buffer.h"
 #include "components/exo/test/exo_test_base.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/window_occlusion_tracker.h"
@@ -67,7 +68,13 @@ class ZAuraSurfaceTest : public test::ExoTestBase,
   void SetUp() override {
     test::ExoTestBase::SetUp();
 
+    gfx::Size buffer_size(10, 10);
+    std::unique_ptr<Buffer> buffer(
+        new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
+
     surface_.reset(new Surface);
+    surface_->Attach(buffer.get());
+
     aura_surface_.reset(new TestAuraSurface(surface_.get()));
 
     gfx::Transform transform;
@@ -128,8 +135,25 @@ class ZAuraSurfaceTest : public test::ExoTestBase,
   DISALLOW_COPY_AND_ASSIGN(ZAuraSurfaceTest);
 };
 
+TEST_F(ZAuraSurfaceTest, OcclusionTrackingStartsAfterCommit) {
+  surface().OnWindowOcclusionChanged();
+
+  EXPECT_EQ(-1.0f, aura_surface().last_sent_occlusion_fraction());
+  EXPECT_EQ(0, aura_surface().num_occlusion_updates());
+  EXPECT_FALSE(surface().IsTrackingOcclusion());
+
+  auto widget = CreateOpaqueWidget(gfx::Rect(0, 0, 10, 10));
+  widget->Show();
+  surface().Commit();
+
+  EXPECT_EQ(0.2f, aura_surface().last_sent_occlusion_fraction());
+  EXPECT_EQ(1, aura_surface().num_occlusion_updates());
+  EXPECT_TRUE(surface().IsTrackingOcclusion());
+}
+
 TEST_F(ZAuraSurfaceTest,
        LosingActivationWithNoAnimatingWindowsSendsCorrectOcclusionFraction) {
+  surface().Commit();
   EXPECT_EQ(0.0f, aura_surface().last_sent_occlusion_fraction());
   EXPECT_EQ(1, aura_surface().num_occlusion_updates());
   ::wm::ActivateWindow(parent_widget().GetNativeWindow());
@@ -146,6 +170,7 @@ TEST_F(ZAuraSurfaceTest,
 
 TEST_F(ZAuraSurfaceTest,
        LosingActivationWithAnimatingWindowsSendsTargetOcclusionFraction) {
+  surface().Commit();
   EXPECT_EQ(0.0f, aura_surface().last_sent_occlusion_fraction());
   EXPECT_EQ(1, aura_surface().num_occlusion_updates());
   ::wm::ActivateWindow(parent_widget().GetNativeWindow());
@@ -194,6 +219,7 @@ TEST_F(ZAuraSurfaceTest,
 
 TEST_F(ZAuraSurfaceTest,
        LosingActivationByTriggeringTheLockScreenDoesNotSendOccludedFraction) {
+  surface().Commit();
   EXPECT_EQ(0.0f, aura_surface().last_sent_occlusion_fraction());
   EXPECT_EQ(1, aura_surface().num_occlusion_updates());
   ::wm::ActivateWindow(parent_widget().GetNativeWindow());
@@ -227,9 +253,16 @@ TEST_F(ZAuraSurfaceTest,
 
 TEST_F(ZAuraSurfaceTest, OcclusionIncludesOffScreenArea) {
   UpdateDisplay("150x150");
+
+  gfx::Size buffer_size(80, 100);
+  std::unique_ptr<Buffer> buffer(
+      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
   // This is scaled by 1.5 - set the bounds to (-60, 75, 120, 150) in screen
   // coordinates so 75% of it is outside of the 100x100 screen.
   surface().window()->SetBounds(gfx::Rect(-40, 50, 80, 100));
+  surface().Attach(buffer.get());
+  surface().Commit();
+
   surface().OnWindowOcclusionChanged();
 
   EXPECT_EQ(0.75f, aura_surface().last_sent_occlusion_fraction());
@@ -238,6 +271,7 @@ TEST_F(ZAuraSurfaceTest, OcclusionIncludesOffScreenArea) {
 TEST_F(ZAuraSurfaceTest, ZeroSizeWindowSendsZeroOcclusionFraction) {
   // Zero sized window should not be occluded.
   surface().window()->SetBounds(gfx::Rect(0, 0, 0, 0));
+  surface().Commit();
   surface().OnWindowOcclusionChanged();
   EXPECT_EQ(0.0f, aura_surface().last_sent_occlusion_fraction());
 }