aura: Don't recycle Desktops between tests.

We were previously using the same static Desktop object
across all tests.  DesktopHostLinux hangs on to
MessagePumpX's connection to the X server.  We create a new
MessageLoop for each test, so any attempts by
DesktopHostLinux to send X requests after the first test
finished would result in a crash.

This change makes us throw out the Desktop object (and hence,
the DesktopHostLinux) after each test, along with cleaning
up some other bits of the testing code.

BUG=100425
TEST=ran a test that formerly crashed due to the problem described above; it works now

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

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

6 files changed, 74 insertions, 63 deletions

diff --git a/ui/aura/desktop.cc b/ui/aura/desktop.cc
index 11b076c..35409a9 100644
--- a/ui/aura/desktop.cc
+++ b/ui/aura/desktop.cc
@@ -29,7 +29,6 @@ ui::Compositor*(*Desktop::compositor_factory_)() = NULL;
 
 Desktop::Desktop()
     : Window(NULL),
-      delegate_(NULL),
       host_(aura::DesktopHost::Create(gfx::Rect(200, 200, 1280, 1024))),
       ALLOW_THIS_IN_INITIALIZER_LIST(schedule_paint_factory_(this)),
       active_window_(NULL),
@@ -59,15 +58,23 @@ Desktop::~Desktop() {
     instance_ = NULL;
 }
 
-void Desktop::SetDelegate(DesktopDelegate* delegate) {
-  delegate_.reset(delegate);
+// static
+Desktop* Desktop::GetInstance() {
+  if (!instance_) {
+    instance_ = new Desktop;
+    instance_->Init();
+  }
+  return instance_;
 }
 
-void Desktop::Init() {
-  Window::Init();
-  SetBounds(gfx::Rect(gfx::Point(), host_->GetSize()));
-  Show();
-  compositor()->SetRootLayer(layer());
+// static
+void Desktop::DeleteInstanceForTesting() {
+  delete instance_;
+  instance_ = NULL;
+}
+
+void Desktop::SetDelegate(DesktopDelegate* delegate) {
+  delegate_.reset(delegate);
 }
 
 void Desktop::ShowDesktop() {
@@ -157,14 +164,6 @@ void Desktop::OnHostResized(const gfx::Size& size) {
   FOR_EACH_OBSERVER(DesktopObserver, observers_, OnDesktopResized(size));
 }
 
-void Desktop::ScheduleDraw() {
-  if (!schedule_paint_factory_.HasWeakPtrs()) {
-    MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&Desktop::Draw, schedule_paint_factory_.GetWeakPtr()));
-  }
-}
-
 void Desktop::SetActiveWindow(Window* window, Window* to_focus) {
   // We only allow top level windows to be active.
   if (window && window != window->GetToplevelWindow()) {
@@ -294,6 +293,14 @@ void Desktop::HandleMouseMoved(const MouseEvent& event, Window* target) {
   }
 }
 
+void Desktop::ScheduleDraw() {
+  if (!schedule_paint_factory_.HasWeakPtrs()) {
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&Desktop::Draw, schedule_paint_factory_.GetWeakPtr()));
+  }
+}
+
 bool Desktop::CanFocus() const {
   return IsVisible();
 }
@@ -326,13 +333,11 @@ bool Desktop::IsFocusedWindow(const Window* window) const {
   return focused_window_ == window;
 }
 
-// static
-Desktop* Desktop::GetInstance() {
-  if (!instance_) {
-    instance_ = new Desktop;
-    instance_->Init();
-  }
-  return instance_;
+void Desktop::Init() {
+  Window::Init();
+  SetBounds(gfx::Rect(gfx::Point(), host_->GetSize()));
+  Show();
+  compositor()->SetRootLayer(layer());
 }
 
 }  // namespace aura
diff --git a/ui/aura/desktop.h b/ui/aura/desktop.h
index df3376d0..94356b8 100644
--- a/ui/aura/desktop.h
+++ b/ui/aura/desktop.h
@@ -41,13 +41,24 @@ class AURA_EXPORT Desktop : public ui::CompositorDelegate,
   Desktop();
   virtual ~Desktop();
 
