New issue
Advanced search Search tips

Issue 664617 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 659155



Sign in to add a comment

WindowTree should likely pass root window to OnWindowInpuEvent

Project Member Reported by sky@chromium.org, Nov 11 2016

Issue description

If the client deletes a window at the same time as the server sends an event to the window than aura::WindowTreeClient can't find the target window and drops the event on the floor. This is going to be racy and likely result in subtle event issues.

We should likely add the id of the root window to OnWindowInputEvent() so that if WindowTreeClient can't find the window it falls back to dispatching using the root window.
 

Comment 1 by sky@chromium.org, Nov 11 2016

Cc: sadrul@chromium.org

Comment 2 by sky@chromium.org, Feb 13 2017

Labels: mustash-1
I plumbed through the display_id here: https://codereview.chromium.org/2685883003 , so it should just be matter of making WindowTreeClient fallback to the display id if the window can't be found.

Comment 3 by sadrul@chromium.org, Feb 13 2017

Owner: riajiang@chromium.org
Status: Assigned (was: Untriaged)

Comment 4 by sadrul@chromium.org, Feb 23 2017

Cc: sky@chromium.org
> WindowTreeClient can't find the window it falls back to dispatching using the
> root window.

I think this can work only for key-events, but not for located events, because the event-location will be in a coord-space of a window the client no longer knows about. Trying to dispatch the located event anyway can lead to unintended interaction (e.g. accidental click on some button). It'd be better to drop the located events in this scenario.

Comment 5 by sky@chromium.org, Feb 23 2017

Can't we use the root coordinates, which the client has?

If we don't dispatch a pointer event don't we risk leaving the client in a confusing state? Say a down without an up? Or a missing enter?
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8b7d84c49f08b6e50de6da191264401c074c5a0

commit d8b7d84c49f08b6e50de6da191264401c074c5a0
Author: riajiang <riajiang@chromium.org>
Date: Sat Mar 04 23:08:10 2017

Start with event_target in for loop to avoid GetEventTargeter crash.

Came across this while working on the CL for dispatching event to root
window if target window has already been deleted (
https://codereview.chromium.org/2696873002). If the target window we
are starting from is already the root window in that WindowTreeHost,
starting with the parent target (in this case aura::Env) for that window
would cause crash when trying to access its event targeter. Change it to
start with the event_target itself.

BUG= 687700 ,  664617 
TEST=aura_unittests

Review-Url: https://codereview.chromium.org/2730903005
Cr-Commit-Position: refs/heads/master@{#454780}

[modify] https://crrev.com/d8b7d84c49f08b6e50de6da191264401c074c5a0/ui/aura/mus/window_tree_client_unittest.cc
[modify] https://crrev.com/d8b7d84c49f08b6e50de6da191264401c074c5a0/ui/aura/window_event_dispatcher.cc

Comment 7 by sky@chromium.org, Mar 6 2017

Status: Fixed (was: Assigned)

Comment 8 by sky@chromium.org, Mar 6 2017

Status: Started (was: Fixed)
Project Member

Comment 9 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09e6e887d03585ba7080c3cbcd5222d119502fff

commit 09e6e887d03585ba7080c3cbcd5222d119502fff
Author: riajiang <riajiang@chromium.org>
Date: Wed May 03 04:35:49 2017

Update event states in Env when target window has been destroyed.

Based on discussions in https://codereview.chromium.org/2696873002, in
WindowTreeClient::OnWindowInputEvent(), if the target window provided by
window server has already been destroyed by the time client-lib got the
event, we should use the event to update global states related to
MouseEvent and TouchEvent kept in Env() and not dispatch the event to
any window.

BUG= 664617 
TEST=aura_unittests

Review-Url: https://codereview.chromium.org/2779093004
Cr-Commit-Position: refs/heads/master@{#468893}

[modify] https://crrev.com/09e6e887d03585ba7080c3cbcd5222d119502fff/ui/aura/env_input_state_controller.cc
[modify] https://crrev.com/09e6e887d03585ba7080c3cbcd5222d119502fff/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/09e6e887d03585ba7080c3cbcd5222d119502fff/ui/aura/mus/window_tree_client_unittest.cc

Status: Fixed (was: Started)
Labels: VerifyIn-61

Comment 12 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment