Support changing a window with tracked occlusion while it's added to a root.

Previously, a DCHECK failure occurred if a WindowObserver or a
LayoutManager changed a tracked window after Window::AddChild() added
it to a root but before WindowOcclusionTracker::OnWindowAddedToRootWindow
was notified. With this CL, the DCHECK is replaced with an early return.
We rely on WindowOcclusionTracker::OnWindowAddedToRootWindow to
mark the new root as dirty shortly after the early return.

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

ui/aura/window_occlusion_tracker.cc

@@ -240,7 +240,16 @@ void WindowOcclusionTracker::MarkRootWindowAsDirtyAndMaybeRecomputeOcclusionIf(
   if (!root_window)
     return;
   auto root_window_state_it = root_windows_.find(root_window);
-  DCHECK(root_window_state_it != root_windows_.end());
+
+  // This may be called if a WindowObserver or a LayoutManager changes |window|
+  // after Window::AddChild() has added it to a new root but before
+  // OnWindowAddedToRootWindow() is called on |this|. In that case, do nothing
+  // here and rely on OnWindowAddedToRootWindow() to mark the new root as dirty.
+  if (root_window_state_it == root_windows_.end()) {
+    DCHECK(WindowIsTracked(window));
+    return;
+  }
+
   if (root_window_state_it->second.dirty)
     return;
   if (predicate()) {

ui/aura/window_occlusion_tracker_unittest.cc

@@ -1118,4 +1118,46 @@ TEST_F(WindowOcclusionTrackerTest, RecreateLayerOfAnimatedWindow) {
   window_a->layer()->SetAnimator(nullptr);
 }
 
+namespace {
+
+class ObserverChangingWindowBounds : public WindowObserver {
+ public:
+  ObserverChangingWindowBounds() = default;
+
+  // WindowObserver:
+  void OnWindowParentChanged(Window* window, Window* parent) override {
+    window->SetBounds(gfx::Rect(1, 2, 3, 4));
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ObserverChangingWindowBounds);
+};
+
+}  // namespace
+
+// Verify that no crash occurs if a tracked window is modified by an observer
+// after it has been added to a new root but before WindowOcclusionTracker has
+// been notified.
+TEST_F(WindowOcclusionTrackerTest, ChangeTrackedWindowBeforeObserveAddToRoot) {
+  // Create a window. Expect it to be non-occluded.
+  MockWindowDelegate* delegate = new MockWindowDelegate();
+  delegate->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window = CreateTrackedWindow(delegate, gfx::Rect(0, 0, 10, 10));
+  EXPECT_FALSE(delegate->is_expecting_call());
+
+  // Remove the window from its root.
+  root_window()->RemoveChild(window);
+
+  // Add an observer that changes the bounds of |window| when it gets a new
+  // parent.
+  ObserverChangingWindowBounds observer;
+  window->AddObserver(&observer);
+
+  // Re-add the window to its root. Expect no crash when |observer| changes the
+  // bounds.
+  root_window()->AddChild(window);
+
+  window->RemoveObserver(&observer);
+}
+
 }  // namespace aura