New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 744923 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Evaluate if ui::ws::EventDispatcher::SetMousePointerDisplayLocation() needs to process mouse

Project Member Reported by sky@chromium.org, Jul 17 2017

Issue description

Currently SetMousePointerDisplayLocation() updates the cursor location, but it doesn't process the call in the same way as if a MouseEvent were generated (clients and internal state aren't updated). This means clients won't get a MouseEvent and some internal processing may not happen. I'm not sure what the expectation of this function is.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 18 2017

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

commit 3ed67525929d3e00bab8da937e402aad43344381
Author: Scott Violet <sky@chromium.org>
Date: Tue Jul 18 00:05:08 2017

chromeos/mus: remove bogus DCHECK form EventDispatcher

At the time I wrote SetMousePointerDisplayLocation() it was assumed
SetMousePointerDisplayLocation() was called right after Reset(), which
meant pointer_targets_ was empty. Now that
SetMousePointerDisplayLocation() may be called by client code, that
assumption is no longer valid and the DCHECK is out date.

BUG= 744923 
TEST=none

Change-Id: I19af55eab935bc05aa025839bd0a297ad585ea32
Reviewed-on: https://chromium-review.googlesource.com/574677
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487297}
[modify] https://crrev.com/3ed67525929d3e00bab8da937e402aad43344381/services/ui/ws/event_dispatcher.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 10 2017

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

commit 5d0ea383e300bdbd091e262c573f4de49cb3c603
Author: Elliot Glaysher <erg@chromium.org>
Date: Thu Aug 10 22:49:26 2017

Have SetMousePointerDisplayLocation() build synthetic events.

Previously, we just changed the location of the mouse pointer when we
set the mouse pointer display location. This meant that internal caches
weren't being updated, and windows weren't receiving mouse move/exit
events when this happened. This patch changes direct setting to building
a synthetic mouse move event and putting it through normal event dispatch.

      EventDispatcherTest.SetMousePointerDisplayLocationWith(out)Flags

Bug:  744923 
Change-Id: Ib8625db3dd1955c9ec41512176e71333dbfcf6fb
Test: WindowTreeTest.TestWindowManagerSettingCursorLocation,
Reviewed-on: https://chromium-review.googlesource.com/609305
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493577}
[modify] https://crrev.com/5d0ea383e300bdbd091e262c573f4de49cb3c603/services/ui/ws/event_dispatcher.cc
[modify] https://crrev.com/5d0ea383e300bdbd091e262c573f4de49cb3c603/services/ui/ws/event_dispatcher.h
[modify] https://crrev.com/5d0ea383e300bdbd091e262c573f4de49cb3c603/services/ui/ws/event_dispatcher_unittest.cc

Comment 3 by e...@chromium.org, Aug 10 2017

Status: Fixed (was: Assigned)
Hmm. Gerrit really mangled those multiline Test: lines...

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

Status: Archived (was: Fixed)

Sign in to add a comment