Observe for window's (self) deletion during activation/focus change

ui/aura/window.cc

 // Copyright (c) 2012 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/aura/window.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "ui/aura/client/capture_client.h"
 #include "ui/aura/client/event_client.h"
 #include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/client/stacking_client.h"
 #include "ui/aura/client/visibility_client.h"
 #include "ui/aura/env.h"
 #include "ui/aura/focus_manager.h"
 #include "ui/aura/layout_manager.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window_delegate.h"
+#include "ui/aura/window_destruction_observer.h"
 #include "ui/aura/window_observer.h"
 #include "ui/base/animation/multi_animation.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/layer.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/path.h"
 #include "ui/gfx/screen.h"
 
 namespace aura {
 
@@ skipping 939 matching lines @@
                       OnWindowHierarchyChanged(local_params));
     break;
   default:
     NOTREACHED();
     break;
   }
 }
 
 void Window::NotifyWindowVisibilityChanged(aura::Window* target,
                                            bool visible) {
-  NotifyWindowVisibilityChangedDown(target, visible);
+  if (!NotifyWindowVisibilityChangedDown(target, visible)) {
+    return; // |this| has been deleted.
+  }
   NotifyWindowVisibilityChangedUp(target, visible);
 }
 
-void Window::NotifyWindowVisibilityChangedAtReceiver(aura::Window* target,
+bool Window::NotifyWindowVisibilityChangedAtReceiver(aura::Window* target,
                                                      bool visible) {
+  // |this| may be deleted during a call to OnWindowVisibilityChanged
+  // on one of the observers. We create an local observer for that. In
+  // that case we exit without further access to any members.
+  WindowDestructionObserver destruction_observer;

[Ben Goodger (Google), 2013/03/19 18:13:17]

I think the API would be a little cleaner here if you just made this ctor take
the window, and you had a destroyed() accessor on it.

[sschmitz, 2013/03/19 18:36:50]

Neat.
Done.

+  AddObserver(&destruction_observer);
+  const Window* copy_this = this;

[Ben Goodger (Google), 2013/03/19 18:13:17]

then you wouldn't have to do things like this which are awkward

[sschmitz, 2013/03/19 18:36:50]

Done.

   FOR_EACH_OBSERVER(WindowObserver, observers_,
                     OnWindowVisibilityChanged(target, visible));
+  if (destruction_observer.was_destroyed(copy_this))
+    return false; // |this| has been deleted.
+  RemoveObserver(&destruction_observer);
+  return true;
 }
 
-void Window::NotifyWindowVisibilityChangedDown(aura::Window* target,
+bool Window::NotifyWindowVisibilityChangedDown(aura::Window* target,
                                                bool visible) {
-  NotifyWindowVisibilityChangedAtReceiver(target, visible);
-  for (Window::Windows::const_iterator it = children_.begin();
-       it != children_.end(); ++it) {
-    (*it)->NotifyWindowVisibilityChangedDown(target, visible);
-  }
+  if (!NotifyWindowVisibilityChangedAtReceiver(target, visible))
+    return false; // |this| was deleted.
+  std::set<const Window*> child_already_processed;

[Ben Goodger (Google), 2013/03/19 18:13:17]

you only fill this, but never actually use it.

[sschmitz, 2013/03/19 18:36:50]

I'm using the fact that set<>::insert() returns a pair where |second| is true if
the insert succeeded and false if the element already existed.

+  bool child_destroyed = false;
+  do {
+    child_destroyed = false;
+    for (Window::Windows::const_iterator it = children_.begin();
+         it != children_.end(); ++it) {
+      if (!child_already_processed.insert(*it).second)
+        continue;
+      if (!(*it)->NotifyWindowVisibilityChangedDown(target, visible)) {
+        // |*it| was deleted, |it| is invalid and |children_| has changed.
+        // We exit the current for-loop and enter a new one.
+        child_destroyed = true;
+        break;
+      }
+    }
+  } while (child_destroyed);
+  return true;
 }
 
 void Window::NotifyWindowVisibilityChangedUp(aura::Window* target,
                                              bool visible) {
-  for (Window* window = this; window; window = window->parent())
-    window->NotifyWindowVisibilityChangedAtReceiver(target, visible);
+  for (Window* window = this; window; window = window->parent()) {
+    bool ret = window->NotifyWindowVisibilityChangedAtReceiver(target, visible);
+    DCHECK(ret);
+  }
 }
 
 void Window::OnLayerBoundsChanged(const gfx::Rect& old_bounds,
                                   bool contained_mouse) {
   if (layout_manager_.get())
     layout_manager_->OnWindowResized();
   if (delegate_)
     delegate_->OnBoundsChanged(old_bounds, bounds());
   FOR_EACH_OBSERVER(WindowObserver,
                     observers_,
@@ skipping 49 matching lines @@
   bool contains_mouse = false;
   if (IsVisible()) {
     RootWindow* root_window = GetRootWindow();
     contains_mouse = root_window &&
         ContainsPointInRoot(root_window->GetLastMouseLocationInRoot());
   }
   return contains_mouse;
 }
 
 }  // namespace aura