-  gfx::Point last_mouse_location() const { return last_mouse_location_; }
+  static Desktop* GetInstance();
+  static void DeleteInstanceForTesting();
+
+  static void set_compositor_factory_for_testing(ui::Compositor*(*factory)()) {
+    compositor_factory_ = factory;
+  }
+  static ui::Compositor* (*compositor_factory())() {
+    return compositor_factory_;
+  }
 
+  ui::Compositor* compositor() { return compositor_.get(); }
+  gfx::Point last_mouse_location() const { return last_mouse_location_; }
   DesktopDelegate* delegate() { return delegate_.get(); }
-  void SetDelegate(DesktopDelegate* delegate);
+  Window* active_window() { return active_window_; }
+  Window* mouse_pressed_handler() { return mouse_pressed_handler_; }
+  Window* capture_window() { return capture_window_; }
 
-  // Initializes the desktop.
-  void Init();
+  void SetDelegate(DesktopDelegate* delegate);
 
   // Shows the desktop host.
   void ShowDesktop();
@@ -77,20 +88,9 @@ class AURA_EXPORT Desktop : public ui::CompositorDelegate,
   // Called when the host changes size.
   void OnHostResized(const gfx::Size& size);
 
-  // Compositor we're drawing to.
-  ui::Compositor* compositor() { return compositor_.get(); }
-
-  static void set_compositor_factory_for_testing(ui::Compositor*(*factory)()) {
-    compositor_factory_ = factory;
-  }
-  static ui::Compositor* (*compositor_factory())() {
-    return compositor_factory_;
-  }
-
   // Sets the active window to |window| and the focused window to |to_focus|.
   // If |to_focus| is NULL, |window| is focused.
   void SetActiveWindow(Window* window, Window* to_focus);
-  Window* active_window() { return active_window_; }
 
   // Activates the topmost window. Does nothing if the topmost window is already
   // active.
@@ -114,22 +114,12 @@ class AURA_EXPORT Desktop : public ui::CompositorDelegate,
   void AddObserver(DesktopObserver* observer);
   void RemoveObserver(DesktopObserver* observer);
 
-  // Current handler for mouse events.
-  Window* mouse_pressed_handler() { return mouse_pressed_handler_; }
-
-  // Capture -------------------------------------------------------------------
-
   // Sets capture to the specified window.
   void SetCapture(Window* window);
 
   // If |window| has mouse capture, the current capture window is set to NULL.
   void ReleaseCapture(Window* window);
 
-  // Returns the window that has mouse capture.
-  Window* capture_window() { return capture_window_; }
-
-  static Desktop* GetInstance();
-
  private:
   // Called whenever the mouse moves, tracks the current |mouse_moved_handler_|,
   // sending exited and entered events as its value changes.
@@ -148,12 +138,15 @@ class AURA_EXPORT Desktop : public ui::CompositorDelegate,
   virtual Window* GetFocusedWindow() OVERRIDE;
   virtual bool IsFocusedWindow(const Window* window) const OVERRIDE;
 
-  scoped_ptr<DesktopDelegate> delegate_;
+  // Initializes the desktop.
+  void Init();
 
   scoped_refptr<ui::Compositor> compositor_;
 
   scoped_ptr<DesktopHost> host_;
 
+  scoped_ptr<DesktopDelegate> delegate_;
+
   static Desktop* instance_;
 
   // Used to schedule painting.
diff --git a/ui/aura/desktop_host_linux.cc b/ui/aura/desktop_host_linux.cc
index 158eb38..57c506e 100644
--- a/ui/aura/desktop_host_linux.cc
+++ b/ui/aura/desktop_host_linux.cc
@@ -4,6 +4,13 @@
 
 #include "ui/aura/desktop_host.h"
 
+#include <X11/Xlib.h>
+
+// Get rid of a macro from Xlib.h that conflicts with Aura's RootWindow class.
+#undef RootWindow
+
+#include <algorithm>
+
 #include "base/message_loop.h"
 #include "base/message_pump_x.h"
 #include "ui/aura/cursor.h"
@@ -152,7 +159,6 @@ DesktopHostLinux::DesktopHostLinux(const gfx::Rect& bounds)
                                  bounds.x(), bounds.y(),
                                  bounds.width(), bounds.height(),
                                  0, 0, 0);
