diff options
author | derat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-18 23:47:39 +0000 |
---|---|---|
committer | derat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-18 23:47:39 +0000 |
commit | 9ed496eea8ee18caaad34646cda3d6313dad3946 (patch) | |
tree | a841cac172b599b0c13907c95e4c0ede0134977d /ui | |
parent | 4a6742a435bf60d73f6ae5533adc991d3bd32aa6 (diff) | |
download | chromium_src-9ed496eea8ee18caaad34646cda3d6313dad3946.zip chromium_src-9ed496eea8ee18caaad34646cda3d6313dad3946.tar.gz chromium_src-9ed496eea8ee18caaad34646cda3d6313dad3946.tar.bz2 |
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
Diffstat (limited to 'ui')
-rw-r--r-- | ui/aura/desktop.cc | 51 | ||||
-rw-r--r-- | ui/aura/desktop.h | 45 | ||||
-rw-r--r-- | ui/aura/desktop_host_linux.cc | 10 | ||||
-rw-r--r-- | ui/aura/test/aura_test_base.cc | 24 | ||||
-rw-r--r-- | ui/aura/test/aura_test_base.h | 4 | ||||
-rw-r--r-- | ui/aura/test/test_desktop_delegate.h | 3 |
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(); |