3 files changed, 135 insertions, 25 deletions

diff --git a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
index 698a342..1151b86 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
@@ -192,12 +192,6 @@ DesktopNativeWidgetAura::~DesktopNativeWidgetAura() {
     delete native_widget_delegate_;
   else
     CloseNow();
-
-  stacking_client_.reset();  // Uses root_window_ at destruction.
-
-  root_window_->RemoveRootWindowObserver(this);
-  root_window_.reset();  // Uses input_method_event_filter_ at destruction.
-  input_method_event_filter_.reset();
 }
 
 // static
@@ -207,10 +201,41 @@ DesktopNativeWidgetAura* DesktopNativeWidgetAura::ForWindow(
 }
 
 void DesktopNativeWidgetAura::OnHostClosed() {
+  // Don't invoke Widget::OnNativeWidgetDestroying(), its done by
+  // DesktopRootWindowHost.
+
+  // Make sure we don't still have capture. Otherwise CaptureController and
+  // RootWindow are left referencing a deleted Window.
+  {
+    aura::Window* capture_window =
+        capture_client_->capture_client()->GetCaptureWindow();
+    if (capture_window && root_window_->Contains(capture_window))
+      capture_window->ReleaseCapture();
+  }
+
+  // DesktopRootWindowHost owns the ActivationController which ShadowController
+  // references. Make sure we destroy ShadowController early on.
+  shadow_controller_.reset();
+  tooltip_manager_.reset();
+  scoped_tooltip_client_.reset();
+  if (window_modality_controller_) {
+    root_window_->RemovePreTargetHandler(window_modality_controller_.get());
+    window_modality_controller_.reset();
+  }
+
   root_window_event_filter_->RemoveHandler(input_method_event_filter_.get());
-  // This will, through a long list of callbacks, trigger |root_window_| going
-  // away. See OnWindowDestroyed()
-  delete window_;
+
+  stacking_client_.reset();  // Uses root_window_ at destruction.
+
+  root_window_->RemoveRootWindowObserver(this);
+  root_window_.reset();  // Uses input_method_event_filter_ at destruction.
+  // RootWindow owns |desktop_root_window_host_|.
+  desktop_root_window_host_ = NULL;
+  window_ = NULL;
+
+  native_widget_delegate_->OnNativeWidgetDestroyed();
+  if (ownership_ == Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET)
+    delete this;
 }
 
 void DesktopNativeWidgetAura::InstallInputMethodEventFilter(
@@ -680,7 +705,8 @@ void DesktopNativeWidgetAura::ClearNativeFocus() {
 }
 
 gfx::Rect DesktopNativeWidgetAura::GetWorkAreaBoundsInScreen() const {
-  return desktop_root_window_host_->GetWorkAreaBoundsInScreen();
+  return desktop_root_window_host_ ?
+      desktop_root_window_host_->GetWorkAreaBoundsInScreen() : gfx::Rect();
 }
 
 void DesktopNativeWidgetAura::SetInactiveRenderingDisabled(bool value) {
@@ -782,23 +808,12 @@ void DesktopNativeWidgetAura::OnDeviceScaleFactorChanged(
 }
 
 void DesktopNativeWidgetAura::OnWindowDestroying() {
-  // DesktopRootWindowHost owns the ActivationController which ShadowController
-  // references. Make sure we destroy ShadowController early on.
-  shadow_controller_.reset();
-  // The DesktopRootWindowHost implementation sends OnNativeWidgetDestroying().
-  tooltip_manager_.reset();
-  scoped_tooltip_client_.reset();
-  if (window_modality_controller_) {
-    root_window_->RemovePreTargetHandler(window_modality_controller_.get());
-    window_modality_controller_.reset();
-  }
+  // Cleanup happens in OnHostClosed().
 }
 
 void DesktopNativeWidgetAura::OnWindowDestroyed() {
-  window_ = NULL;
-  native_widget_delegate_->OnNativeWidgetDestroyed();
-  if (ownership_ == Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET)
-    delete this;
+  // Cleanup happens in OnHostClosed(). We own |window_| (indirectly by way of
+  // |root_window_|) so there should be no need to do any processing here.
 }
 
 void DesktopNativeWidgetAura::OnWindowTargetVisibilityChanged(bool visible) {
diff --git a/ui/views/widget/desktop_aura/desktop_native_widget_aura.h b/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
index c1c97a0..9185248 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
@@ -59,7 +59,7 @@ class VIEWS_EXPORT DesktopNativeWidgetAura
 
   // Called by our DesktopRootWindowHost after it has deleted native resources;
   // this is the signal that we should start our shutdown.
-  void OnHostClosed();
+  virtual void OnHostClosed();
 
   // Installs the input method filter on |root|. This is intended to be invoked
   // by the DesktopRootWindowHost implementation during Init().
diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc
index 1c8a7a2..538788a 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -1872,5 +1872,100 @@ TEST_F(WidgetTest, GetAllChildWidgets) {
   EXPECT_TRUE(std::equal(expected.begin(), expected.end(), widgets.begin()));
 }
 
+// Used by DestroyChildWidgetsInOrder. On destruction adds the supplied name to
+// a vector.
+class DestroyedTrackingView : public View {
+ public:
+  DestroyedTrackingView(const std::string& name,
+                        std::vector<std::string>* add_to)
+      : name_(name),
+        add_to_(add_to) {
+  }
+
+  virtual ~DestroyedTrackingView() {
+    add_to_->push_back(name_);
+  }
+
+ private:
+  const std::string name_;
+  std::vector<std::string>* add_to_;
+
+  DISALLOW_COPY_AND_ASSIGN(DestroyedTrackingView);
+};
+
+class WidgetChildDestructionTest : public WidgetTest {
+ public:
+  WidgetChildDestructionTest() {}
+
+  // Creates a top level and a child, destroys the child and verifies the views
+  // of the child are destroyed before the views of the parent.
+  void RunDestroyChildWidgetsTest(bool top_level_has_desktop_native_widget_aura,
+                                  bool child_has_desktop_native_widget_aura) {
+    // When a View is destroyed its name is added here.
+    std::vector<std::string> destroyed;
+
+    Widget* top_level = new Widget;
+    Widget::InitParams params =
+        CreateParams(views::Widget::InitParams::TYPE_WINDOW);
+#if defined(USE_AURA) && !defined(OS_CHROMEOS)
+    if (top_level_has_desktop_native_widget_aura)
+      params.native_widget = new DesktopNativeWidgetAura(top_level);
+#endif
+    top_level->Init(params);
+    top_level->GetRootView()->AddChildView(
+        new DestroyedTrackingView("parent", &destroyed));
+    top_level->Show();
+
+    Widget* child = new Widget;
+    Widget::InitParams child_params =
+        CreateParams(views::Widget::InitParams::TYPE_POPUP);
+    child_params.parent = top_level->GetNativeView();
+#if defined(USE_AURA) && !defined(OS_CHROMEOS)
+    if (child_has_desktop_native_widget_aura)
+      child_params.native_widget = new DesktopNativeWidgetAura(child);
+#endif
+    child->Init(child_params);
+    child->GetRootView()->AddChildView(
+        new DestroyedTrackingView("child", &destroyed));
+    child->Show();
+
+    // Should trigger destruction of the child too.
+    top_level->native_widget_private()->CloseNow();
+
+    // Child should be destroyed first.
+    ASSERT_EQ(2u, destroyed.size());
+    EXPECT_EQ("child", destroyed[0]);
+    EXPECT_EQ("parent", destroyed[1]);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(WidgetChildDestructionTest);
+};
+
+#if defined(USE_AURA) && !defined(OS_CHROMEOS)
+// See description of RunDestroyChildWidgetsTest(). Parent uses
+// DesktopNativeWidgetAura.
+TEST_F(WidgetChildDestructionTest,
+       DestroyChildWidgetsInOrderWithDesktopNativeWidget) {
+  RunDestroyChildWidgetsTest(true, false);
+}
+
+// TODO: test fails on linux as destroying parent X widget does not
+// automatically destroy transients. http://crbug.com/300020 .
+#if !defined(OS_LINUX)
+// See description of RunDestroyChildWidgetsTest(). Both parent and child use
+// DesktopNativeWidgetAura.
+TEST_F(WidgetChildDestructionTest,
+       DestroyChildWidgetsInOrderWithDesktopNativeWidgetForBoth) {
+  RunDestroyChildWidgetsTest(true, true);
+}
+#endif
+#endif
+
+// See description of RunDestroyChildWidgetsTest().
+TEST_F(WidgetChildDestructionTest, DestroyChildWidgetsInOrder) {
+  RunDestroyChildWidgetsTest(false, false);
+}
+
 }  // namespace test
 }  // namespace views