-  XMapWindow(xdisplay_, xwindow_);
 
   long event_mask = ButtonPressMask | ButtonReleaseMask |
                     KeyPressMask | KeyReleaseMask |
@@ -271,6 +277,8 @@ gfx::AcceleratedWidget DesktopHostLinux::GetAcceleratedWidget() {
 }
 
 void DesktopHostLinux::Show() {
+  XMapWindow(xdisplay_, xwindow_);
+  XFlush(xdisplay_);
 }
 
 gfx::Size DesktopHostLinux::GetSize() const {
diff --git a/ui/aura/test/aura_test_base.cc b/ui/aura/test/aura_test_base.cc
index cbe0fdc..a0e27ce 100644
--- a/ui/aura/test/aura_test_base.cc
+++ b/ui/aura/test/aura_test_base.cc
@@ -25,6 +25,12 @@ AuraTestBase::AuraTestBase()
 #if defined(OS_WIN)
   OleInitialize(NULL);
 #endif
+
+  aura::Desktop::set_compositor_factory_for_testing(&TestCreateCompositor);
+  // TestDesktopDelegate is owned by the desktop.
+  new TestDesktopDelegate();
+  Desktop::GetInstance()->Show();
+  Desktop::GetInstance()->SetHostSize(gfx::Size(600, 600));
 }
 
 AuraTestBase::~AuraTestBase() {
@@ -35,25 +41,25 @@ AuraTestBase::~AuraTestBase() {
       << "You have overridden SetUp but never called super class's SetUp";
   CHECK(teardown_called_)
       << "You have overridden TearDown but never called super class's TearDown";
+
+  // Flush the message loop because we have pending release tasks
+  // and these tasks if un-executed would upset Valgrind.
+  message_loop_.RunAllPending();
+
+  // Ensure that we don't use the previously-allocated static Desktop object
+  // later -- on Linux, it holds a reference to our message loop's X connection.
+  aura::Desktop::DeleteInstanceForTesting();
+  aura::Desktop::set_compositor_factory_for_testing(NULL);
 }
 
 void AuraTestBase::SetUp() {
   testing::Test::SetUp();
   setup_called_ = true;
-  aura::Desktop::set_compositor_factory_for_testing(&TestCreateCompositor);
-  // TestDesktopDelegate is owned by the desktop.
-  new TestDesktopDelegate();
-  Desktop::GetInstance()->Show();
-  Desktop::GetInstance()->SetHostSize(gfx::Size(600, 600));
 }
 
 void AuraTestBase::TearDown() {
-  // Flush the message loop because we have pending release tasks
-  // and these tasks if un-executed would upset Valgrind.
-  RunPendingMessages();
   teardown_called_ = true;
   testing::Test::TearDown();
-  aura::Desktop::set_compositor_factory_for_testing(NULL);
 }
 
 }  // namespace test
diff --git a/ui/aura/test/aura_test_base.h b/ui/aura/test/aura_test_base.h
index 5076969..4aafa0f 100644
--- a/ui/aura/test/aura_test_base.h
+++ b/ui/aura/test/aura_test_base.h
@@ -20,10 +20,6 @@ class AuraTestBase : public testing::Test {
   AuraTestBase();
   virtual ~AuraTestBase();
 
-  void RunPendingMessages() {
-    message_loop_.RunAllPending();
-  }
-
   // testing::Test:
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
diff --git a/ui/aura/test/test_desktop_delegate.h b/ui/aura/test/test_desktop_delegate.h
index 1174eb48..09bfc31e 100644
--- a/ui/aura/test/test_desktop_delegate.h
+++ b/ui/aura/test/test_desktop_delegate.h
@@ -19,6 +19,9 @@ namespace test {
 
 class TestDesktopDelegate : public DesktopDelegate {
  public:
+  // Callers should allocate a TestDesktopDelegate on the heap and then forget
+  // about it -- the c'tor passes ownership of the TestDesktopDelegate to the
+  // static Desktop object.
   TestDesktopDelegate();
   virtual ~TestDesktopDelegate();