Skip to content

Commit

Permalink
Fix "modal isn't modal in multi displays" issue
Browse files Browse the repository at this point in the history
This is pretty bad bug because you can do almost everything while system modal dialog is open.

* Generalized the logic to check if the window is above blocking container.

I believe I kept the original behavior otherwise. Please let me know if you noticed something different.

BUG=620406
TEST=manual, covered by tests

Review-Url: https://codereview.chromium.org/2070163002
Cr-Commit-Position: refs/heads/master@{#400589}
  • Loading branch information
mitoshima authored and Commit bot committed Jun 18, 2016
1 parent 6c3ee78 commit 9a61ecf
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 27 deletions.
91 changes: 91 additions & 0 deletions ash/root_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,53 @@ void ReparentAllWindows(aura::Window* src, aura::Window* dst) {
}
}

bool IsWindowAboveContainer(aura::Window* window,
aura::Window* blocking_container) {
std::vector<aura::Window*> target_path;
std::vector<aura::Window*> blocking_path;

while (window) {
target_path.push_back(window);
window = window->parent();
}

while (blocking_container) {
blocking_path.push_back(blocking_container);
blocking_container = blocking_container->parent();
}

// The root window is put at the end so that we compare windows at
// the same depth.
while (!blocking_path.empty()) {
if (target_path.empty())
return false;

aura::Window* target = target_path.back();
target_path.pop_back();
aura::Window* blocking = blocking_path.back();
blocking_path.pop_back();

// Still on the same path, continue.
if (target == blocking)
continue;

// This can happen only if unparented window is passed because
// first element must be the same root.
if (!target->parent() || !blocking->parent())
return false;

aura::Window* common_parent = target->parent();
DCHECK_EQ(common_parent, blocking->parent());
aura::Window::Windows windows = common_parent->children();
auto blocking_iter = std::find(windows.begin(), windows.end(), blocking);
// If the target window is above blocking window, the window can handle
// events.
return std::find(blocking_iter, windows.end(), target) != windows.end();
}

return true;
}

