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

Issue 756300 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

We are doing hit-testing too often

Project Member Reported by sadrul@chromium.org, Aug 17 2017

Issue description

It looks like we are doing hit-testing too often. For example, for mouse press+drag, we should do a hit-test only for the mouse-press, but it looks like we end up doing hit-testing for all the drag events too.

The same happens for touch-scroll: we should hit-test only for touch-press, but currently, it looks like we do hit-testing for touch-press, and all subsequent touch-move events too.

Ria: can you please investigate?
 

Comment 1 by sky@chromium.org, Aug 28 2017

Owner: sky@chromium.org
Status: Started (was: Assigned)
I'm going to take this as it'll make some other changes I'm doing easier.
Project Member

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

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

commit 694135c1762f6d74f699661e834184fddaa5f752
Author: Scott Violet <sky@chromium.org>
Date: Tue Aug 29 21:00:55 2017

chromeos: minor cleanup of EventDispatcher

This should contain no functional difference to existing code. I'm
doing it as a precursor to not always calling EventTargeter. When that
happens DeepestWindow won't be supplied (it'll become a pointer). I've
also tried to reduce the size of ProcessPointerEventOnFoundTarget() by
moving processing to secondary functions in hopes of making it easier
to maintain and more obvious which state is needed.

BUG= 756300 
TEST=covered by tests

Change-Id: Ied2e2da6090221a19dd03951572847a365c29a24
Reviewed-on: https://chromium-review.googlesource.com/640291
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498226}
[modify] https://crrev.com/694135c1762f6d74f699661e834184fddaa5f752/services/ui/ws/event_dispatcher.cc
[modify] https://crrev.com/694135c1762f6d74f699661e834184fddaa5f752/services/ui/ws/event_dispatcher.h
[modify] https://crrev.com/694135c1762f6d74f699661e834184fddaa5f752/services/ui/ws/event_targeter.cc
[modify] https://crrev.com/694135c1762f6d74f699661e834184fddaa5f752/services/ui/ws/event_targeter.h

Cc: riajiang@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 30 2017

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

commit b91b9caf6853e0263aa572fb5888b7667a6ad256
Author: Scott Violet <sky@chromium.org>
Date: Wed Aug 30 23:55:23 2017

chromeos: make EventDispatcher only query EventTargeter when necessary

Prior to this patch all processed events triggered querying the
EventTargeter. There are many types of events that don't need to query
the EventTargeter. This patch makes it so we only query the
EventTargeter when necessary.

BUG= 756300 
TEST=covered by tests

Change-Id: Idc4db0b611738dac5d7f5fef4cf3074fe692166e
Reviewed-on: https://chromium-review.googlesource.com/642118
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498676}
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/event_dispatcher.cc
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/event_dispatcher.h
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/event_dispatcher_unittest.cc
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/event_targeter.cc
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/event_targeter.h
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/test_utils.h
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/window_manager_state.cc
[modify] https://crrev.com/b91b9caf6853e0263aa572fb5888b7667a6ad256/services/ui/ws/window_manager_state_unittest.cc

Comment 5 by sky@chromium.org, Aug 31 2017

Status: Fixed (was: Started)
Components: -MUS Internals>Services>WindowService

Sign in to add a comment