Fix event targeting bug

The bug occurred in finding the valid target. Specifically if the root
has a transform we transform the event first, and then compare against
the size of the root. For transforms that result in increasing the
location (transform value < 1) the location would end up outside the
bounds of the window, which results in treating the location as in the
non-client area and targeting the root.

The fix for now is not to consider the root as a target for the non-client
area. A better fix may be to potentially transform the size by the
targets transform (if there is one), but I'm holding off on that for
now. We don't do scale bounds in WindowTargeter either.

BUG=794262
TEST=covered by tests

Change-Id: If7a913d934808fd4ffc404f74eea73d9f98b71ab
Reviewed-on: https://chromium-review.googlesource.com/877219Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530683}

services/ui/ws/window_finder.cc

@@ -6,7 +6,6 @@
 
 #include "base/containers/adapters.h"
 #include "services/ui/ws/server_window.h"
-#include "services/ui/ws/server_window_delegate.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/point3_f.h"
 #include "ui/gfx/geometry/point_f.h"
@@ -87,6 +86,7 @@ gfx::Transform TransformFromParent(const ServerWindow* window,
 
 bool FindDeepestVisibleWindowForLocationImpl(
     ServerWindow* window,
+    bool is_root_window,
     EventSource event_source,
     const gfx::Point& location_in_root,
     const gfx::Point& location_in_window,
@@ -95,7 +95,11 @@ bool FindDeepestVisibleWindowForLocationImpl(
   // The non-client area takes precedence over descendants, as otherwise the
   // user would likely not be able to hit the non-client area as it's common
   // for descendants to go into the non-client area.
-  if (IsLocationInNonclientArea(window, location_in_window)) {
+  //
+  // Display roots aren't allowed to have non-client areas. This is important
+  // as roots may have a transform, which causes problem in comparing sizes.
+  if (!is_root_window &&
+      IsLocationInNonclientArea(window, location_in_window)) {
     deepest_window->window = window;
     deepest_window->in_non_client_area = true;
     return true;
@@ -133,9 +137,10 @@ bool FindDeepestVisibleWindowForLocationImpl(
         continue;
       }
 
+      const bool child_is_root = false;
       if (FindDeepestVisibleWindowForLocationImpl(
-              child, event_source, location_in_root, location_in_child,
-              child_transform, deepest_window)) {
+              child, child_is_root, event_source, location_in_root,
+              location_in_child, child_transform, deepest_window)) {
         return true;
       }
     }
@@ -164,9 +169,10 @@ DeepestWindow FindDeepestVisibleWindowForLocation(ServerWindow* root_window,
   DeepestWindow result;
   // Allow the root to have a transform, which mirrors what happens with
   // WindowManagerDisplayRoot.
-  FindDeepestVisibleWindowForLocationImpl(root_window, event_source, location,
-                                          initial_location, root_transform,
-                                          &result);
+  const bool is_root_window = true;
+  FindDeepestVisibleWindowForLocationImpl(
+      root_window, is_root_window, event_source, location, initial_location,
+      root_transform, &result);
   return result;
 }
 

services/ui/ws/window_finder_unittest.cc

@@ -273,5 +273,47 @@ TEST_F(WindowFinderTest, FindDeepestVisibleWindowWithTransformOnParent) {
                          .window);
 }
 
+// Creates the following window hierarchy:
+// root
+// |- c1 (has .5x transform, and is used as the root in
+//        FindDeepestVisibleWindowForLocation).
+//    |- c2
+//       |- c3
+// With various assertions around hit testing.
+TEST_F(WindowFinderTest,
+       FindDeepestVisibleWindowWithTransformOnParentMagnified) {
+  TestServerWindowDelegate window_delegate(viz_host_proxy());
+  ServerWindow root(&window_delegate, WindowId(1, 2));
+  root.set_event_targeting_policy(
+      mojom::EventTargetingPolicy::TARGET_AND_DESCENDANTS);
+  root.SetVisible(true);
+  root.SetBounds(gfx::Rect(0, 0, 100, 100), base::nullopt);
+  window_delegate.set_root_window(&root);
+  ServerWindow c1(&window_delegate, WindowId(1, 3));
+  root.Add(&c1);
+  c1.SetVisible(true);
+  c1.SetBounds(gfx::Rect(0, 0, 100, 100), base::nullopt);
+  gfx::Transform transform;
+  transform.Scale(SkFloatToMScalar(.5f), SkFloatToMScalar(.5f));
+  c1.SetTransform(transform);
+
+  ServerWindow c2(&window_delegate, WindowId(1, 4));
+  c1.Add(&c2);
+  c2.SetVisible(true);
+  c2.SetBounds(gfx::Rect(0, 0, 200, 200), base::nullopt);
+
+  ServerWindow c3(&window_delegate, WindowId(1, 5));
+  c2.Add(&c3);
+  c3.SetVisible(true);
+  c3.SetBounds(gfx::Rect(0, 190, 200, 10), base::nullopt);
+
+  EXPECT_EQ(&c2, FindDeepestVisibleWindowForLocation(&c1, EventSource::MOUSE,
+                                                     gfx::Point(55, 55))
+                     .window);
+  EXPECT_EQ(&c3, FindDeepestVisibleWindowForLocation(&c1, EventSource::MOUSE,
+                                                     gfx::Point(0, 99))
+                     .window);
+}
+
 }  // namespace ws
 }  // namespace ui

services/ui/ws/window_manager_state_unittest.cc

@@ -775,6 +775,13 @@ TEST(WindowManagerStateEventTest, AdjustEventLocation) {
   window_server->user_id_tracker()->SetActiveUserId(kUserId1);
   const int64_t first_display_id = screen_manager.AddDisplay();
   const int64_t second_display_id = screen_manager.AddDisplay();
+  Display* first_display =
+      window_server->display_manager()->GetDisplayById(first_display_id);
+  // As there are no child windows make sure the root is a valid target.
+  first_display->GetWindowManagerDisplayRootForUser(kUserId1)
+      ->GetClientVisibleRoot()
+      ->set_event_targeting_policy(
+          mojom::EventTargetingPolicy::TARGET_AND_DESCENDANTS);
   Display* second_display =
       window_server->display_manager()->GetDisplayById(second_display_id);
   ASSERT_TRUE(second_display);