Revert of Fix X11TopmostWindowFinder (https://codereview.chromium.org/268673017/)

Reason for revert:
Speculative revert because of failure in desktop_screen_x11_unittest.cc (DesktopScreenX11Test.GetWindowAtScreenPoint).

Original issue's description:
> Fix X11TopmostWindowFinder to take into account
> - Window manager provided window borders
> - Custom shapes specified via views::Widget::SetShape()
> 
> BUG=None
> TEST=None
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270877

TBR=sadrul@chromium.org,pkotwicz@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=None

Review URL: https://codereview.chromium.org/291583003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270964 0039d316-1c4b-4281-b951-d872f2087c98

ui/base/x/x11_util.cc

@@ -224,6 +224,14 @@ class XCustomCursorCache {
   DISALLOW_COPY_AND_ASSIGN(XCustomCursorCache);
 };
 
+bool IsShapeAvailable() {
+  int dummy;
+  static bool is_shape_available =
+    XShapeQueryExtension(gfx::GetXDisplay(), &dummy, &dummy);
+  return is_shape_available;
+
+}
+
 }  // namespace
 
 bool IsXInput2Available() {
@@ -463,13 +471,6 @@ void HideHostCursor() {
   return invisible_cursor;
 }
 
-bool IsShapeExtensionAvailable() {
-  int dummy;
-  static bool is_shape_available =
-      XShapeQueryExtension(gfx::GetXDisplay(), &dummy, &dummy);
-  return is_shape_available;
-}
-
 XID GetX11RootWindow() {
   return DefaultRootWindow(gfx::GetXDisplay());
 }
@@ -578,7 +579,7 @@ bool WindowContainsPoint(XID window, gfx::Point screen_loc) {
   if (!window_rect.Contains(screen_loc))
     return false;
 
-  if (!IsShapeExtensionAvailable())
+  if (!IsShapeAvailable())
     return true;
 
   // According to http://www.x.org/releases/X11R7.6/doc/libXext/shapelib.html,

ui/base/x/x11_util.h

@@ -95,9 +95,6 @@ UI_BASE_EXPORT ::Cursor CreateInvisibleCursor();
 
 // These functions do not cache their results --------------------------
 
-// Returns true if the shape extension is supported.
-UI_BASE_EXPORT bool IsShapeExtensionAvailable();
-
 // Get the X window id for the default root window
 UI_BASE_EXPORT XID GetX11RootWindow();
 

ui/views/views.gyp

@@ -679,7 +679,6 @@
         'widget/desktop_aura/desktop_native_widget_aura_unittest.cc',
         'widget/desktop_aura/desktop_screen_x11_unittest.cc',
         'widget/desktop_aura/desktop_screen_position_client_unittest.cc',
-        'widget/desktop_aura/desktop_window_tree_host_x11_unittest.cc',
         'widget/native_widget_aura_unittest.cc',
         'widget/native_widget_unittest.cc',
         'widget/root_view_unittest.cc',
@@ -732,12 +731,6 @@
             '../../base/allocator/allocator.gyp:allocator',
           ],
         }],
