Merge #1208

1208: XWaylandWM::get_wm_surface() r=wmww a=wmww More code left over from Thursday. Adds a new `XWaylandWM::get_wm_surface()` method. This makes converting from XCB windows to wm surfaces more convenient and publicly accessible (the latter will be important when shell surfaces want to look up their parent windows). This also fixes the issue of accidentally creating null shell surfaces in the surfaces map via careless usage of the '[]` map operator. Co-authored-by: William Wold <[email protected]>
canonical · Feb 14, 2020 · 8e56313 · 8e56313
1 parent 6d9b832
commit 8e56313
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 49 deletions.
diff --git a/src/server/frontend_xwayland/xwayland_wm.cpp b/src/server/frontend_xwayland/xwayland_wm.cpp
@@ -286,6 +286,15 @@ auto mf::XWaylandWM::build_shell_surface(
     return shell->build_shell_surface(wm_surface, wayland_client, wayland_surface);
 }
 
+auto mf::XWaylandWM::get_wm_surface(xcb_window_t xcb_window) -> std::experimental::optional<std::shared_ptr<XWaylandWMSurface>>
+{
+    auto const surface = surfaces.find(xcb_window);
+    if (surface == surfaces.end() || !surface->second)
+        return std::experimental::nullopt;
+    else
+        return surface->second;
+}
+
 void mf::XWaylandWM::run_on_wayland_thread(std::function<void()>&& work)
 {
     wayland_connector->run_on_wayland_display([work = move(work)](auto){ work(); });
@@ -379,12 +388,6 @@ void mf::XWaylandWM::handle_events()
 
 void mf::XWaylandWM::handle_property_notify(xcb_property_notify_event_t *event)
 {
-    auto surface = surfaces[event->window];
-    if (!surface)
-        return;
-
-    surface->dirty_properties();
-
     if (verbose_xwayland_logging_enabled())
     {
         if (event->state == XCB_PROPERTY_DELETE)
@@ -419,6 +422,11 @@ void mf::XWaylandWM::handle_property_notify(xcb_property_notify_event_t *event)
             free(reply);
         }
     }
+
+    if (auto const surface = get_wm_surface(event->window))
+    {
+        surface.value()->dirty_properties();
+    }
 }
 
 void mf::XWaylandWM::handle_create_notify(xcb_create_notify_event_t *event)
@@ -460,12 +468,6 @@ void mf::XWaylandWM::handle_destroy_notify(xcb_destroy_notify_event_t *event)
             get_window_debug_string(event->event).c_str());
     }
 
-    if (is_ours(event->window))
-        return;
-
-    if (surfaces.find(event->window) == surfaces.end())
-        return;
-
     surfaces.erase(event->window);
 }
 
@@ -479,21 +481,15 @@ void mf::XWaylandWM::handle_map_request(xcb_map_request_event_t *event)
             get_window_debug_string(event->parent).c_str());
     }
 
-    if (is_ours(event->window))
-        return;
-
-    auto const surface_iter = surfaces.find(event->window);
-    if (surface_iter == surfaces.end())
-        return;
-
-    auto const surface = surface_iter->second;
-
-    surface->read_properties();
-    surface->set_wm_state(XWaylandWMSurface::NormalState);
-    surface->set_net_wm_state();
-    surface->set_workspace(0);
-    xcb_map_window(xcb_connection, event->window);
-    xcb_flush(xcb_connection);
+    if (auto const surface = get_wm_surface(event->window))
+    {
+        surface.value()->read_properties();
+        surface.value()->set_wm_state(XWaylandWMSurface::NormalState);
+        surface.value()->set_net_wm_state();
+        surface.value()->set_workspace(0);
+        xcb_map_window(xcb_connection, event->window);
+        xcb_flush(xcb_connection);
+    }
 }
 
 void mf::XWaylandWM::handle_unmap_notify(xcb_unmap_notify_event_t *event)
@@ -514,16 +510,13 @@ void mf::XWaylandWM::handle_unmap_notify(xcb_unmap_notify_event_t *event)
     if (event->response_type & ~0x80)
         return;
 
-    auto const surface_iter = surfaces.find(event->window);
-    if (surface_iter == surfaces.end())
-        return;
-
-    auto const surface = surface_iter->second;
-
-    surface->set_wm_state(XWaylandWMSurface::WithdrawnState);
-    surface->set_workspace(-1);
-    xcb_unmap_window(xcb_connection, event->window);
-    xcb_flush(xcb_connection);
+    if (auto const surface = get_wm_surface(event->window))
+    {
+        surface.value()->set_wm_state(XWaylandWMSurface::WithdrawnState);
+        surface.value()->set_workspace(-1);
+        xcb_unmap_window(xcb_connection, event->window);
+        xcb_flush(xcb_connection);
+    }
 }
 
 void mf::XWaylandWM::handle_client_message(xcb_client_message_event_t *event)
@@ -536,18 +529,15 @@ void mf::XWaylandWM::handle_client_message(xcb_client_message_event_t *event)
             get_window_debug_string(event->window).c_str());
     }
 
-    auto const surface_iter = surfaces.find(event->window);
-    if (surface_iter == surfaces.end())
-        return;
-
-    auto const surface = surface_iter->second;
-
-    if (event->type == xcb_atom.net_wm_moveresize)
-        handle_move_resize(surface, event);
-    else if (event->type == xcb_atom.net_wm_state)
-        handle_state(surface, event);
-    else if (event->type == xcb_atom.wl_surface_id)
-        handle_surface_id(surface, event);
+    if (auto const surface = get_wm_surface(event->window))
+    {
+        if (event->type == xcb_atom.net_wm_moveresize)
+            handle_move_resize(surface.value(), event);
+        else if (event->type == xcb_atom.net_wm_state)
+            handle_state(surface.value(), event);
+        else if (event->type == xcb_atom.wl_surface_id)
+            handle_surface_id(surface.value(), event);
+    }
 }
 
 void mf::XWaylandWM::handle_move_resize(std::shared_ptr<XWaylandWMSurface> surface, xcb_client_message_event_t *event)

diff --git a/src/server/frontend_xwayland/xwayland_wm.h b/src/server/frontend_xwayland/xwayland_wm.h
@@ -73,6 +73,7 @@ class XWaylandWM
     auto build_shell_surface(
         XWaylandWMSurface* wm_surface,
         WlSurface* wayland_surface) -> XWaylandWMShellSurface*;
+    auto get_wm_surface(xcb_window_t xcb_window) -> std::experimental::optional<std::shared_ptr<XWaylandWMSurface>>;
     void run_on_wayland_thread(std::function<void()>&& work);
 
     XCBAtoms const xcb_atom;