Find display root ServerWindow for located events in mus.

services/ui/ws/window_manager_state.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "services/ui/ws/accelerator.h"
 #include "services/ui/ws/display_manager.h"
 #include "services/ui/ws/platform_display.h"
 #include "services/ui/ws/server_window.h"
 #include "services/ui/ws/user_display_manager.h"
 #include "services/ui/ws/user_id_tracker.h"
 #include "services/ui/ws/window_manager_display_root.h"
@@ skipping 498 matching lines @@
   }
 
   if (!tree) {
     DCHECK(in_nonclient_area);
     tree = window_tree_;
   }
   return tree->id();
 }
 
 ServerWindow* WindowManagerState::GetRootWindowContaining(
-    const gfx::Point& location) {
-  if (display_manager()->displays().empty())
+    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());
+  }
+
+  WindowManagerDisplayRoot* display_root =
+      target_display->GetWindowManagerDisplayRootForUser(user_id());
+
+  if (!display_root)
     return nullptr;
 
-  // 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;
+  // 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) {
   window_server()->SendToPointerWatchers(event, user_id(),
                                          nullptr /* ignore_tree */);
 }
 
 }  // namespace ws
 }  // namespace ui