Support changing the window tree from aura::WindowDelegate::OnWindowOcclusionChanged.

Some code in Chrome needs to change the window tree in response
to a change of visibility for a WebContents (e.g. close zoom
bubble, create a new RenderWidgetHostViewAura). Because of that,
we need to allow changes to the window tree from
aura::WindowDelegate::OnWindowOcclusionChanged().

With this CL, aura::Window::SetOcclusionState() is not called
immediately when the occlusion of a tracked window is computed.
Instead, we compute occlusion for all tracked windows. Then,
we call aura::Window::SetOcclusionState() on all tracked
windows. Having separate phases ensures that the window tree is
not modified while occlusion is being computed. If a call to
aura::Window::SetOcclusionState() in the 2nd phase changes the
window tree in a way that could affect occlusion, we go back
to phase 1. There is a hard cap of 2 iterations, after which we
crash.

Bug: 668690, 813076
Change-Id: I61e5cdbb605343a5b587aeb96effcbd8dc07de33
Reviewed-on: https://chromium-review.googlesource.com/861953
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537310}

ui/aura/window_occlusion_tracker.h

@@ -9,6 +9,7 @@
 #include "base/containers/flat_set.h"
 #include "base/macros.h"
 #include "ui/aura/aura_export.h"
+#include "ui/aura/window.h"
 #include "ui/aura/window_observer.h"
 #include "ui/compositor/layer_animation_observer.h"
 
@@ -21,8 +22,6 @@ class Transform;
 
 namespace aura {
 
-class Window;
-
 // Notifies tracked Windows when their occlusion state change.
 //
 // To start tracking the occlusion state of a Window, call
@@ -55,13 +54,22 @@ class AURA_EXPORT WindowOcclusionTracker : public ui::LayerAnimationObserver,
   static void Track(Window* window);
 
  private:
+  struct RootWindowState {
+    // Number of Windows whose occlusion state is tracked under this root
+    // Window.
+    int num_tracked_windows = 0;
+
+    // Whether the occlusion state of tracked Windows under this root is stale.
+    bool dirty = false;
+  };
+
   WindowOcclusionTracker();
   ~WindowOcclusionTracker() override;
 
   // Recomputes the occlusion state of tracked windows under roots marked as
   // dirty in |root_windows_| if there are no active
   // ScopedPauseOcclusionTracking instance.
-  void MaybeRecomputeOcclusion();
+  void MaybeComputeOcclusion();
 
   // Recomputes the occlusion state of |window| and its descendants.
   // |parent_transform_relative_to_root| is the transform of |window->parent()|
@@ -87,8 +95,8 @@ class AURA_EXPORT WindowOcclusionTracker : public ui::LayerAnimationObserver,
   // are in |tracked_windows_|.
   void SetWindowAndDescendantsAreOccluded(Window* window, bool is_occluded);
 
-  // Calls SetOccluded() on |window| with |occluded| as argument if |window| is
-  // in |tracked_windows_|.
+  // Updates the occlusion state of |window| in |tracked_windows_|. No-op if
+  // |window| is not in |tracked_windows_|.
   void SetOccluded(Window* window, bool occluded);
 
   // Returns true if |window| is in |tracked_windows_|.
@@ -98,12 +106,15 @@ class AURA_EXPORT WindowOcclusionTracker : public ui::LayerAnimationObserver,
   bool WindowIsAnimated(Window* window) const;
 
   // If the root of |window| is not dirty and |predicate| is true, marks the
-  // root of |window| as dirty. Then, calls MaybeRecomputeOcclusion().
+  // root of |window| as dirty. Then, calls MaybeComputeOcclusion().
   // |predicate| is not evaluated if the root of |window| is already dirty when
   // this is called.
   template <typename Predicate>
-  void MarkRootWindowAsDirtyAndMaybeRecomputeOcclusionIf(Window* window,
-                                                         Predicate predicate);
+  void MarkRootWindowAsDirtyAndMaybeComputeOcclusionIf(Window* window,
+                                                       Predicate predicate);
+
+  // Marks |root_window| as dirty.
+  void MarkRootWindowAsDirty(RootWindowState* root_window_state);
 
   // Returns true if |window| or one of its parents is in |animated_windows_|.
   bool WindowOrParentIsAnimated(Window* window) const;
@@ -163,17 +174,8 @@ class AURA_EXPORT WindowOcclusionTracker : public ui::LayerAnimationObserver,
                                       Window* new_root) override;
   void OnWindowLayerRecreated(Window* window) override;
 
-  struct RootWindowState {
-    // Number of Windows whose occlusion state is tracked under this root
-    // Window.
-    int num_tracked_windows = 0;
-
-    // Whether the occlusion state of tracked Windows under this root is stale.
-    bool dirty = false;
-  };
-
   // Windows whose occlusion state is tracked.
-  base::flat_set<Window*> tracked_windows_;
+  base::flat_map<Window*, Window::OcclusionState> tracked_windows_;
 
   // Windows whose bounds or transform are animated.
   //
@@ -187,6 +189,10 @@ class AURA_EXPORT WindowOcclusionTracker : public ui::LayerAnimationObserver,
   // Root Windows of Windows in |tracked_windows_|.
   base::flat_map<Window*, RootWindowState> root_windows_;
 
+  // Number of times that the current call to MaybeComputeOcclusion() has
+  // recomputed occlusion states. Always 0 when not in MaybeComputeOcclusion().
+  int num_times_occlusion_recomputed_ = 0;
+
   DISALLOW_COPY_AND_ASSIGN(WindowOcclusionTracker);
 };
 

ui/aura/window_occlusion_tracker_unittest.cc

@@ -1321,4 +1321,191 @@ TEST_F(WindowOcclusionTrackerTest,
   window->layer()->GetAnimator()->StopAnimating();
 }
 
+namespace {
+
+class WindowDelegateHidingWindowIfOccluded : public MockWindowDelegate {
+ public:
+  WindowDelegateHidingWindowIfOccluded(Window* other_window,
+                                       MockWindowDelegate* other_delegate)
+      : other_window_(other_window), other_delegate_(other_delegate) {}
+
+  // MockWindowDelegate:
+  void OnWindowOcclusionChanged(bool is_occluded) override {
+    MockWindowDelegate::OnWindowOcclusionChanged(is_occluded);
+    if (is_occluded) {
+      other_window_->Hide();
+      other_delegate_->set_expectation(
+          WindowOcclusionChangedExpectation::OCCLUDED);
+    }
+  }
+
+ private:
+  Window* other_window_;
+  MockWindowDelegate* other_delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(WindowDelegateHidingWindowIfOccluded);
+};
+
+}  // namespace
+
+// Verify that a window delegate can change the visibility of another window
+// when it is notified that its occlusion changed.
+TEST_F(WindowOcclusionTrackerTest, HideFromOnWindowOcclusionChanged) {
+  // Create a tracked window. Expect it to be visible.
+  MockWindowDelegate* delegate_a = new MockWindowDelegate();
+  delegate_a->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window_a = CreateTrackedWindow(delegate_a, gfx::Rect(0, 0, 10, 10));
+  EXPECT_FALSE(delegate_a->is_expecting_call());
+
+  // Create a tracked window. Expect it to be visible.
+  MockWindowDelegate* delegate_b =
+      new WindowDelegateHidingWindowIfOccluded(window_a, delegate_a);
+  delegate_b->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window_b = CreateTrackedWindow(delegate_b, gfx::Rect(5, 5, 10, 10));
+  EXPECT_FALSE(delegate_b->is_expecting_call());
+
+  // Hide the tracked window. It should be able to hide |window_a|.
+  delegate_b->set_expectation(WindowOcclusionChangedExpectation::OCCLUDED);
+  window_b->Hide();
+  EXPECT_FALSE(delegate_a->is_expecting_call());
+  EXPECT_FALSE(delegate_b->is_expecting_call());
+  EXPECT_FALSE(window_a->IsVisible());
+  EXPECT_FALSE(window_b->IsVisible());
+}
+
+namespace {
+
+class WindowDelegateDeletingWindow : public MockWindowDelegate {
+ public:
+  WindowDelegateDeletingWindow() = default;
+
+  void set_other_window(Window* other_window) { other_window_ = other_window; }
+
+  // MockWindowDelegate:
+  void OnWindowOcclusionChanged(bool is_occluded) override {
+    MockWindowDelegate::OnWindowOcclusionChanged(is_occluded);
+    if (is_occluded) {
+      delete other_window_;
+      other_window_ = nullptr;
+    }
+  }
+
+ private:
+  Window* other_window_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(WindowDelegateDeletingWindow);
+};
+
+}  // namespace
+
+// Verify that a window can delete a window that is on top of it when it is
+// notified that its occlusion changed (a crash would occur if
+// WindowOcclusionTracker accessed that window after it was deleted).
+TEST_F(WindowOcclusionTrackerTest, DeleteFromOnWindowOcclusionChanged) {
+  // Create a tracked window. Expect it to be visible.
+  WindowDelegateDeletingWindow* delegate_a = new WindowDelegateDeletingWindow();
+  delegate_a->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window_a = CreateTrackedWindow(delegate_a, gfx::Rect(0, 0, 10, 10));
+  EXPECT_FALSE(delegate_a->is_expecting_call());
+
+  // Create a tracked window. Expect it to be visible.
+  MockWindowDelegate* delegate_b = new MockWindowDelegate();
+  delegate_b->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window_b = CreateTrackedWindow(delegate_b, gfx::Rect(10, 0, 10, 10));
+  EXPECT_FALSE(delegate_b->is_expecting_call());
+
+  // Create a tracked window. Expect it to be visible.
+  MockWindowDelegate* delegate_c = new MockWindowDelegate();
+  delegate_c->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window_c = CreateTrackedWindow(delegate_c, gfx::Rect(20, 0, 10, 10));
+  EXPECT_FALSE(delegate_c->is_expecting_call());
+
+  // |window_c| will be deleted when |window_a| is occluded.
+  delegate_a->set_other_window(window_c);
+
+  // Move |window_b| on top of |window_a|.
+  delegate_a->set_expectation(WindowOcclusionChangedExpectation::OCCLUDED);
+  window_b->SetBounds(window_a->bounds());
+  EXPECT_FALSE(delegate_a->is_expecting_call());
+}
+
+namespace {
+
+class WindowDelegateChangingWindowVisibility : public MockWindowDelegate {
+ public:
+  WindowDelegateChangingWindowVisibility() = default;
+
+  void set_window_to_update(Window* window) { window_to_update_ = window; }
+
+  // MockWindowDelegate:
+  void OnWindowOcclusionChanged(bool is_occluded) override {
+    MockWindowDelegate::OnWindowOcclusionChanged(is_occluded);
+    if (!window_to_update_)
+      return;
+
+    if (window_to_update_->IsVisible()) {
+      window_to_update_->Hide();
+    } else {
+      window_to_update_->Show();
+      EXPECT_FALSE(did_set_expectation_from_occlusion_changed_);
+      set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+      did_set_expectation_from_occlusion_changed_ = true;
+    }
+  }
+
+ private:
+  Window* window_to_update_ = nullptr;
+  bool did_set_expectation_from_occlusion_changed_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(WindowDelegateChangingWindowVisibility);
+};
+}  // namespace
+
+// Verify that if a window changes its visibility every time it is notified that
+// its occlusion state changed, the occlusion state of all IsVisible() windows
+// is set to NOT_OCCLUDED and no infinite loop is entered.
+TEST_F(WindowOcclusionTrackerTest, OcclusionStatesDontBecomeStable) {
+  // Create 2 superposed tracked windows.
+  MockWindowDelegate* delegate_a = new MockWindowDelegate();
+  delegate_a->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  CreateTrackedWindow(delegate_a, gfx::Rect(0, 0, 10, 10));
+  EXPECT_FALSE(delegate_a->is_expecting_call());
+
+  MockWindowDelegate* delegate_b = new MockWindowDelegate();
+  delegate_a->set_expectation(WindowOcclusionChangedExpectation::OCCLUDED);
+  delegate_b->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  CreateTrackedWindow(delegate_b, gfx::Rect(0, 0, 10, 10));
+  EXPECT_FALSE(delegate_a->is_expecting_call());
+  EXPECT_FALSE(delegate_b->is_expecting_call());
+
+  // Create a hidden tracked window.
+  MockWindowDelegate* delegate_c = new MockWindowDelegate();
+  delegate_c->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window_c = CreateTrackedWindow(delegate_c, gfx::Rect(10, 0, 10, 10));
+  EXPECT_FALSE(delegate_c->is_expecting_call());
+  delegate_c->set_expectation(WindowOcclusionChangedExpectation::OCCLUDED);
+  window_c->Hide();
+  EXPECT_FALSE(delegate_c->is_expecting_call());
+
+  // Create a tracked window. Expect it to be non-occluded.
+  auto* delegate_d = new WindowDelegateChangingWindowVisibility();
+  delegate_d->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window_d = CreateTrackedWindow(delegate_d, gfx::Rect(20, 0, 10, 10));
+  EXPECT_FALSE(delegate_d->is_expecting_call());
+
+  // Store a pointer to |window_d| in |delegate_d|. This will cause a call to
+  // Show()/Hide() every time |delegate_d| is notified of an occlusion change.
+  delegate_d->set_window_to_update(window_d);
+
+  // Hide |window_d|. This will cause occlusion to be recomputed multiple times.
+  // Once the maximum number of times that occlusion can be recomputed is
+  // reached, the occlusion state of all IsVisible() windows should be set to
+  // NOT_OCCLUDED.
+  delegate_a->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  delegate_d->set_expectation(WindowOcclusionChangedExpectation::OCCLUDED);
+  window_d->Hide();
+  EXPECT_FALSE(delegate_a->is_expecting_call());
+  EXPECT_FALSE(delegate_d->is_expecting_call());
+}
+
 }  // namespace aura