New issue
Advanced search Search tips

Issue 878836 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 814675



Sign in to add a comment

Wire up WindowTargeter customization for mash

Project Member Reported by sky@chromium.org, Aug 29

Issue description

Chrome code makes use of the following window targeter customizations:
. changing the mouse/touch insets.
. hit test rects.

The mouse/touch insets are plumbed through via EasyResizeWindowTargeter, but not the hit test rects.

The rounded omnibox makes use of a custom window targeter, which is why it's broke in mash now.
 
Components: Internals>Services>WindowService Internals>Aura
Blocking: 814675
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 30

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

commit db1f068d738086878b733bb3242e3502b727e4de
Author: Scott Violet <sky@chromium.org>
Date: Thu Aug 30 19:19:46 2018

exo: removes unnecessary override of GetExtraHitTestShapeRects

GetExtraHitTestShapeRects() is called solely from
WindowTargeter::EventLocationInsideBounds(). As CustomTargeter completely
overrides EventLocationInsideBounds() there is no need to also override
GetExtraHitTestShapeRects(). This removes the unnecessary override.

I'm also removing the implementation of Surface::GetHitTestShapeRects() to
the test, as that is now the only place that needs to call it.

BUG= 878836 
TEST=covered by tests

Change-Id: I75569904bdfe5bd4d4384436488e4cc0b224ee3d
Reviewed-on: https://chromium-review.googlesource.com/1196913
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587696}
[modify] https://crrev.com/db1f068d738086878b733bb3242e3502b727e4de/components/exo/client_controlled_shell_surface_unittest.cc
[modify] https://crrev.com/db1f068d738086878b733bb3242e3502b727e4de/components/exo/surface.cc
[modify] https://crrev.com/db1f068d738086878b733bb3242e3502b727e4de/components/exo/surface.h
[modify] https://crrev.com/db1f068d738086878b733bb3242e3502b727e4de/components/exo/surface_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 30

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

commit ec3b3b688c4c3bdae474d718cb51553c95d6dd2c
Author: Scott Violet <sky@chromium.org>
Date: Thu Aug 30 20:06:30 2018

aura: Makes Window::SetEventTargeter() take a WindowTargeter

I'm doing this as a precursor to having the WindowTarget know
the Window it's associated with. We could certainly support
non-WindowTargeters, but I don't think that really buys us
anything.

BUG= 878836 
TEST=covered by tests

Change-Id: Iabe32319ba28796d4b90e967617bc4f95ff5581e
Reviewed-on: https://chromium-review.googlesource.com/1195666
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587718}
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ash/frame/wide_frame_view.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ash/host/ash_window_tree_host_platform.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/chrome/browser/ui/views/apps/shaped_app_window_targeter_unittest.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/chromecast/graphics/cast_window_manager_aura.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/components/exo/shell_surface_base.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/components/exo/surface.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/BUILD.gn
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/hit_test_data_provider_aura.cc
[add] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/null_window_targeter.cc
[add] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/null_window_targeter.h
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/scoped_window_targeter.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/scoped_window_targeter.h
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/test/aura_test_helper.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/window.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/window.h
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/window_event_dispatcher_unittest.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/aura/window_targeter_unittest.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/events/null_event_targeter.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/views/controls/menu/menu_controller_unittest.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/views/touchui/touch_selection_controller_impl.cc
[modify] https://crrev.com/ec3b3b688c4c3bdae474d718cb51553c95d6dd2c/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc

Status: Fixed (was: Started)
I've wired up enough of the windowtargeter for mus. Shapes aren't there yet, but that should be fixed once 879308 is addressed.
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 5

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

commit aca0bd9bba2f7a8e6754a09e3022a7862951979c
Author: Scott Violet <sky@chromium.org>
Date: Tue Sep 04 20:14:24 2018

aura: makes WindowTargeter know the window it is installed on

WindowTargeter can only be associated with one Window at a time, so make
the WindowTargeter know the Window it is associated with.

I'm doing this as a precursor to consolidating logic into WindowTargeter that
is currently in some subclasses. The reason to consolidate into WindowTargeter
is to get the various uses of subclasses to work with mus.

BUG= 878836 
TEST=covered by tests

Change-Id: I5f18f6d0726420817a1dbfb2c03fc1e5278effc5
Reviewed-on: https://chromium-review.googlesource.com/1204711
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588631}
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ash/shelf/shelf_window_targeter.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ash/wm/window_util.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/chrome/browser/ui/views/apps/app_window_easy_resize_window_targeter.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/chrome/browser/ui/views/apps/app_window_easy_resize_window_targeter.h
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/chrome/browser/ui/views/apps/shaped_app_window_targeter_unittest.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/services/ws/server_window_unittest.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ui/aura/window.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ui/aura/window_targeter.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ui/aura/window_targeter.h
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ui/wm/core/easy_resize_window_targeter.cc
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ui/wm/core/easy_resize_window_targeter.h
[modify] https://crrev.com/aca0bd9bba2f7a8e6754a09e3022a7862951979c/ui/wm/core/easy_resize_window_targeter_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 5

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

commit 427807c713fd39f89a353129af04f0cf65405efd
Author: Scott Violet <sky@chromium.org>
Date: Tue Sep 04 22:15:20 2018

aura: makes WindowTargeter's insets only apply to window installed on

I believe it was an oversight that made the insets apply to *all* descendants.

BUG= 878836 
TEST=covered by tests

Change-Id: I1acbcf2f56e8571136659cbc66007b33a6f29220
Reviewed-on: https://chromium-review.googlesource.com/1205095
Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588668}
[modify] https://crrev.com/427807c713fd39f89a353129af04f0cf65405efd/ui/aura/hit_test_data_provider_aura_unittest.cc
[modify] https://crrev.com/427807c713fd39f89a353129af04f0cf65405efd/ui/aura/window_targeter.cc
[modify] https://crrev.com/427807c713fd39f89a353129af04f0cf65405efd/ui/aura/window_targeter.h

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 5

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

commit 5f7e0b4646d2a2bd3354732f16c7938c9b2a442a
Author: Scott Violet <sky@chromium.org>
Date: Wed Sep 05 18:05:17 2018

aura: rename WindowTree::SetHitTestMask and update usage

The hit test mask code wasn't really wired up for the mask, and it turns out
aura's window targetting code doesn't actually use the mask. So, this makes the
code use the hit test insets, which *is* honored by window targetting and
updates the one usage in Chrome.

BUG= 878836 ,814675
TEST=covered by tests

Change-Id: I02e26b62f2847960a0c4d16eee44d653c6e45539
Reviewed-on: https://chromium-review.googlesource.com/1206172
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588935}
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/public/mojom/window_tree.mojom
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/server_window.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/server_window.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/window_tree.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/window_tree.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/window_tree_test_helper.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/window_tree_test_helper.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/services/ws/window_tree_unittest.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/mus/window_port_mus.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/mus/window_port_mus.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/mus/window_tree_host_mus_unittest.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/test/mus/test_window_tree.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/test/mus/test_window_tree.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/window_targeter.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/window_targeter.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/aura/window_targeter_unittest.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/views/mus/desktop_window_tree_host_mus.h
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/views/touchui/touch_selection_controller_impl.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/wm/BUILD.gn
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/wm/core/easy_resize_window_targeter.cc
[modify] https://crrev.com/5f7e0b4646d2a2bd3354732f16c7938c9b2a442a/ui/wm/core/easy_resize_window_targeter.h
[delete] https://crrev.com/d86eb25c3d298976a90e63742caa2764ac3bbf35/ui/wm/core/easy_resize_window_targeter_unittest.cc

Sign in to add a comment