Defer WidgetGtk's focus manager destruction

This is for the case when a WidgetGtk is hosted in another WidgtGtk and
holds a reference to its focus manager. When hosting widget is destroied,
it gets the "destroy" signal before the contained widget and
thus schedules its destruction before the contained widget. And currently,
we are not clearing the focus manager reference in another root view and
when contained widget destructs, we crash. By changing focus manager's
release into another message, the focus manager remains valid during
contained widget destruction.

BUG=none
TEST=In ChromeOS, bring up options dialog and then click on buttons such as language settings, networ config etc. Then dismiss the options dialog and Chrome should not crash.

Review URL: http://codereview.chromium.org/1661003

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

4 files changed, 124 insertions, 5 deletions

diff --git a/views/focus/focus_manager.h b/views/focus/focus_manager.h
index 2150560..565184c 100644
--- a/views/focus/focus_manager.h
+++ b/views/focus/focus_manager.h
@@ -181,7 +181,7 @@ class FocusManager {
   };
 
   explicit FocusManager(Widget* widget);
-  ~FocusManager();
+  virtual ~FocusManager();
 
   // Returns the global WidgetFocusManager instance for the running application.
   static WidgetFocusManager* GetWidgetFocusManager() {
diff --git a/views/focus/focus_manager_unittest.cc b/views/focus/focus_manager_unittest.cc
index cd89d5c..80cbd5f4 100644
--- a/views/focus/focus_manager_unittest.cc
+++ b/views/focus/focus_manager_unittest.cc
@@ -31,6 +31,7 @@
 #include "views/controls/tabbed_pane/tabbed_pane.h"
 #include "views/focus/accelerator_handler.h"
 #include "views/widget/root_view.h"
+#include "views/window/non_client_view.h"
 #include "views/window/window.h"
 #include "views/window/window_delegate.h"
 
@@ -1401,4 +1402,105 @@ TEST_F(FocusManagerTest, CreationForNativeRoot) {
 }
 #endif
 
+#if defined(OS_CHROMEOS)
+class FocusManagerDtorTest : public FocusManagerTest {
+ protected:
+  typedef std::vector<std::string> DtorTrackVector;
+
+  class FocusManagerDtorTracked : public FocusManager {
+   public:
+    FocusManagerDtorTracked(Widget* widget, DtorTrackVector* dtor_tracker)
+      : FocusManager(widget),
+        dtor_tracker_(dtor_tracker) {
+    }
+
+    virtual ~FocusManagerDtorTracked() {
+      dtor_tracker_->push_back("FocusManagerDtorTracked");
+    }
+
+    DtorTrackVector* dtor_tracker_;
+  };
+
+  class NativeButtonDtorTracked : public NativeButton {
+   public:
+    NativeButtonDtorTracked(const std::wstring& text,
+                            DtorTrackVector* dtor_tracker)
+        : NativeButton(NULL, text),
+          dtor_tracker_(dtor_tracker) {
+    };
+    virtual ~NativeButtonDtorTracked() {
+      dtor_tracker_->push_back("NativeButtonDtorTracked");
+    }
+
+    DtorTrackVector* dtor_tracker_;
+  };
+
+  class WindowGtkDtorTracked : public WindowGtk {
+   public:
+    WindowGtkDtorTracked(WindowDelegate* window_delegate,
+                         DtorTrackVector* dtor_tracker)
+        : WindowGtk(window_delegate),
+          dtor_tracker_(dtor_tracker) {
+      tracked_focus_manager_ = new FocusManagerDtorTracked(this,
+          dtor_tracker_);
+      // Replace focus_manager_ with FocusManagerDtorTracked
+      set_focus_manager(tracked_focus_manager_);
+
+      GetNonClientView()->SetFrameView(CreateFrameViewForWindow());
+      Init(NULL, gfx::Rect(0, 0, 100, 100));
+    }
+
+    virtual ~WindowGtkDtorTracked() {
+      dtor_tracker_->push_back("WindowGtkDtorTracked");
+    }
+
+    FocusManagerDtorTracked* tracked_focus_manager_;
+    DtorTrackVector* dtor_tracker_;
+  };
+
+ public:
+  virtual void SetUp() {
+   // Create WindowGtkDtorTracked that uses FocusManagerDtorTracked.
+   window_ = new WindowGtkDtorTracked(this, &dtor_tracker_);
+   ASSERT_TRUE(GetFocusManager() ==
+        static_cast<WindowGtkDtorTracked*>(window_)->tracked_focus_manager_);
+
+   window_->Show();
+  }
+
+  virtual void TearDown() {
+    if (window_) {
+      window_->Close();
+      message_loop()->RunAllPending();
+    }
+  }
+
+  DtorTrackVector dtor_tracker_;
+};
+
+TEST_F(FocusManagerDtorTest, FocusManagerDestructedLast) {
+  // Setup views hierarchy.
+  TabbedPane* tabbed_pane = new TabbedPane();
+  content_view_->AddChildView(tabbed_pane);
+
+  NativeButtonDtorTracked* button = new NativeButtonDtorTracked(L"button",
+                                                                &dtor_tracker_);
+  tabbed_pane->AddTab(L"Awesome tab", button);
+
+  // Close the window.
+  window_->Close();
+  message_loop()->RunAllPending();
+
+  // Test window, button and focus manager should all be destructed.
+  ASSERT_EQ(3, static_cast<int>(dtor_tracker_.size()));
+
+  // Focus manager should be the last one to destruct.
+  ASSERT_STREQ("FocusManagerDtorTracked", dtor_tracker_[2].c_str());
+
+  // Clear window_ so that we don't try to close it again.
+  window_ = NULL;
+}
+
+#endif  // defined(OS_CHROMEOS)
+
 }  // namespace views
