wm: Focus newest-mapped modal window.
Update the window manager so that when a browser has
multiple modal transient windows, the most-recently-mapped
one gets the focus and is raised above its siblings.
BUG=chromium-os:17642
TEST=added; manual testing suggests that there may be another issue in chrome
Change-Id: I1cbe1dff6c590a25aa0c134be5748e44ff7d98dd
Reviewed-on: http://gerrit.chromium.org/gerrit/4551
Reviewed-by: James Cook <jamescook@chromium.org>
Tested-by: Daniel Erat <derat@chromium.org>
diff --git a/layout2/browser_window.cc b/layout2/browser_window.cc
index 95fba27..5c0698c 100644
--- a/layout2/browser_window.cc
+++ b/layout2/browser_window.cc
@@ -168,6 +168,10 @@
transients_->SetPreferredWindowToFocus(transient_win);
}
+void BrowserWindow::HandleTransientWindowModalityChange(Window* transient_win) {
+ transients_->HandleWindowModalityChange(transient_win);
+}
+
void BrowserWindow::SetTransientWindowVisibility(bool shown) {
if (shown)
transients_->Show();
diff --git a/layout2/browser_window.h b/layout2/browser_window.h
index 1a08b62..5bf28d9 100644
--- a/layout2/browser_window.h
+++ b/layout2/browser_window.h
@@ -82,9 +82,13 @@
// Specify that a previously-registered transient window should get the input
// focus the next time that TakeFocus() is called. NULL can be passed to make
- // the browser window itself get the focus.
+ // the browser window itself get the focus (assuming that no modal transients
+ // are mapped).
void SetPreferredTransientWindowToFocus(Window* transient_win);
+ // Handle one of this browser's transient windows' modality changing.
+ void HandleTransientWindowModalityChange(Window* transient_win);
+
// Show or hide all of this browser's transient windows.
void SetTransientWindowVisibility(bool shown);
diff --git a/layout2/layout_manager2.cc b/layout2/layout_manager2.cc
index 26da27b..e27f81c 100644
--- a/layout2/layout_manager2.cc
+++ b/layout2/layout_manager2.cc
@@ -1447,7 +1447,8 @@
new_state[wm_->GetXAtom(ATOM_NET_WM_STATE_MODAL)] = modal;
win->ChangeWmState(new_state);
- if (modal)
+ owning_browser->HandleTransientWindowModalityChange(win);
+ if (modal || owning_browser == GetActiveBrowserWindow())
FocusBrowserWindow(owning_browser);
}
diff --git a/layout2/layout_manager2_test.cc b/layout2/layout_manager2_test.cc
index c2b8f8d..8f06252 100644
--- a/layout2/layout_manager2_test.cc
+++ b/layout2/layout_manager2_test.cc
@@ -380,13 +380,29 @@
SendKey(xconn_->GetRootWindow(), kAltTabKeyCombo, kEventTime, kEventTime);
EXPECT_EQ(transient_xid2, xconn_->focused_xid());
- // Send an event asking for |transient_xid2| to be made non-modal.
- xconn_->InitClientMessageEvent(
- &event, transient_xid2, kWmStateAtom, 0, kWmStateModalAtom, 0, 0, 0);
- wm_->HandleEvent(&event);
- xconn_->InitPropertyNotifyEvent(&event, transient_xid2, kWmStateAtom);
- wm_->HandleEvent(&event);
+ // Send an event asking for |transient_xid3| to be made modal. It should be
+ // focused and raised on top of |transient_xid2| (see
+ // http://crosbug.com/17642).
+ SendWmStateMessage(transient_xid3, kWmStateModalAtom, true);
+ EXPECT_EQ(transient_xid3, xconn_->focused_xid());
+ EXPECT_TRUE(
+ TestWindowIsAboveOtherWindow(wm_->GetWindowOrDie(transient_xid3),
+ wm_->GetWindowOrDie(transient_xid2)));
+
+ // Send an event asking for |transient_xid3| to be made non-modal.
+ // Since |transient_xid2| is still modal, it should now be focused and on top.
+ SendWmStateMessage(transient_xid3, kWmStateModalAtom, false);
EXPECT_EQ(transient_xid2, xconn_->focused_xid());
+ EXPECT_TRUE(
+ TestWindowIsAboveOtherWindow(wm_->GetWindowOrDie(transient_xid2),
+ wm_->GetWindowOrDie(transient_xid3)));
+
+ // Now make |transient_xid2| non-modal as well.
+ SendWmStateMessage(transient_xid2, kWmStateModalAtom, false);
+ EXPECT_EQ(transient_xid2, xconn_->focused_xid());
+ EXPECT_TRUE(
+ TestWindowIsAboveOtherWindow(wm_->GetWindowOrDie(transient_xid2),
+ wm_->GetWindowOrDie(transient_xid3)));
// Click on |xid2| and check that it takes the focus.
xconn_->set_pointer_grab_xid(xid2);
@@ -408,11 +424,7 @@
// Send a message asking for |transient_xid| to be made modal.
// We should focus it and automatically switch to |xid|.
- xconn_->InitClientMessageEvent(
- &event, transient_xid, kWmStateAtom, 1, kWmStateModalAtom, 0, 0, 0);
- wm_->HandleEvent(&event);
- xconn_->InitPropertyNotifyEvent(&event, transient_xid, kWmStateAtom);
- wm_->HandleEvent(&event);
+ SendWmStateMessage(transient_xid, kWmStateModalAtom, true);
EXPECT_EQ(transient_xid, xconn_->focused_xid());
EXPECT_EQ(xconn_->root_bounds(), xconn_->GetWindowInfoOrDie(xid)->bounds);
EXPECT_EQ(xconn_->root_bounds(), GetCompositedWindowBounds(xid));
@@ -478,10 +490,7 @@
// Request that the window be made fullscreen. Check that the fullscreen hint
// is set on it and that it's restacked.
- XEvent event;
- xconn_->InitClientMessageEvent(
- &event, xid, kWmStateAtom, 1, kWmStateFullscreenAtom, 0, 0, 0);
- wm_->HandleEvent(&event);
+ SendWmStateMessage(xid, kWmStateFullscreenAtom, true);
EXPECT_EQ(xid, xconn_->focused_xid());
EXPECT_EQ(wm_->root_bounds(), info->bounds);
EXPECT_EQ(wm_->root_bounds(), GetCompositedWindowBounds(xid));
@@ -522,17 +531,13 @@
IntArrayPropertyContains(xid3, kWmStateAtom, kWmStateFullscreenAtom));
// Send a message to make the first window fullscreen again...
- xconn_->InitClientMessageEvent(
- &event, xid, kWmStateAtom, 1, kWmStateFullscreenAtom, 0, 0, 0);
- wm_->HandleEvent(&event);
+ SendWmStateMessage(xid, kWmStateFullscreenAtom, true);
EXPECT_TRUE(WindowIsInLayer(win, StackingManager::LAYER_FULLSCREEN_WINDOW));
EXPECT_TRUE(
IntArrayPropertyContains(xid, kWmStateAtom, kWmStateFullscreenAtom));
// ... and then send a message to make it not fullscreen.
- xconn_->InitClientMessageEvent(
- &event, xid, kWmStateAtom, 0, kWmStateFullscreenAtom, 0, 0, 0);
- wm_->HandleEvent(&event);
+ SendWmStateMessage(xid, kWmStateFullscreenAtom, false);
EXPECT_TRUE(
WindowIsInLayer(win, StackingManager::LAYER_ACTIVE_BROWSER_WINDOW));
EXPECT_FALSE(
@@ -540,18 +545,15 @@
// Creating a panel (which will take the focus) should also unfullscreen the
// browser.
- xconn_->InitClientMessageEvent(
- &event, xid, kWmStateAtom, 1, kWmStateFullscreenAtom, 0, 0, 0);
- wm_->HandleEvent(&event);
+ SendWmStateMessage(xid, kWmStateFullscreenAtom, true);
Panel* panel = CreatePanel(200, 20, 400);
ASSERT_EQ(panel->content_xid(), xconn_->focused_xid());
EXPECT_FALSE(
IntArrayPropertyContains(xid, kWmStateAtom, kWmStateFullscreenAtom));
// Check that we don't crash when a fullscreen window is unmapped.
- xconn_->InitClientMessageEvent(
- &event, xid, kWmStateAtom, 1, kWmStateFullscreenAtom, 0, 0, 0);
- wm_->HandleEvent(&event);
+ SendWmStateMessage(xid, kWmStateFullscreenAtom, true);
+ XEvent event;
xconn_->InitUnmapEvent(&event, xid);
wm_->HandleEvent(&event);
}
diff --git a/test_lib.cc b/test_lib.cc
index 8b275f5..cb1555c 100644
--- a/test_lib.cc
+++ b/test_lib.cc
@@ -401,6 +401,20 @@
wm_->HandleEvent(&event);
}
+void BasicWindowManagerTest::SendWmStateMessage(XWindow xid,
+ XAtom state_atom,
+ bool set) {
+ const XAtom prop_atom = xconn_->GetAtomOrDie("_NET_WM_STATE");
+ XEvent event;
+ xconn_->InitClientMessageEvent(
+ &event, xid, prop_atom, set, state_atom, 0, 0, 0);
+ wm_->HandleEvent(&event);
+ // Assuming that the window manager changed the property as requested, tell it
+ // to take another look.
+ xconn_->InitPropertyNotifyEvent(&event, xid, prop_atom);
+ wm_->HandleEvent(&event);
+}
+
void BasicWindowManagerTest::SendConfigureNotifyEvent(XWindow xid) {
XEvent event;
xconn_->InitConfigureNotifyEvent(&event, xid);
diff --git a/test_lib.h b/test_lib.h
index 0e62feb..48abead 100644
--- a/test_lib.h
+++ b/test_lib.h
@@ -177,6 +177,10 @@
// window.
void SendActiveWindowMessage(XWindow xid);
+ // Send a message to the window manager asking it to set or unset |state_atom|
+ // (e.g. _NET_WM_STATE_MODAL) in |xid|'s _NET_WM_STATE property.
+ void SendWmStateMessage(XWindow xid, XAtom state_atom, bool set);
+
// Initialize a ConfigureNotify event for |xid|'s current bounds and stacking
// and pass it to WindowManager::HandleEvent().
void SendConfigureNotifyEvent(XWindow xid);
diff --git a/transient_window_collection.cc b/transient_window_collection.cc
index b51a5e9..bd01316 100644
--- a/transient_window_collection.cc
+++ b/transient_window_collection.cc
@@ -180,6 +180,23 @@
}
}
+void TransientWindowCollection::HandleWindowModalityChange(
+ Window* transient_win) {
+ if (transient_win->wm_state_modal()) {
+ // If this window just became modal, focus it.
+ transient_to_focus_ = GetTransientWindow(*transient_win);
+ } else {
+ // Otherwise, if it's not modal but we were previously preferring it, make
+ // sure that there aren't any other transient windows that we should focus
+ // instead.
+ if (transient_to_focus_ && transient_to_focus_->win == transient_win) {
+ TransientWindow* preferred_transient = FindTransientWindowToFocus();
+ if (preferred_transient && preferred_transient->win->wm_state_modal())
+ transient_to_focus_ = preferred_transient;
+ }
+ }
+}
+
void TransientWindowCollection::ConfigureAllWindowsRelativeToOwner(
int anim_ms) {
for (TransientWindowMap::iterator it = transients_.begin();
diff --git a/transient_window_collection.h b/transient_window_collection.h
index a46b67e..8eb7844 100644
--- a/transient_window_collection.h
+++ b/transient_window_collection.h
@@ -97,6 +97,12 @@
// This should be called in response to the window being unmapped.
void RemoveWindow(Window* transient_win);
+ // Handle a change in |transient_win|'s modality.
+ // If it's modal, then we set it as the preferred window to focus; if it's
+ // non-modal, then we check if there's another still-modal window that we
+ // should be focusing instead.
+ void HandleWindowModalityChange(Window* transient_win);
+
// Update all transient windows' positions and scales based on the owner
// window's position and scale.
void ConfigureAllWindowsRelativeToOwner(int anim_ms);
