Skip to content

Commit

Permalink
EventDispatcher shouldn't always cancel on a hierarchy change
Browse files Browse the repository at this point in the history
Currently when a window is moved to a different parent we cancel
events. This is problematic for window dragging as we want to reparent
the window to ensure it's stacked on top of other windows.

The fix here isn't ideal, but I'm going to address the bigger issue as
part of 613646.

BUG=613646
TEST=covered by test
R=jonross@chromium.org

Review-Url: https://codereview.chromium.org/1998323002
Cr-Commit-Position: refs/heads/master@{#395145}
  • Loading branch information
sky authored and Commit bot committed May 20, 2016
1 parent 54fbd40 commit b863e7e
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 1 deletion.
13 changes: 12 additions & 1 deletion components/mus/ws/event_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,18 @@ Accelerator* EventDispatcher::FindAccelerator(
void EventDispatcher::OnWillChangeWindowHierarchy(ServerWindow* window,
ServerWindow* new_parent,
ServerWindow* old_parent) {
CancelPointerEventsToTarget(window);
// TODO(sky): moving to a different root likely needs to transfer capture.
// TODO(sky): this isn't quite right, I think the logic should be (assuming
// moving in same root and still drawn):
// . if there is capture and window is still in the same root, continue
// sending to it.
// . if there isn't capture, then reevaluate each of the pointer targets
// sending exit as necessary.
// http://crbug.com/613646 .
if (!new_parent || !new_parent->IsDrawn() ||
new_parent->GetRoot() != old_parent->GetRoot()) {
CancelPointerEventsToTarget(window);
}
}

void EventDispatcher::OnWindowVisibilityChanged(ServerWindow* window) {
Expand Down
34 changes: 34 additions & 0 deletions components/mus/ws/event_dispatcher_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,40 @@ TEST_F(EventDispatcherTest, ModalWindowMultipleSystemModals) {
EXPECT_EQ(nullptr, GetActiveSystemModalWindow());
}

TEST_F(EventDispatcherTest, CaptureNotResetOnParentChange) {
std::unique_ptr<ServerWindow> w1 = CreateChildWindow(WindowId(1, 3));
DisableHitTest(w1.get());
std::unique_ptr<ServerWindow> w11 =
CreateChildWindowWithParent(WindowId(1, 4), w1.get());
std::unique_ptr<ServerWindow> w2 = CreateChildWindow(WindowId(1, 5));
DisableHitTest(w2.get());

root_window()->SetBounds(gfx::Rect(0, 0, 100, 100));
w1->SetBounds(gfx::Rect(0, 0, 100, 100));
w11->SetBounds(gfx::Rect(10, 10, 10, 10));
w2->SetBounds(gfx::Rect(0, 0, 100, 100));

// Send event that is over |w11|.
const ui::PointerEvent mouse_pressed(ui::MouseEvent(
ui::ET_MOUSE_PRESSED, gfx::Point(15, 15), gfx::Point(15, 15),
base::TimeDelta(), ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
event_dispatcher()->ProcessEvent(mouse_pressed);
event_dispatcher()->SetCaptureWindow(w11.get(), false);

std::unique_ptr<DispatchedEventDetails> details =
test_event_dispatcher_delegate()->GetAndAdvanceDispatchedEventDetails();
ASSERT_TRUE(details);
EXPECT_EQ(w11.get(), details->window);
EXPECT_FALSE(details->in_nonclient_area);

// Move |w11| to |w2| and verify the mouse is still down, and |w11| has
// capture.
w2->Add(w11.get());
EXPECT_TRUE(IsMouseButtonDown());
EXPECT_EQ(w11.get(),
EventDispatcherTestApi(event_dispatcher()).capture_window());
}

} // namespace test
} // namespace ws
} // namespace mus
10 changes: 10 additions & 0 deletions components/mus/ws/server_window_surface_manager_test_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,21 @@ void ServerWindowSurfaceManagerTestApi::CreateEmptyDefaultSurface() {
manager_->type_to_surface_map_[mojom::SurfaceType::DEFAULT] = nullptr;
}

void ServerWindowSurfaceManagerTestApi::DestroyDefaultSurface() {
manager_->type_to_surface_map_.erase(mojom::SurfaceType::DEFAULT);
}

void EnableHitTest(ServerWindow* window) {
ServerWindowSurfaceManagerTestApi test_api(
window->GetOrCreateSurfaceManager());
test_api.CreateEmptyDefaultSurface();
}

void DisableHitTest(ServerWindow* window) {
ServerWindowSurfaceManagerTestApi test_api(
window->GetOrCreateSurfaceManager());
test_api.DestroyDefaultSurface();
}

} // namespace ws
} // namespace mus
2 changes: 2 additions & 0 deletions components/mus/ws/server_window_surface_manager_test_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class ServerWindowSurfaceManagerTestApi {
~ServerWindowSurfaceManagerTestApi();

void CreateEmptyDefaultSurface();
void DestroyDefaultSurface();

private:
ServerWindowSurfaceManager* manager_;
Expand All @@ -30,6 +31,7 @@ class ServerWindowSurfaceManagerTestApi {

// Use to make |window| a target for events.
void EnableHitTest(ServerWindow* window);
void DisableHitTest(ServerWindow* window);

} // namespace ws
} // namespace mus
Expand Down
1 change: 1 addition & 0 deletions components/mus/ws/test_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ class EventDispatcherTestApi {
ModalWindowController* modal_window_controller() const {
return &ed_->modal_window_controller_;
}
ServerWindow* capture_window() { return ed_->capture_window_; }

private:
EventDispatcher* ed_;
Expand Down

0 comments on commit b863e7e

Please sign in to comment.