diff --git a/views/widget/widget_gtk.cc b/views/widget/widget_gtk.cc
index 20b0287..6e78a71 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -136,6 +136,7 @@ WidgetGtk::WidgetGtk(Type type)
       type_(type),
       widget_(NULL),
       window_contents_(NULL),
+      focus_manager_(NULL),
       is_mouse_down_(false),
       has_capture_(false),
       last_mouse_event_was_move_(false),
@@ -162,7 +163,7 @@ WidgetGtk::WidgetGtk(Type type)
   }
 
   if (type_ != TYPE_CHILD)
-    focus_manager_.reset(new FocusManager(this));
+    focus_manager_ = new FocusManager(this);
 }
 
 WidgetGtk::~WidgetGtk() {
@@ -170,6 +171,16 @@ WidgetGtk::~WidgetGtk() {
   if (type_ != TYPE_CHILD)
     ActiveWindowWatcherX::RemoveObserver(this);
   MessageLoopForUI::current()->RemoveObserver(this);
+
+  // Defer focus manager's destruction. This is for the case when the
+  // focus manager is referenced by a child WidgetGtk (e.g. TabbedPane in a
+  // dialog). When gtk_widget_destroy is called on the parent, the destroy
+  // signal reaches parent first and then the child. Thus causing the parent
+  // WidgetGtk's dtor executed before the child's. If child's view hierarchy
+  // references this focus manager, it crashes. This will defer focus manager's
+  // destruction after child WidgetGtk's dtor.
+  if (focus_manager_)
+    MessageLoop::current()->DeleteSoon(FROM_HERE, focus_manager_);
 }
 
 GtkWindow* WidgetGtk::GetTransientParent() const {
@@ -680,8 +691,8 @@ ThemeProvider* WidgetGtk::GetDefaultThemeProvider() const {
 }
 
 FocusManager* WidgetGtk::GetFocusManager() {
-  if (focus_manager_.get())
-    return focus_manager_.get();
+  if (focus_manager_)
+    return focus_manager_;
 
   Widget* root = GetRootWidget();
   if (root && root != this) {
diff --git a/views/widget/widget_gtk.h b/views/widget/widget_gtk.h
index d10237d..b86820a 100644
--- a/views/widget/widget_gtk.h
+++ b/views/widget/widget_gtk.h
@@ -282,6 +282,12 @@ class WidgetGtk
   // Are we a subclass of WindowGtk?
   bool is_window_;
 
+  // For test code to provide a customized focus manager.
+  void set_focus_manager(FocusManager* focus_manager) {
+    delete focus_manager_;
+    focus_manager_ = focus_manager;
+  }
+
  private:
   class DropObserver;
   friend class DropObserver;
@@ -341,7 +347,7 @@ class WidgetGtk
   // children.  NULL for non top-level widgets.
   // WARNING: RootView's destructor calls into the FocusManager. As such, this
   // must be destroyed AFTER root_view_.
-  scoped_ptr<FocusManager> focus_manager_;
+  FocusManager* focus_manager_;
 
   // The root of the View hierarchy attached to this window.
   scoped_ptr<RootView> root_view_;