Change destruction order of FocusController

Currently the FocusController is destroyed after the root
window. FocusController may use
DesktopNativeWidgetAura::content_window_. Since the FocusController is
destroyed after content_window_ it is entirely possible for
FocusController to attempt to use content_window_ after it's been
destroyed.

This patch destroys FocusController before the root.

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

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

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

3 files changed, 40 insertions, 6 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 0f1a5e1..76a9a6a 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
@@ -286,6 +286,15 @@ void DesktopNativeWidgetAura::OnHostClosed() {
 
   capture_client_.reset();  // Uses root_window_ at destruction.
 
+  // FocusController uses |content_window_|. Destroy it now so that we don't
+  // have to worry about the possibility of FocusController attempting to use
+  // |content_window_| after it's been destroyed but before all child windows
+  // have been destroyed.
+  root_window_->window()->RemovePreTargetHandler(focus_client_.get());
+  aura::client::SetFocusClient(root_window_->window(), NULL);
+  aura::client::SetActivationClient(root_window_->window(), NULL);
+  focus_client_.reset();
+
   root_window_->RemoveRootWindowObserver(this);
   root_window_.reset();  // Uses input_method_event_filter_ at destruction.
   // RootWindow owns |desktop_root_window_host_|.
@@ -301,10 +310,6 @@ void DesktopNativeWidgetAura::OnDesktopWindowTreeHostDestroyed(
     aura::RootWindow* root) {
   // |root_window_| is still valid, but DesktopWindowTreeHost is nearly
   // destroyed. Do cleanup here of members DesktopWindowTreeHost may also use.
-  aura::client::SetFocusClient(root->window(), NULL);
-  aura::client::SetActivationClient(root->window(), NULL);
-  focus_client_.reset();
-
   aura::client::SetDispatcherClient(root->window(), NULL);
   dispatcher_client_.reset();
 
@@ -457,7 +462,8 @@ void DesktopNativeWidgetAura::InitNativeWidget(
   aura::client::SetDragDropClient(root_window_->window(),
                                   drag_drop_client_.get());
 
-  focus_client_->FocusWindow(content_window_);
+  static_cast<aura::client::FocusClient*>(focus_client_.get())->
+      FocusWindow(content_window_);
 
   OnWindowTreeHostResized(root_window_.get());
 
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 1f25303..64a1fe7 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
@@ -31,6 +31,7 @@ namespace views {
 namespace corewm {
 class CompoundEventFilter;
 class CursorManager;
+class FocusController;
 class InputMethodEventFilter;
 class ShadowController;
 class TooltipController;
@@ -268,7 +269,7 @@ class VIEWS_EXPORT DesktopNativeWidgetAura
 
   internal::NativeWidgetDelegate* native_widget_delegate_;
 
-  scoped_ptr<aura::client::FocusClient> focus_client_;
+  scoped_ptr<corewm::FocusController> focus_client_;
   scoped_ptr<DesktopDispatcherClient> dispatcher_client_;
   scoped_ptr<aura::client::ScreenPositionClient> position_client_;
   scoped_ptr<aura::client::DragDropClient> drag_drop_client_;
diff --git a/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc b/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc
index 17a5f97..61dde12 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/root_window.h"
+#include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/window.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
@@ -159,4 +160,30 @@ TEST_F(DesktopNativeWidgetAuraTest, GlobalCursorState) {
   EXPECT_TRUE(cursor_client_b->IsCursorVisible());
 }
 
+// Verifies FocusController doesn't attempt to access |content_window_| during
+// destruction. Previously the FocusController was destroyed after the window.
+// This could be problematic as FocusController references |content_window_| and
+// could attempt to use it after |content_window_| was destroyed. This test
+// verifies this doesn't happen. Note that this test only failed under ASAN.
+TEST_F(DesktopNativeWidgetAuraTest, DontAccessContentWindowDuringDestruction) {
+  aura::test::TestWindowDelegate delegate;
+  {
+    Widget widget;
+    Widget::InitParams init_params =
+        CreateParams(Widget::InitParams::TYPE_WINDOW);
+    init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+    DesktopNativeWidgetAura* desktop_native_widget_aura =
+        new DesktopNativeWidgetAura(&widget);
+    init_params.native_widget = desktop_native_widget_aura;
+    widget.Init(init_params);
+
+    // Owned by |widget|.
+    aura::Window* window = new aura::Window(&delegate);
+    window->Show();
+    widget.GetNativeWindow()->parent()->AddChild(window);
+
+    widget.Show();
+  }
+}
+
 }  // namespace views