commit fe7446153bb65867b8214b09ee4edcdd79f72fd9
author Zijie He <zijiehe@chromium.org> Tue Aug 29 18:35:14 2017
committer Commit Bot <commit-bot@chromium.org> Tue Aug 29 20:50:37 2017
tree 62d1c37c59015852f8df0d0c14ba8d548a036298
parent c7da79884a729a79c8a9de5b921cb9c745f46a11

GetWindowRect() on X11 does not correctly translate the coordinate

XWindowAttributes.x and y are in the coordinate of the parent window, so
XTranslateCoordinates() should be used to do the translation.
Meanwhile this change reduces the scope of the XErrorTrap in GetWindowList()
function to ensure the callback can use other XErrorTrap to cover other X
function calls.

Bug: webrtc:7950
Change-Id: I520227b11704c5a0cb665d9de86925ed4e86540d
Reviewed-on: https://chromium-review.googlesource.com/639590
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Zijie He <zijiehe@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#19591}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: a787702417b1cef9d2ea2354b770f93c3e144528

modules/desktop_capture/x11/window_list_utils.cc

diff --git a/modules/desktop_capture/x11/window_list_utils.cc b/modules/desktop_capture/x11/window_list_utils.cc
index f71779d..d8c6e57 100644
--- a/modules/desktop_capture/x11/window_list_utils.cc
+++ b/modules/desktop_capture/x11/window_list_utils.cc
@@ -178,7 +178,6 @@
   RTC_DCHECK(cache);
   RTC_DCHECK(on_window);
   ::Display* const display = cache->display();
-  XErrorTrap error_trap(display);
 
   int failed_screens = 0;
   const int num_screens = XScreenCount(display);
@@ -187,16 +186,20 @@
     ::Window parent;
     ::Window* children;
     unsigned int num_children;
-    if (XQueryTree(display,
-                   root_window,
-                   &root_window,
-                   &parent,
-                   &children,
-                   &num_children) == 0) {
-      failed_screens++;
-      LOG(LS_ERROR) << "Failed to query for child windows for screen "
-                    << screen;
-      continue;
+    {
+      XErrorTrap error_trap(display);
+      if (XQueryTree(display,
+                     root_window,
+                     &root_window,
+                     &parent,
+                     &children,
+                     &num_children) == 0 ||
+          error_trap.GetLastErrorAndDisable() != 0) {
+        failed_screens++;
+        LOG(LS_ERROR) << "Failed to query for child windows for screen "
+                      << screen;
+        continue;
+      }
     }
 
     DeferXFree free_children(children);
@@ -221,6 +224,8 @@
                    DesktopRect* rect,
                    XWindowAttributes* attributes /* = nullptr */) {
   XWindowAttributes local_attributes;
+  int offset_x;
+  int offset_y;
   if (attributes == nullptr) {
     attributes = &local_attributes;
   }
@@ -232,8 +237,24 @@
       return false;
     }
   }
+  {
+    XErrorTrap error_trap(display);
+    ::Window child;
+    if (!XTranslateCoordinates(display,
+                               window,
+                               attributes->root,
+                               0,
+                               0,
+                               &offset_x,
+                               &offset_y,
+                               &child) ||
+        error_trap.GetLastErrorAndDisable() != 0) {
+      return false;
+    }
+  }
 
   *rect = DesktopRectFromXAttributes(*attributes);
+  rect->Translate(offset_x, offset_y);
   return true;
 }
 

modules/desktop_capture/x11/window_list_utils.h

diff --git a/modules/desktop_capture/x11/window_list_utils.h b/modules/desktop_capture/x11/window_list_utils.h
index f7c4b5e..8557e24 100644
--- a/modules/desktop_capture/x11/window_list_utils.h
+++ b/modules/desktop_capture/x11/window_list_utils.h
@@ -35,7 +35,8 @@
 
 // Returns the rectangle of the |window| in the coordinates of |display|. This
 // function returns false if native APIs failed. If |attributes| is provided, it
-// will be filled with the attributes of |window|.
+// will be filled with the attributes of |window|. The |rect| is in system
+// coordinate, i.e. the primary monitor always starts from (0, 0).
 bool GetWindowRect(::Display* display,
                    ::Window window,
                    DesktopRect* rect,