wm: Avoid leak if a duplicate login window is mapped.
This is just a guess at a possible cause for a failed assert
that I've noticed on the crash server. If a second WebUI,
background, or wizard/guest window gets mapped,
LoginController should unregister its interest in the
earlier window.
BUG=chromium-os:18809
TEST=added a test for the duplicate-wizard-window case
Change-Id: I46ec5b1c3277dcc2f87e6a3b0a4e30d1bc448335
Reviewed-on: http://gerrit.chromium.org/gerrit/5495
Tested-by: Daniel Erat <[email protected]>
Reviewed-by: Daniel Erat <[email protected]>
diff --git a/login/login_controller.cc b/login/login_controller.cc
index 702770d..90dbfa0 100644
--- a/login/login_controller.cc
+++ b/login/login_controller.cc
@@ -241,8 +241,10 @@
switch (win->type()) {
case chromeos::WM_IPC_WINDOW_LOGIN_GUEST: {
- if (wizard_window_)
+ if (wizard_window_) {
LOG(WARNING) << "Two wizard windows encountered";
+ registrar_.UnregisterForWindowEvents(wizard_window_->xid());
+ }
wizard_window_ = win;
wm_->focus_manager()->UseClickToFocusForWindow(
wizard_window_, FocusManager::PASS_CLICKS_THROUGH);
@@ -275,8 +277,10 @@
break;
}
case chromeos::WM_IPC_WINDOW_LOGIN_BACKGROUND: {
- if (background_window_)
+ if (background_window_) {
LOG(WARNING) << "Two background windows encountered";
+ registrar_.UnregisterForWindowEvents(background_window_->xid());
+ }
background_window_ = win;
wm_->focus_manager()->UseClickToFocusForWindow(
background_window_, FocusManager::PASS_CLICKS_THROUGH);
@@ -284,8 +288,10 @@
break;
}
case chromeos::WM_IPC_WINDOW_LOGIN_WEBUI: {
- if (webui_window_)
+ if (webui_window_) {
LOG(WARNING) << "Two webui browser windows encountered";
+ registrar_.UnregisterForWindowEvents(webui_window_->xid());
+ }
webui_window_ = win;
wm_->focus_manager()->UseClickToFocusForWindow(
webui_window_, FocusManager::PASS_CLICKS_THROUGH);
diff --git a/login/login_controller_test.cc b/login/login_controller_test.cc
index 3d3753f..281c7a1 100644
--- a/login/login_controller_test.cc
+++ b/login/login_controller_test.cc
@@ -1295,6 +1295,31 @@
EXPECT_EQ(webui_window_xid_, GetActiveWindowProperty());
}
+// Check that when we see duplicate wizard windows, we unregister our interest
+// in the first one when the second one is mapped so that we won't fail an
+// assert if the first window is destroyed later. This is just a shot in the
+// dark as a possible cause of http://crosbug.com/18809.
+TEST_F(LoginControllerTest, HandleDuplicateWindows) {
+ CreateWebUILoginWindow();
+
+ XWindow wizard_xid1 = CreateSimpleWindow();
+ wm_->wm_ipc()->SetWindowType(
+ wizard_xid1, chromeos::WM_IPC_WINDOW_LOGIN_GUEST, NULL);
+ SendInitialEventsForWindow(wizard_xid1);
+
+ XWindow wizard_xid2 = CreateSimpleWindow();
+ wm_->wm_ipc()->SetWindowType(
+ wizard_xid2, chromeos::WM_IPC_WINDOW_LOGIN_GUEST, NULL);
+ SendInitialEventsForWindow(wizard_xid2);
+
+ XEvent event;
+ xconn_->UnmapWindow(wizard_xid1);
+ xconn_->InitUnmapEvent(&event, wizard_xid1);
+ wm_->HandleEvent(&event);
+ xconn_->InitDestroyWindowEvent(&event, wizard_xid1);
+ wm_->HandleEvent(&event);
+}
+
// Test that we focus the first controls window as soon as we map it.
TEST_F(LoginControllerTest, FocusFirstControlsWindowImmediately) {
// Create just a background window.