// A window delegate which does nothing. Used to create a window that
// is a event target, but do nothing.
class EmptyWindowDelegate : public aura::WindowDelegate {
Expand Down Expand Up @@ -363,6 +410,50 @@ void RootWindowController::Shutdown() {
aura::client::SetScreenPositionClient(root_window, NULL);
}

bool RootWindowController::CanWindowReceiveEvents(aura::Window* window) {
if (GetRootWindow() != window->GetRootWindow())
return false;

// Always allow events to fall through to the virtual keyboard even if
// displaying a system modal dialog.
if (IsVirtualKeyboardWindow(window))
return true;

aura::Window* blocking_container = nullptr;

int modal_container_id = 0;
if (Shell::GetInstance()->session_state_delegate()->IsUserSessionBlocked()) {
blocking_container =
GetContainer(kShellWindowId_LockScreenContainersContainer);
modal_container_id = kShellWindowId_LockSystemModalContainer;
} else {
modal_container_id = kShellWindowId_SystemModalContainer;
}
aura::Window* modal_container = GetContainer(modal_container_id);
SystemModalContainerLayoutManager* modal_layout_manager = nullptr;
modal_layout_manager = static_cast<SystemModalContainerLayoutManager*>(
modal_container->layout_manager());

if (modal_layout_manager->has_modal_background())
blocking_container = modal_container;
else
modal_container = nullptr; // Don't check modal dialogs.

// In normal session.
if (!blocking_container)
return true;

if (!IsWindowAboveContainer(window, blocking_container))
return false;

// If the window is in the target modal container, only allow the top most
// one.
if (modal_container && modal_container->Contains(window))
return modal_layout_manager->IsPartOfActiveModalWindow(window);

return true;
}

SystemModalContainerLayoutManager*
RootWindowController::GetSystemModalLayoutManager(aura::Window* window) {
aura::Window* modal_container = NULL;
Expand Down
3 changes: 3 additions & 0 deletions ash/root_window_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ class ASH_EXPORT RootWindowController : public ShellObserver {
void ShowContextMenu(const gfx::Point& location_in_screen,
ui::MenuSourceType source_type);

// True if the window can receive events on this root window.
bool CanWindowReceiveEvents(aura::Window* window);

// Returns the layout-manager for the appropriate modal-container. If the
// window is inside the lockscreen modal container, then the layout manager
// for that is returned. Otherwise the layout manager for the default modal
Expand Down
12 changes: 2 additions & 10 deletions ash/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1178,18 +1178,10 @@ void Shell::InitRootWindow(aura::Window* root_window) {

bool Shell::CanWindowReceiveEvents(aura::Window* window) {
RootWindowControllerList controllers = GetAllRootWindowControllers();
for (RootWindowControllerList::iterator iter = controllers.begin();
iter != controllers.end(); ++iter) {
SystemModalContainerLayoutManager* layout_manager =
(*iter)->GetSystemModalLayoutManager(window);
if (layout_manager && layout_manager->CanWindowReceiveEvents(window))
return true;
// Allow events to fall through to the virtual keyboard even if displaying
// a system modal dialog.
if ((*iter)->IsVirtualKeyboardWindow(window))
for (RootWindowController* controller : controllers) {
if (controller->CanWindowReceiveEvents(window))
return true;
}

return false;
}

Expand Down
2 changes: 2 additions & 0 deletions ash/wm/event_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ EventClientImpl::~EventClientImpl() {

bool EventClientImpl::CanProcessEventsWithinSubtree(
const aura::Window* window) const {
// TODO(oshima): Migrate this logic to Shell::CanWindowReceieveEvents and
// remove this.
const aura::Window* root_window = window ? window->GetRootWindow() : NULL;
if (!root_window ||
!Shell::GetInstance()->session_state_delegate()->IsUserSessionBlocked()) {
Expand Down
17 changes: 2 additions & 15 deletions ash/wm/system_modal_container_layout_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,9 @@ void SystemModalContainerLayoutManager::OnKeyboardBoundsChanging(
PositionDialogsAfterWorkAreaResize();
}

bool SystemModalContainerLayoutManager::CanWindowReceiveEvents(
bool SystemModalContainerLayoutManager::IsPartOfActiveModalWindow(
aura::Window* window) {
// We could get when we're at lock screen and there is modal window at
// system modal window layer which added event filter.
// Now this lock modal windows layer layout manager should not block events
// for windows at lock layer.
// See SystemModalContainerLayoutManagerTest.EventFocusContainers and
// http://crbug.com/157469
if (modal_windows_.empty())
return true;
// This container can not handle events if the screen is locked and it is not
// above the lock screen layer (crbug.com/110920).
if (Shell::GetInstance()->session_state_delegate()->IsUserSessionBlocked() &&
container_->id() < ash::kShellWindowId_LockScreenContainer)
return true;
return wm::GetActivatableWindow(window) == modal_window();
return modal_window() && wm::GetActivatableWindow(window) == modal_window();
}

bool SystemModalContainerLayoutManager::ActivateNextModalWindow() {
Expand Down
5 changes: 3 additions & 2 deletions ash/wm/system_modal_container_layout_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ class ASH_EXPORT SystemModalContainerLayoutManager
// Overridden from keyboard::KeyboardControllerObserver:
void OnKeyboardBoundsChanging(const gfx::Rect& new_bounds) override;

// Can a given |window| receive and handle input events?
bool CanWindowReceiveEvents(aura::Window* window);
// True if the window is either contained by the top most modal window,
// or contained by its transient children.
bool IsPartOfActiveModalWindow(aura::Window* window);

// Activates next modal window if any. Returns false if there
// are no more modal windows in this layout manager.
Expand Down
55 changes: 55 additions & 0 deletions ash/wm/system_modal_container_layout_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "ash/wm/system_modal_container_layout_manager.h"

#include <memory>

#include "ash/common/session/session_state_delegate.h"
#include "ash/common/shell_window_ids.h"
#include "ash/common/wm_shell.h"
Expand All @@ -15,6 +17,7 @@
#include "base/compiler_specific.h"
#include "base/run_loop.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/test/test_window_delegate.h"
#include "ui/aura/window.h"
#include "ui/aura/window_event_dispatcher.h"
#include "ui/compositor/layer.h"
Expand Down Expand Up @@ -676,5 +679,57 @@ TEST_F(SystemModalContainerLayoutManagerTest, UpdateModalType) {
EXPECT_FALSE(WmShell::Get()->IsSystemModalWindowOpen());
}

namespace {

class InputTestDelegate : public aura::test::TestWindowDelegate {
public:
InputTestDelegate() {}
~InputTestDelegate() override {}

// ui::EventHandler:
void OnMouseEvent(ui::MouseEvent* event) override { mouse_event_count_++; }

int mouse_event_count() const { return mouse_event_count_; }

private:
int mouse_event_count_ = 0;

DISALLOW_COPY_AND_ASSIGN(InputTestDelegate);
};

} // namespace

// Make sure that events are properly blocked in multi displays environment.
TEST_F(SystemModalContainerLayoutManagerTest, BlockEvent) {
UpdateDisplay("500x500, 500x500");
InputTestDelegate delegate;
std::unique_ptr<aura::Window> window(CreateTestWindowInShellWithDelegate(
&delegate, 0, gfx::Rect(0, 0, 100, 100)));
window->Show();

ui::test::EventGenerator event_generator(Shell::GetPrimaryRootWindow(),
window.get());
event_generator.ClickLeftButton();
EXPECT_EQ(2, delegate.mouse_event_count());

views::Widget* widget = views::Widget::CreateWindowWithContextAndBounds(
new TestWindow(true), Shell::GetPrimaryRootWindow(),
gfx::Rect(200, 200, 100, 100));
widget->Show();
EXPECT_TRUE(WmShell::Get()->IsSystemModalWindowOpen());

// Events should be blocked.
event_generator.ClickLeftButton();
EXPECT_EQ(2, delegate.mouse_event_count());
widget->Close();

EXPECT_FALSE(WmShell::Get()->IsSystemModalWindowOpen());
event_generator.ClickLeftButton();
EXPECT_EQ(4, delegate.mouse_event_count());

// We need to delete window here because delegate is on the stack.
window.reset();
}

} // namespace test
} // namespace ash

0 comments on commit 9a61ecf

Please sign in to comment.