-        ['use_x11==1', {
-          'dependencies': [
-            '../../build/linux/system.gyp:x11',
-            '../../build/linux/system.gyp:xext',
-          ],
-        }],
         ['use_ozone==1', {
           'sources!': [
             'corewm/capture_controller_unittest.cc',

ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc

@@ -76,7 +76,6 @@ const char* kAtomsToCache[] = {
   "UTF8_STRING",
   "WM_DELETE_WINDOW",
   "WM_PROTOCOLS",
-  "_NET_FRAME_EXTENTS",
   "_NET_WM_CM_S0",
   "_NET_WM_ICON",
   "_NET_WM_NAME",
@@ -141,8 +140,7 @@ DesktopWindowTreeHostX11::DesktopWindowTreeHostX11(
       desktop_native_widget_aura_(desktop_native_widget_aura),
       content_window_(NULL),
       window_parent_(NULL),
-      window_shape_(NULL),
-      custom_window_shape_(false),
+      custom_window_shape_(NULL),
       urgency_hint_set_(false) {
 }
 
@@ -150,8 +148,8 @@ DesktopWindowTreeHostX11::~DesktopWindowTreeHostX11() {
   window()->ClearProperty(kHostForRootWindow);
   aura::client::SetWindowMoveClient(window(), NULL);
   desktop_native_widget_aura_->OnDesktopWindowTreeHostDestroyed(this);
-  if (window_shape_)
-    XDestroyRegion(window_shape_);
+  if (custom_window_shape_)
+    XDestroyRegion(custom_window_shape_);
   DestroyDispatcher();
 }
 
@@ -183,16 +181,6 @@ gfx::Rect DesktopWindowTreeHostX11::GetX11RootWindowBounds() const {
   return bounds_;
 }
 
-gfx::Rect DesktopWindowTreeHostX11::GetX11RootWindowOuterBounds() const {
-  gfx::Rect outer_bounds(bounds_);
-  outer_bounds.Inset(-native_window_frame_borders_);
-  return outer_bounds;
-}
-
-::Region DesktopWindowTreeHostX11::GetWindowShape() const {
-  return window_shape_;
-}
-
 void DesktopWindowTreeHostX11::HandleNativeWidgetActivationChanged(
     bool active) {
   if (active) {
@@ -344,10 +332,8 @@ aura::WindowTreeHost* DesktopWindowTreeHostX11::AsWindowTreeHost() {
 
 void DesktopWindowTreeHostX11::ShowWindowWithState(
     ui::WindowShowState show_state) {
-  if (!window_mapped_) {
+  if (!window_mapped_)
     MapWindow(show_state);
-    ResetWindowRegion();
-  }
 
   if (show_state == ui::SHOW_STATE_NORMAL ||
       show_state == ui::SHOW_STATE_MAXIMIZED) {
@@ -478,10 +464,9 @@ gfx::Rect DesktopWindowTreeHostX11::GetWorkAreaBoundsInScreen() const {
 }
 
 void DesktopWindowTreeHostX11::SetShape(gfx::NativeRegion native_region) {
-  if (window_shape_)
-    XDestroyRegion(window_shape_);
-  custom_window_shape_ = true;
-  window_shape_ = gfx::CreateRegionFromSkRegion(*native_region);
+  if (custom_window_shape_)
+    XDestroyRegion(custom_window_shape_);
+  custom_window_shape_ = gfx::CreateRegionFromSkRegion(*native_region);
   ResetWindowRegion();
   delete native_region;
 }
@@ -1127,68 +1112,6 @@ void DesktopWindowTreeHostX11::InitX11Window(
   CreateCompositor(GetAcceleratedWidget());
 }
 
-void DesktopWindowTreeHostX11::OnWMStateUpdated() {
-  std::vector< ::Atom> atom_list;
-  if (!ui::GetAtomArrayProperty(xwindow_, "_NET_WM_STATE", &atom_list))
-    return;
-
-  window_properties_.clear();
-  std::copy(atom_list.begin(), atom_list.end(),
-            inserter(window_properties_, window_properties_.begin()));
-
-  if (!restored_bounds_.IsEmpty() && !IsMaximized()) {
-    // If we have restored bounds, but WM_STATE no longer claims to be
-    // maximized, we should clear our restored bounds.
-    restored_bounds_ = gfx::Rect();
-  } else if (IsMaximized() && restored_bounds_.IsEmpty()) {
-    // The request that we become maximized originated from a different process.
-    // |bounds_| already contains our maximized bounds. Do a best effort attempt
-    // to get restored bounds by setting it to our previously set bounds (and if
-    // we get this wrong, we aren't any worse off since we'd otherwise be
-    // returning our maximized bounds).
-    restored_bounds_ = previous_bounds_;
-  }
-
-  is_fullscreen_ = HasWMSpecProperty("_NET_WM_STATE_FULLSCREEN");
-  is_always_on_top_ = HasWMSpecProperty("_NET_WM_STATE_ABOVE");
-
-  // Now that we have different window properties, we may need to relayout the
-  // window. (The windows code doesn't need this because their window change is
-  // synchronous.)
-  //
-  // TODO(erg): While this does work, there's a quick flash showing the
-  // tabstrip/toolbar/etc. when going into fullscreen mode before hiding those
-  // parts of the UI because we receive the sizing event from the window
-  // manager before we receive the event that changes the fullscreen state.
-  // Unsure what to do about that.
-  Widget* widget = native_widget_delegate_->AsWidget();
-  NonClientView* non_client_view = widget->non_client_view();
-  // non_client_view may be NULL, especially during creation.
-  if (non_client_view) {
-    non_client_view->client_view()->InvalidateLayout();
-    non_client_view->InvalidateLayout();
-  }
-  widget->GetRootView()->Layout();
-  // Refresh the window's border, which may need to be updated if we have
-  // changed the window's maximization state.
-  ResetWindowRegion();
-}
-
-void DesktopWindowTreeHostX11::OnFrameExtentsUpdated() {
-  std::vector<int> insets;
-  if (ui::GetIntArrayProperty(xwindow_, "_NET_FRAME_EXTENTS", &insets) &&
-      insets.size() == 4) {
-    // |insets| are returned in the order: [left, right, top, bottom].
-    native_window_frame_borders_ = gfx::Insets(
-        insets[2],
-        insets[0],
-        insets[3],
-        insets[1]);
-  } else {
-    native_window_frame_borders_ = gfx::Insets();
-  }
-}
-
 void DesktopWindowTreeHostX11::SetWMSpecState(bool enabled,
                                                 ::Atom state1,
                                                 ::Atom state2) {
@@ -1282,14 +1205,10 @@ void DesktopWindowTreeHostX11::ResetWindowRegion() {
   // If a custom window shape was supplied then apply it.
   if (custom_window_shape_) {
     XShapeCombineRegion(
-        xdisplay_, xwindow_, ShapeBounding, 0, 0, window_shape_, false);
+        xdisplay_, xwindow_, ShapeBounding, 0, 0, custom_window_shape_, false);
     return;
   }
 
-  if (window_shape_)
-    XDestroyRegion(window_shape_);
-  window_shape_ = NULL;
-
   if (!IsMaximized()) {
     gfx::Path window_mask;
     views::Widget* widget = native_widget_delegate_->AsWidget();
@@ -1298,9 +1217,10 @@ void DesktopWindowTreeHostX11::ResetWindowRegion() {
       // so, use it to define the window shape. If not, fall through.
       widget->non_client_view()->GetWindowMask(bounds_.size(), &window_mask);
       if (window_mask.countPoints() > 0) {
-        window_shape_ = gfx::CreateRegionFromSkPath(window_mask);
+        Region region = gfx::CreateRegionFromSkPath(window_mask);
         XShapeCombineRegion(xdisplay_, xwindow_, ShapeBounding,
-                            0, 0, window_shape_, false);
+                            0, 0, region, false);
+        XDestroyRegion(region);
         return;
       }
     }
@@ -1683,11 +1603,53 @@ uint32_t DesktopWindowTreeHostX11::DispatchEvent(
       break;
     }
     case PropertyNotify: {
-      ::Atom changed_atom = xev->xproperty.atom;
-      if (changed_atom == atom_cache_.GetAtom("_NET_WM_STATE"))
-        OnWMStateUpdated();
-      else if (changed_atom == atom_cache_.GetAtom("_NET_FRAME_EXTENTS"))
-        OnFrameExtentsUpdated();
+      // Get our new window property state if the WM has told us its changed.
+      ::Atom state = atom_cache_.GetAtom("_NET_WM_STATE");
+
+      std::vector< ::Atom> atom_list;
+      if (xev->xproperty.atom == state &&
+          ui::GetAtomArrayProperty(xwindow_, "_NET_WM_STATE", &atom_list)) {
+        window_properties_.clear();
+        std::copy(atom_list.begin(), atom_list.end(),
+                  inserter(window_properties_, window_properties_.begin()));
+
+        if (!restored_bounds_.IsEmpty() && !IsMaximized()) {
+          // If we have restored bounds, but WM_STATE no longer claims to be
+          // maximized, we should clear our restored bounds.
+          restored_bounds_ = gfx::Rect();
+        } else if (IsMaximized() && restored_bounds_.IsEmpty()) {
+          // The request that we become maximized originated from a different
+          // process. |bounds_| already contains our maximized bounds. Do a
+          // best effort attempt to get restored bounds by setting it to our
+          // previously set bounds (and if we get this wrong, we aren't any
+          // worse off since we'd otherwise be returning our maximized bounds).
+          restored_bounds_ = previous_bounds_;
+        }
+
+        is_fullscreen_ = HasWMSpecProperty("_NET_WM_STATE_FULLSCREEN");
+        is_always_on_top_ = HasWMSpecProperty("_NET_WM_STATE_ABOVE");
+
+        // Now that we have different window properties, we may need to
+        // relayout the window. (The windows code doesn't need this because
+        // their window change is synchronous.)
+        //
+        // TODO(erg): While this does work, there's a quick flash showing the
+        // tabstrip/toolbar/etc. when going into fullscreen mode before hiding
+        // those parts of the UI because we receive the sizing event from the
+        // window manager before we receive the event that changes the
+        // fullscreen state. Unsure what to do about that.
+        Widget* widget = native_widget_delegate_->AsWidget();
+        NonClientView* non_client_view = widget->non_client_view();
+        // non_client_view may be NULL, especially during creation.
+        if (non_client_view) {
+          non_client_view->client_view()->InvalidateLayout();
+          non_client_view->InvalidateLayout();
+        }
+        widget->GetRootView()->Layout();
+        // Refresh the window's border, which may need to be updated if we have
+        // changed the window's maximization state.
+        ResetWindowRegion();
+      }
       break;
     }
     case SelectionNotify: {

ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h

@@ -15,7 +15,6 @@
 #include "ui/base/cursor/cursor_loader_x11.h"
 #include "ui/events/event_source.h"
 #include "ui/events/platform/platform_event_dispatcher.h"
-#include "ui/gfx/insets.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/x/x11_atom_cache.h"
 #include "ui/views/views_export.h"
@@ -59,14 +58,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostX11
   // Returns the current bounds in terms of the X11 Root Window.
   gfx::Rect GetX11RootWindowBounds() const;
 
-  // Returns the current bounds in terms of the X11 Root Window including the
-  // borders provided by the window manager (if any).
-  gfx::Rect GetX11RootWindowOuterBounds() const;
-
-  // Returns the window shape if the window is not rectangular. Returns NULL
-  // otherwise.
-  ::Region GetWindowShape() const;
-
   // Called by X11DesktopHandler to notify us that the native windowing system
   // has changed our activation.
   void HandleNativeWidgetActivationChanged(bool active);
@@ -167,12 +158,6 @@ private:
   // along with all aura client objects that direct behavior.
   aura::WindowEventDispatcher* InitDispatcher(const Widget::InitParams& params);
 
-  // Called when |xwindow_|'s _NET_WM_STATE property is updated.
-  void OnWMStateUpdated();
-
-  // Called when |xwindow_|'s _NET_FRAME_EXTENTS property is updated.
-  void OnFrameExtentsUpdated();
-
   // Sends a message to the x11 window manager, enabling or disabling the
   // states |state1| and |state2|.
   void SetWMSpecState(bool enabled, ::Atom state1, ::Atom state2);
@@ -290,14 +275,8 @@ private:
 
   ObserverList<DesktopWindowTreeHostObserverX11> observer_list_;
 
-  // The window shape if the window is non-rectangular.
-  ::Region window_shape_;
-
-  // Whether |window_shape_| was set via SetShape().
-  bool custom_window_shape_;
-
-  // The size of the window manager provided borders (if any).
-  gfx::Insets native_window_frame_borders_;
+  // Copy of custom window shape specified via SetShape(), if any.
+  ::Region custom_window_shape_;
 
   // The current root window host that has capture. While X11 has something
   // like Windows SetCapture()/ReleaseCapture(), it is entirely implicit and

ui/views/widget/desktop_aura/x11_topmost_window_finder.cc

@@ -4,9 +4,6 @@
 
 #include "ui/views/widget/desktop_aura/x11_topmost_window_finder.h"
 
-#include <X11/Xutil.h>
-
-#include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/window.h"
 #include "ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h"
 
@@ -37,7 +34,7 @@ aura::Window* X11TopmostWindowFinder::FindLocalProcessWindowAt(
     return NULL;
 
   ui::EnumerateTopLevelWindows(this);
-  return DesktopWindowTreeHostX11::GetContentWindowForXID(toplevel_);
+  return views::DesktopWindowTreeHostX11::GetContentWindowForXID(toplevel_);
 }
 
 XID X11TopmostWindowFinder::FindWindowAt(const gfx::Point& screen_loc) {
@@ -74,24 +71,8 @@ bool X11TopmostWindowFinder::ShouldStopIteratingAtLocalProcessWindow(
 
   // Currently |window|->IsVisible() always returns true.
   // TODO(pkotwicz): Fix this. crbug.com/353038
-  if (!window->IsVisible())
-    return false;
-
-  DesktopWindowTreeHostX11* host =
-      DesktopWindowTreeHostX11::GetHostForXID(
-          window->GetHost()->GetAcceleratedWidget());
-  if (!host->GetX11RootWindowOuterBounds().Contains(screen_loc_))
-    return false;
-
-  ::Region shape = host->GetWindowShape();
-  if (!shape)
-    return true;
-
-  aura::client::ScreenPositionClient* screen_position_client =
-      aura::client::GetScreenPositionClient(window->GetRootWindow());
-  gfx::Point window_loc(screen_loc_);
-  screen_position_client->ConvertPointFromScreen(window, &window_loc);
-  return XPointInRegion(shape, window_loc.x(), window_loc.y()) == True;
+  return window->IsVisible() &&
+      window->GetBoundsInScreen().Contains(screen_loc_);
 }
 
 }  // namespace views