Allows OnAppTerminating at AshTestBase::TearDown().

Adding OnAppTerminating causes some failures. This CL also includes
the fixes for them.

OverlayEventFilterTest.CancelAtActivating:
- OverlayEventFilter::Cancel() is called, but that does not cleanup
  |delegate_|, therefore further OnAppTerminating() invokes Cancel()
  to already canceled delegate_.

PartialScreenshotViewTest.DontStartOverOverlay:
- delegate should be deactivated before it's removed.

RootWindowControllerTest.DontDeleteWindowsNotOwnedByParent:
- this test case explicitly invokes CloseAllChildWindows(), therefore
  it cleans up all of the containers.
- SessionStateAnimatorImpl assumed containers still exist when
  OnAppTerminating() is invoked.

WindowManagerTest.TestCursorClientObserver:
- observer_a is added to cursor_manager, but not destructed without
  calling RemoveObserver().

BUG=None
R=oshima@chromium.org
TEST=ash_unittests

Review URL: https://codereview.chromium.org/809603008

Cr-Commit-Position: refs/heads/master@{#308772}

4 files changed, 21 insertions, 11 deletions

diff --git a/ash/wm/overlay_event_filter.cc b/ash/wm/overlay_event_filter.cc
index 2e1c998..03c4947 100644
--- a/ash/wm/overlay_event_filter.cc
+++ b/ash/wm/overlay_event_filter.cc
@@ -63,16 +63,18 @@ void OverlayEventFilter::Activate(Delegate* delegate) {
 
 void OverlayEventFilter::Deactivate(Delegate* delegate) {
   if (delegate_ == delegate)
-    delegate_ = NULL;
+    delegate_ = nullptr;
 }
 
 void OverlayEventFilter::Cancel() {
-  if (delegate_)
+  if (delegate_) {
     delegate_->Cancel();
+    delegate_ = nullptr;
+  }
 }
 
 bool OverlayEventFilter::IsActive() {
-  return delegate_ != NULL;
+  return delegate_ != nullptr;
 }
 
 }  // namespace ash
diff --git a/ash/wm/partial_screenshot_view_unittest.cc b/ash/wm/partial_screenshot_view_unittest.cc
index 6a31968..7ad5205 100644
--- a/ash/wm/partial_screenshot_view_unittest.cc
+++ b/ash/wm/partial_screenshot_view_unittest.cc
@@ -117,6 +117,8 @@ TEST_F(PartialScreenshotViewTest, DontStartOverOverlay) {
   RunAllPendingInMessageLoop();
   EXPECT_EQ(&delegate, overlay_filter->delegate_);
   EXPECT_TRUE(view_ == NULL);
+
+  overlay_filter->Deactivate(&delegate);
 }
 
 }  // namespace ash
diff --git a/ash/wm/session_state_animator_impl.cc b/ash/wm/session_state_animator_impl.cc
index e606953..babe3cc 100644
--- a/ash/wm/session_state_animator_impl.cc
+++ b/ash/wm/session_state_animator_impl.cc
@@ -505,14 +505,13 @@ void SessionStateAnimatorImpl::GetContainers(int container_mask,
     // in such way.
     aura::Window* non_lock_screen_containers = Shell::GetContainer(
         root_window, kShellWindowId_NonLockScreenContainersContainer);
-    aura::Window::Windows children = non_lock_screen_containers->children();
-
-    for (aura::Window::Windows::const_iterator it = children.begin();
-         it != children.end(); ++it) {
-      aura::Window* window = *it;
-      if (window->id() == kShellWindowId_ShelfContainer)
-        continue;
-      containers->push_back(window);
+    // |non_lock_screen_containers| may already be removed in some tests.
+    if (non_lock_screen_containers) {
+      for (aura::Window* window : non_lock_screen_containers->children()) {
+        if (window->id() == kShellWindowId_ShelfContainer)
+          continue;
+        containers->push_back(window);
+      }
     }
   }
   if (container_mask & LOCK_SCREEN_BACKGROUND) {
@@ -527,6 +526,11 @@ void SessionStateAnimatorImpl::GetContainers(int container_mask,
     containers->push_back(Shell::GetContainer(
         root_window, kShellWindowId_LockScreenRelatedContainersContainer));
   }
+
+  // Some of containers may be null in some tests.
+  containers->erase(
+      std::remove(containers->begin(), containers->end(), nullptr),
+      containers->end());
 }
 
 void SessionStateAnimatorImpl::StartAnimation(int container_mask,
diff --git a/ash/wm/window_manager_unittest.cc b/ash/wm/window_manager_unittest.cc
index 6a51a0f..61aaaf2 100644
--- a/ash/wm/window_manager_unittest.cc
+++ b/ash/wm/window_manager_unittest.cc
@@ -832,6 +832,8 @@ TEST_F(WindowManagerTest, TestCursorClientObserver) {
   EXPECT_TRUE(observer_a.did_visibility_change());
   EXPECT_FALSE(observer_b.did_visibility_change());
   EXPECT_TRUE(observer_a.is_cursor_visible());
+
+  cursor_manager->RemoveObserver(&observer_a);
 }
 #endif  // defined(OS_CHROMEOS)