Check that child window was not destroyed inside UpdateTransientChildVisibility.

ui/wm/core/transient_window_manager.cc

Index: ui/wm/core/transient_window_manager.cc
diff --git a/ui/wm/core/transient_window_manager.cc b/ui/wm/core/transient_window_manager.cc
index 2ca145d2420dbdd9f9a2fc81fd785e8e64d7a495..dff5bdf046479b5e7d151fa4ce6150ceccead6ad 100644
--- a/ui/wm/core/transient_window_manager.cc
+++ b/ui/wm/core/transient_window_manager.cc
@@ -162,8 +162,14 @@ void TransientWindowManager::OnWindowVisibilityChanged(Window* window,
 
   // 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