Find display root ServerWindow for located events in mus.

services/ui/ws/window_manager_state.cc

Index: services/ui/ws/window_manager_state.cc
diff --git a/services/ui/ws/window_manager_state.cc b/services/ui/ws/window_manager_state.cc
index 943d3c2389b3871d1303b31bc06084fc2e022e07..dbf01b5e081ef1ab3165ceb388dff1d1568caa8d 100644
--- a/services/ui/ws/window_manager_state.cc
+++ b/services/ui/ws/window_manager_state.cc
@@ -4,6 +4,8 @@
 
 #include "services/ui/ws/window_manager_state.h"
 
+#include <utility>
+
 #include "base/memory/weak_ptr.h"
 #include "services/shell/public/interfaces/connector.mojom.h"
 #include "services/ui/common/event_matcher_util.h"
@@ -522,16 +524,33 @@ ClientSpecificId WindowManagerState::GetEventTargetClientId(
 }
 
 ServerWindow* WindowManagerState::GetRootWindowContaining(
-    const gfx::Point& location) {
-  if (display_manager()->displays().empty())
-    return nullptr;
+    gfx::Point* location) {
+  Display* target_display = nullptr;
+  for (Display* display : display_manager()->displays()) {
+    if (display->platform_display()->GetBounds().Contains(*location)) {
+      target_display = display;
+      break;
+    }
+  }
+
+  // TODO(kylechar): Better handle locations outside the window when running
+  // with X11. These locations crop up when windows overlap or when dragging.

[sky, 2016/08/11 20:58:04]

I believe this will also happen on some devices that have sensors outside the
display bounds. You'll likely need a better way to pit the target_display in
this case. I'm OK with a TODO if you want to deal later.

[kylechar, 2016/08/12 16:09:11]

Yep, it definitely needs more work. This will avoid it crashing for now and 

+  if (!target_display) {
+    LOG(ERROR) << "Invalid event location " << location->ToString();

[sky, 2016/08/11 20:58:04]

Make this a DVLOG(1) given it can happen in normal practice.

[kylechar, 2016/08/12 16:09:11]

Done.

+    target_display = *(display_manager()->displays().begin());
+  }
 
-  // TODO(sky): this isn't right. To correctly implement need bounds of
-  // Display, which we aren't tracking yet. For now, use the first display.
-  Display* display = *(display_manager()->displays().begin());
   WindowManagerDisplayRoot* display_root =
-      display->GetWindowManagerDisplayRootForUser(user_id());
-  return display_root ? display_root->root() : nullptr;
+      target_display->GetWindowManagerDisplayRootForUser(user_id());
+
+  if (!display_root)
+    return nullptr;
+
+  // Translate the location to be relative to the display instead of relative
+  // to the screen space.
+  gfx::Point origin = target_display->root_window()->bounds().origin();

[sky, 2016/08/11 20:58:04]

The location of the root is always 0, 0. Don't you want to use the origin from
the PlatformDisplay?

[kylechar, 2016/08/12 16:09:11]

I'll change it to use the PlatformDisplay bounds be consistent with what I'm
checking above.

Although, I definitely misunderstood this before and changed the Display root
ServerWindow origin in a previous CL. My reasoning was that since the Display
root ServerWindow doesn't have a parent, the origin should be in screen
coordinates and made it the same as the PlatformDisplay origin.

I'm happy to change it back so that Display root ServerWindows are all at (0,
0). Although, changing it back breaks a bunch of drawing and event handling code
with regards to the second display. With ToT this morning + this patch, drawing
on the second display is working. I spent the last hour trying to figure out
where exactly it breaks but haven't found it yet.

[sky, 2016/08/12 16:47:46]

I think we need to put them at 0, 0, otherwise I worry that we're going to have
inconsistencies with what all the client code is expecting. By that I mean
existing ash code (and other places) convert to screen bounds using something
like:
convert window bounds to root
add display bounds

If the root includes the display bounds then we're going to get the wrong thing.

+  location->Offset(-origin.x(), -origin.y());
+  return display_root->root();
 }
 
 void WindowManagerState::OnEventTargetNotFound(const ui::Event& event) {