Check that child window was not destroyed inside UpdateTransientChildVisibility.

ui/wm/core/transient_window_manager.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/wm/core/transient_window_manager.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/auto_reset.h"
@@ skipping 144 matching lines @@
   }
 }
 
 void TransientWindowManager::OnWindowVisibilityChanged(Window* window,
                                                        bool visible) {
   if (window_ != window)
     return;
 
   // If the window has transient children, updates the transient children's
   // visiblity as well.
-  for (Window* child : transient_children_)
-    Get(child)->UpdateTransientChildVisibility(visible);
+  Windows transient_children = transient_children_;
+  for (Window* child : transient_children) {
+    // Child window can be removed inside UpdateTransientChildVisibility call.

[sky, 2015/11/20 15:56:57]

Your description says some of the children can be destroyed. If that's the case,
I don't like checking for pointer values to determine validity.

[oshima, 2015/11/20 18:39:18]

Sorry if I'm missing something, but the window lifetime is observed explicitly
by OnWindowDestroying, so it looks to me that it's checking if it has been
destroyed (like WindowTracker)?

[sky, 2015/11/20 18:53:44]

The copy of the windows (transient_children) is referencing a deleted window.
The change assumes a window with the same value hasn't been created.

[oshima, 2015/11/20 19:07:26]

If a new window is created at the same address, it won't be in the
transient_children_, unless it's explicitly added as a transient children, in
which case, the visibility will be updated with this CL, but will not with
WindowTracker.

I don't have strong opinion about which behavior is better. Such code (adding
transient children during visibility change) should probably be avoided anyway.

+    if (std::find(transient_children_.begin(), transient_children_.end(),
+                  child) != transient_children_.end()) {
+      Get(child)->UpdateTransientChildVisibility(visible);
+    }

[sadrul, 2015/11/21 08:42:43]

Maybe this could use a WindowTracker? e.g.

  WindowTracker tracker;
  for (Window* child : transient_children_)
    tracker.Add(child);

  while (!tracker.windows().empty()) {
    Window* window = *(tracker.windows().begin());
    Get(window)->Update...();
    tracker.Remove(window);
  }

+  }
 
   // Remember the show request in |show_on_parent_visible_| and hide it again
   // if the following conditions are met
   // - |parent_controls_visibility| is set to true.
   // - the window is hidden while the transient parent is not visible.
   // - Show/Hide was NOT called from TransientWindowManager.
   if (ignore_visibility_changed_event_ ||
       !transient_parent_ || !parent_controls_visibility_) {
     return;
   }
@@ skipping 36 matching lines @@
 
   // Destroy transient children, only after we've removed ourselves from our
   // parent, as destroying an active transient child may otherwise attempt to
   // refocus us.
   Windows transient_children(transient_children_);
   STLDeleteElements(&transient_children);
   DCHECK(transient_children_.empty());
 }
 
 }  // namespace wm