Revert 225755 "Makes destruction of DesktopNativeWidgetAura saner"

> Makes destruction of DesktopNativeWidgetAura saner
> 
> Prior to this patch DesktopNativeWidgetAura destruction keyed off
> destruction of the window_ (not the rootwindow). This meant that by
> the time we destroyed the Widget the RootWindow was still live. This
> meant any children of the rootwindow, such as child widgets, were
> alive as well. The general expectation is that by the time a Widget is
> destroyed and child Widgets have also been destroyed.
> 
> This patch changes destruction so that we key off the
> AcceleratedWidget going away. When that happens we destroy the
> RootWindow. By doing this we ensure child Widgets have also been
> destroyed.
> 
> BUG=288953
> TEST=covered by test
> R=ben@chromium.org
> 
> Review URL: https://codereview.chromium.org/24515004

TBR=sky@chromium.org

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

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

3 files changed, 25 insertions, 131 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 794e434..7dd3dfe 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
@@ -192,6 +192,12 @@ 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
@@ -201,41 +207,10 @@ 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());
-
-  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;
+  // This will, through a long list of callbacks, trigger |root_window_| going
+  // away. See OnWindowDestroyed()
+  delete window_;
 }
 
 void DesktopNativeWidgetAura::InstallInputMethodEventFilter(
@@ -703,8 +678,7 @@ void DesktopNativeWidgetAura::ClearNativeFocus() {
 }
 
 gfx::Rect DesktopNativeWidgetAura::GetWorkAreaBoundsInScreen() const {
-  return desktop_root_window_host_ ?
-      desktop_root_window_host_->GetWorkAreaBoundsInScreen() : gfx::Rect();
+  return desktop_root_window_host_->GetWorkAreaBoundsInScreen();
 }
 
 void DesktopNativeWidgetAura::SetInactiveRenderingDisabled(bool value) {
@@ -806,12 +780,23 @@ void DesktopNativeWidgetAura::OnDeviceScaleFactorChanged(
 }
 
 void DesktopNativeWidgetAura::OnWindowDestroying() {
-  // Cleanup happens in OnHostClosed().
+  // 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();
+  }
 }
 
 void DesktopNativeWidgetAura::OnWindowDestroyed() {
-  // Cleanup happens in OnHostClosed(). We own |window_| (indirectly by way of
-  // |root_window_|) so there should be no need to do any processing here.
+  window_ = NULL;
+  native_widget_delegate_->OnNativeWidgetDestroyed();
+  if (ownership_ == Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET)
+    delete this;
 }
 
 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 19ae21d..b644e7c 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.
-  virtual void OnHostClosed();
+  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 3605b35..1c8a7a2 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -1872,96 +1872,5 @@ 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);
-}
-
-// See description of RunDestroyChildWidgetsTest(). Both parent and child use
-// DesktopNativeWidgetAura.
-TEST_F(WidgetChildDestructionTest,
-       DestroyChildWidgetsInOrderWithDesktopNativeWidgetForBoth) {
-  RunDestroyChildWidgetsTest(true, true);
-}
-#endif
-
-// See description of RunDestroyChildWidgetsTest().
-TEST_F(WidgetChildDestructionTest, DestroyChildWidgetsInOrder) {
-  RunDestroyChildWidgetsTest(false, false);
-}
-
 }  // namespace test
 }  // namespace views