Makes OnWindowDestroyed be sent after transients are destroyed

Currently WindowObservers are notified after transients are destroyed,
but not WindowDelegate. This patch makes the two consistent. Now both
are notified after transients are destroyed.

This is a precursor to making sure child widgets are destroyed before
the parent widget.

BUG=288953
TEST=covered by test
R=ben@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@225509 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 49 insertions, 7 deletions

diff --git a/ui/aura/window.cc b/ui/aura/window.cc
index b140482..20ce9b5 100644
--- a/ui/aura/window.cc
+++ b/ui/aura/window.cc
@@ -94,13 +94,6 @@ Window::~Window() {
   if (parent_)
     parent_->RemoveChild(this);
 
-  // And let the delegate do any post cleanup.
-  // TODO(beng): Figure out if this notification needs to happen here, or if it
-  // can be moved down adjacent to the observer notification. If it has to be
-  // done here, the reason why should be documented.
-  if (delegate_)
-    delegate_->OnWindowDestroyed();
-
   // Destroy transient children, only after we've removed ourselves from our
   // parent, as destroying an active transient child may otherwise attempt to
   // refocus us.
@@ -108,6 +101,9 @@ Window::~Window() {
   STLDeleteElements(&transient_children);
   DCHECK(transient_children_.empty());
 
+  // Delegate and observers need to be notified after transients are deleted.
+  if (delegate_)
+    delegate_->OnWindowDestroyed();
   FOR_EACH_OBSERVER(WindowObserver, observers_, OnWindowDestroyed(this));
 
   // Clear properties.
diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc
index a62a35b..ac0846a 100644
--- a/ui/aura/window_unittest.cc
+++ b/ui/aura/window_unittest.cc
@@ -4,7 +4,9 @@
 
 #include "ui/aura/window.h"
 
+#include <string>
 #include <utility>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
@@ -3050,5 +3052,49 @@ TEST_F(WindowTest, OnWindowHierarchyChange) {
 
 }
 
+namespace {
+
+// Used by NotifyDelegateAfterDeletingTransients. Adds a string to a vector when
+// OnWindowDestroyed() is invoked so that destruction order can be verified.
+class DestroyedTrackingDelegate : public TestWindowDelegate {
+ public:
+  explicit DestroyedTrackingDelegate(const std::string& name,
+                                     std::vector<std::string>* results)
+      : name_(name),
+        results_(results) {}
+
+  virtual void OnWindowDestroyed() OVERRIDE {
+    results_->push_back(name_);
+  }
+
+ private:
+  const std::string name_;
+  std::vector<std::string>* results_;
+
+  DISALLOW_COPY_AND_ASSIGN(DestroyedTrackingDelegate);
+};
+
+}  // namespace
+
+// Verifies the delegate is notified of destruction after transients are
+// destroyed.
+TEST_F(WindowTest, NotifyDelegateAfterDeletingTransients) {
+  std::vector<std::string> destruction_order;
+
+  DestroyedTrackingDelegate parent_delegate("parent", &destruction_order);
+  scoped_ptr<Window> parent(new Window(&parent_delegate));
+  parent->Init(ui::LAYER_NOT_DRAWN);
+
+  DestroyedTrackingDelegate transient_delegate("transient", &destruction_order);
+  Window* transient = new Window(&transient_delegate);  // Owned by |parent|.
+  transient->Init(ui::LAYER_NOT_DRAWN);
+  parent->AddTransientChild(transient);
+  parent.reset();
+
+  ASSERT_EQ(2u, destruction_order.size());
+  EXPECT_EQ("transient", destruction_order[0]);
+  EXPECT_EQ("parent", destruction_order[1]);
+}
+
 }  // namespace test
 }  // namespace aura