Fix on DragWithMaskedWindows

The current combination of WindowTargeter with MaskedWindowDelegate
isn't working well with Mash. Instead of doing this, it needs to
use window targeter's new API for setting window masks.

The GetEventHandlerForPoint should be fixed in a way to follow
this new way.

BUG=901546
TEST=the new test case also covers

Change-Id: I0b095777450fab3fa28ff9ebd8473667f644ab9e
Reviewed-on: https://chromium-review.googlesource.com/c/1320689Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605853}

ash/wm/window_finder.cc

@@ -9,6 +9,8 @@
 #include "services/ws/window_service.h"
 #include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/window.h"
+#include "ui/aura/window_targeter.h"
+#include "ui/events/event.h"
 
 namespace {
 
@@ -20,10 +22,35 @@ bool IsTopLevelWindow(aura::Window* window) {
          ws::WindowService::HasRemoteClient(window);
 }
 
+// Returns true if |window| can be a target at |screen_point| by |targeter|.
+// If |targeter| is null, it will check with Window::GetEventHandlerForPoint().
+bool IsWindowTargeted(aura::Window* window,
+                      const gfx::Point& screen_point,
+                      aura::WindowTargeter* targeter) {
+  aura::client::ScreenPositionClient* client =
+      aura::client::GetScreenPositionClient(window->GetRootWindow());
+  gfx::Point local_point = screen_point;
+  if (targeter) {
+    client->ConvertPointFromScreen(window->parent(), &local_point);
+    // TODO(mukai): consider the hittest differences between mouse and touch.
+    gfx::Point point_in_root = local_point;
+    aura::Window::ConvertPointToTarget(window, window->GetRootWindow(),
+                                       &point_in_root);
+    ui::MouseEvent event(ui::ET_MOUSE_MOVED, local_point, point_in_root,
+                         base::TimeTicks::Now(), 0, 0);
+    return targeter->SubtreeShouldBeExploredForEvent(window, event);
+  }
+  // TODO(mukai): maybe we can remove this, simply return false if targeter does
+  // not exist.
+  client->ConvertPointFromScreen(window, &local_point);
+  return window->GetEventHandlerForPoint(local_point);
+}
+
 // Get the toplevel window at |screen_point| among the descendants of |window|.
 aura::Window* GetTopmostWindowAtPointWithinWindow(
     const gfx::Point& screen_point,
     aura::Window* window,
+    aura::WindowTargeter* targeter,
     const std::set<aura::Window*> ignore,
     aura::Window** real_topmost) {
   if (!window->IsVisible())
@@ -35,11 +62,7 @@ aura::Window* GetTopmostWindowAtPointWithinWindow(
     return nullptr;
 
   if (IsTopLevelWindow(window)) {
-    aura::client::ScreenPositionClient* client =
-        aura::client::GetScreenPositionClient(window->GetRootWindow());
-    gfx::Point local_point = screen_point;
-    client->ConvertPointFromScreen(window, &local_point);
-    if (window->GetEventHandlerForPoint(local_point)) {
+    if (IsWindowTargeted(window, screen_point, targeter)) {
       if (real_topmost && !(*real_topmost))
         *real_topmost = window;
       return (ignore.find(window) == ignore.end()) ? window : nullptr;
@@ -50,8 +73,10 @@ aura::Window* GetTopmostWindowAtPointWithinWindow(
   for (aura::Window::Windows::const_reverse_iterator i =
            window->children().rbegin();
        i != window->children().rend(); ++i) {
+    aura::WindowTargeter* child_targeter =
+        (*i)->targeter() ? (*i)->targeter() : targeter;
     aura::Window* result = GetTopmostWindowAtPointWithinWindow(
-        screen_point, *i, ignore, real_topmost);
+        screen_point, *i, child_targeter, ignore, real_topmost);
     if (result)
       return result;
   }
@@ -68,8 +93,9 @@ aura::Window* GetTopmostWindowAtPoint(const gfx::Point& screen_point,
                                       aura::Window** real_topmost) {
   if (real_topmost)
     *real_topmost = nullptr;
+  aura::Window* root = GetRootWindowAt(screen_point);
   return GetTopmostWindowAtPointWithinWindow(
-      screen_point, GetRootWindowAt(screen_point), ignore, real_topmost);
+      screen_point, root, root->targeter(), ignore, real_topmost);
 }
 
 }  // namespace wm

ash/wm/window_finder_unittest.cc

@@ -7,7 +7,9 @@
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
+#include "ui/aura/window_targeter.h"
 #include "ui/compositor/layer_type.h"
+#include "ui/gfx/geometry/insets.h"
 
 namespace ash {
 namespace wm {
@@ -91,5 +93,30 @@ TEST_F(WindowFinderTest, MultipleDisplays) {
             GetTopmostWindowAtPoint(gfx::Point(10, 210), ignore, nullptr));
 }
 
+TEST_F(WindowFinderTest, WindowTargeterWithHitTestRects) {
+  std::unique_ptr<aura::Window> window1 =
+      CreateTestWindow(gfx::Rect(0, 0, 100, 100));
+  std::unique_ptr<aura::Window> window2 =
+      CreateTestWindow(gfx::Rect(0, 0, 100, 100));
+
+  std::set<aura::Window*> ignore;
+
+  aura::Window* real_topmost = nullptr;
+  EXPECT_EQ(window2.get(),
+            GetTopmostWindowAtPoint(gfx::Point(10, 10), ignore, &real_topmost));
+  EXPECT_EQ(window2.get(), real_topmost);
+
+  auto targeter = std::make_unique<aura::WindowTargeter>();
+  targeter->SetInsets(gfx::Insets(0, 50, 0, 0));
+  window2->SetEventTargeter(std::move(targeter));
+
+  EXPECT_EQ(window1.get(),
+            GetTopmostWindowAtPoint(gfx::Point(10, 10), ignore, &real_topmost));
+  EXPECT_EQ(window1.get(), real_topmost);
+  EXPECT_EQ(window2.get(),
+            GetTopmostWindowAtPoint(gfx::Point(60, 10), ignore, &real_topmost));
+  EXPECT_EQ(window2.get(), real_topmost);
+}
+
 }  // namespace wm
 }  // namespace ash

chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc

@@ -656,31 +656,18 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest, DragInSameWindow) {
 }
 
 #if defined(USE_AURA)
-namespace {
-
-// We need both MaskedWindowTargeter and MaskedWindowDelegate as they
-// are used in two different pathes. crbug.com/493354.
-class MaskedWindowTargeter : public aura::WindowTargeter {
- public:
-  MaskedWindowTargeter() {}
-  ~MaskedWindowTargeter() override {}
-
-  // aura::WindowTargeter:
-  bool EventLocationInsideBounds(aura::Window* target,
-                                 const ui::LocatedEvent& event) const override {
-    aura::Window* window = static_cast<aura::Window*>(target);
-    gfx::Point local_point = event.location();
-    if (window->parent())
-      aura::Window::ConvertPointToTarget(window->parent(), window,
-                                         &local_point);
-    return window->GetEventHandlerForPoint(local_point);
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MaskedWindowTargeter);
-};
-
-}  // namespace
+bool SubtreeShouldBeExplored(aura::Window* window,
+                             const gfx::Point& local_point) {
+  gfx::Point point_in_parent = local_point;
+  aura::Window::ConvertPointToTarget(window, window->parent(),
+                                     &point_in_parent);
+  gfx::Point point_in_root = local_point;
+  aura::Window::ConvertPointToTarget(window, window->GetRootWindow(),
+                                     &point_in_root);
+  ui::MouseEvent event(ui::ET_MOUSE_MOVED, point_in_parent, point_in_root,
+                       base::TimeTicks::Now(), 0, 0);
+  return window->targeter()->SubtreeShouldBeExploredForEvent(window, event);
+}
 
 // The logic to find the target tabstrip should take the window mask into
 // account. This test hangs without the fix. crbug.com/473080.
@@ -690,19 +677,20 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
 
   aura::Window* browser_window = browser()->window()->GetNativeWindow();
   const gfx::Rect bounds = browser_window->GetBoundsInScreen();
-  aura::test::MaskedWindowDelegate masked_window_delegate(
-      gfx::Rect(bounds.width() - 10, 0, 10, bounds.height()));
-  gfx::Rect test(bounds);
+  aura::test::TestWindowDelegate masked_window_delegate;
   masked_window_delegate.set_can_focus(false);
   std::unique_ptr<aura::Window> masked_window(
-      aura::test::CreateTestWindowWithDelegate(&masked_window_delegate, 10,
-                                               test, browser_window->parent()));
-  masked_window->SetEventTargeter(std::make_unique<MaskedWindowTargeter>());
-
-  ASSERT_FALSE(masked_window->GetEventHandlerForPoint(
-      gfx::Point(bounds.width() - 11, 0)));
-  ASSERT_TRUE(masked_window->GetEventHandlerForPoint(
-      gfx::Point(bounds.width() - 9, 0)));
+      aura::test::CreateTestWindowWithDelegate(
+          &masked_window_delegate, 10, bounds, browser_window->parent()));
+  masked_window->SetProperty(aura::client::kAlwaysOnTopKey, true);
+  auto targeter = std::make_unique<aura::WindowTargeter>();
+  targeter->SetInsets(gfx::Insets(0, bounds.width() - 10, 0, 0));
+  masked_window->SetEventTargeter(std::move(targeter));
+
+  ASSERT_FALSE(SubtreeShouldBeExplored(masked_window.get(),
+                                       gfx::Point(bounds.width() - 11, 0)));
+  ASSERT_TRUE(SubtreeShouldBeExplored(masked_window.get(),
+                                      gfx::Point(bounds.width() - 9, 0)));
   TabStrip* tab_strip = GetTabStripForBrowser(browser());
   TabStripModel* model = browser()->tab_strip_model();
 

testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter

@@ -40,8 +40,6 @@
 # TabDragging: crbug.com/890071
 -TabDragging/DetachToBrowserTabDragControllerTest.DetachToOwnWindowWhileInImmersiveFullscreenMode/1
 -TabDragging/DetachToBrowserTabDragControllerTest.DragToSeparateWindow/1
--TabDragging/DetachToBrowserTabDragControllerTest.DragWithMaskedWindows/0
--TabDragging/DetachToBrowserTabDragControllerTest.DragWithMaskedWindows/1
 -TabDragging/DetachToBrowserTabDragControllerTestTouch.PressSecondFingerWhileDetached/0
 
 # This test is flaky. https://crbug.com/897879