tray: Make m_clients exclusive owner of clients

include/x11/tray_client.hpp

@@ -11,14 +11,14 @@ POLYBAR_NS
 // fwd declarations
 class connection;
 
-class tray_client {
+class tray_client : public non_copyable_mixin {
  public:
   explicit tray_client(connection& conn, xcb_window_t win, size s);
-  tray_client(const tray_client& c) = delete;
-  tray_client& operator=(tray_client& c) = delete;
-
   ~tray_client();
 
+  tray_client(tray_client&&);
+  tray_client& operator=(tray_client&&);
+
   unsigned int width() const;
   unsigned int height() const;
   void clear_window() const;
@@ -39,7 +39,7 @@ class tray_client {
 
  protected:
   connection& m_connection;
-  xcb_window_t m_window{0};
+  xcb_window_t m_window{XCB_NONE};
 
   /**
    * Whether the client window supports XEMBED.

include/x11/tray_manager.hpp

@@ -140,8 +140,8 @@ class tray_manager : public xpp::event::sink<evt::expose, evt::visibility_notify
   int calculate_client_y();
 
   bool is_embedded(const xcb_window_t& win) const;
-  shared_ptr<tray_client> find_client(const xcb_window_t& win) const;
+  tray_client* find_client(const xcb_window_t& win);
-  void remove_client(shared_ptr<tray_client>& client, bool reconfigure = true);
+  void remove_client(const tray_client& client, bool reconfigure = true);
   void remove_client(xcb_window_t win, bool reconfigure = true);
   int mapped_clients() const;
   bool has_mapped_clients() const;
@@ -169,7 +169,7 @@ class tray_manager : public xpp::event::sink<evt::expose, evt::visibility_notify
   const logger& m_log;
   background_manager& m_background_manager;
   std::shared_ptr<bg_slice> m_bg_slice;
-  vector<shared_ptr<tray_client>> m_clients;
+  vector<tray_client> m_clients;
 
   tray_settings m_opts{};
   const bar_settings& m_bar_opts;
@@ -190,7 +190,7 @@ class tray_manager : public xpp::event::sink<evt::expose, evt::visibility_notify
 
   thread m_delaythread;
 
-  mutex m_mtx{};
+  mutable mutex m_mtx{};
 
   bool m_firstactivation{true};
 };

src/x11/tray_client.cpp

@@ -11,7 +11,20 @@ POLYBAR_NS
 tray_client::tray_client(connection& conn, xcb_window_t win, size s) : m_connection(conn), m_window(win), m_size(s) {}
 
 tray_client::~tray_client() {
-  xembed::unembed(m_connection, window(), m_connection.root());
+  if (m_window != XCB_NONE) {
+    xembed::unembed(m_connection, m_window, m_connection.root());
+  }
+}
+
+tray_client::tray_client(tray_client&& c) : m_connection(c.m_connection), m_size(c.m_size) {
+  std::swap(m_window, c.m_window);
+}
+
+tray_client& tray_client::operator=(tray_client&& c) {
+  m_connection = c.m_connection;
+  std::swap(m_window, c.m_window);
+  std::swap(m_size, c.m_size);
+  return *this;
 }
 
 unsigned int tray_client::width() const {
@@ -95,7 +108,6 @@ void tray_client::reconfigure(int x, int y) const {
   XCB_AUX_ADD_PARAM(&configure_mask, &configure_params, height, m_size.h);
   XCB_AUX_ADD_PARAM(&configure_mask, &configure_params, x, x);
   XCB_AUX_ADD_PARAM(&configure_mask, &configure_params, y, y);
-
   connection::pack_values(configure_mask, &configure_params, configure_values);
   m_connection.configure_window_checked(window(), configure_mask, configure_values.data());
 }

src/x11/tray_manager.cpp

@@ -359,15 +359,14 @@ void tray_manager::reconfigure_clients() {
   int x = m_opts.spacing;
 
   for (auto it = m_clients.rbegin(); it != m_clients.rend(); it++) {
-    auto client = *it;
-
     try {
-      client->ensure_state();
+      it->ensure_state();
-      client->reconfigure(x, calculate_client_y());
+      it->reconfigure(x, calculate_client_y());
 
       x += m_opts.client_size.w + m_opts.spacing;
     } catch (const xpp::x::error::window& err) {
-      remove_client(client, false);
+      // TODO print error
+      remove_client(*it, false);
     }
   }
 
@@ -427,14 +426,14 @@ void tray_manager::refresh_window() {
 
   m_connection.clear_area(0, m_tray, 0, 0, width, height);
 
-  for (auto&& client : m_clients) {
+  for (auto& client : m_clients) {
     try {
-      if (client->mapped()) {
+      if (client.mapped()) {
-        client->clear_window();
+        client.clear_window();
       }
     } catch (const std::exception& e) {
-      m_log.err("Failed to clear tray client %s '%s' (%s)", m_connection.id(client->window()),
+      m_log.err("Failed to clear tray client %s '%s' (%s)", m_connection.id(client.window()),
-          ewmh_util::get_wm_name(client->window()), e.what());
+          ewmh_util::get_wm_name(client.window()), e.what());
     }
   }
 
@@ -519,6 +518,11 @@ void tray_manager::create_bg() {
 
   if (!m_pixmap) {
     try {
+      /*
+       * Use depths of bar window.
+       *
+       * TODO store depth determined in renderer somehwere and use that everywhere.
+       */
       auto depth = m_connection.get_geometry(m_bar_opts.window)->depth;
       m_pixmap = m_connection.generate_id();
       m_connection.create_pixmap_checked(depth, m_pixmap, m_tray, w, h);
@@ -695,57 +699,60 @@ void tray_manager::track_selection_owner(xcb_window_t owner) {
 void tray_manager::process_docking_request(xcb_window_t win) {
   m_log.info("Processing docking request from '%s' (%s)", ewmh_util::get_wm_name(win), m_connection.id(win));
 
-  m_clients.emplace_back(std::make_shared<tray_client>(m_connection, win, m_opts.client_size));
+  tray_client client(m_connection, win, m_opts.client_size);
-  auto& client = m_clients.back();
+
+  auto geom = m_connection.get_geometry(win);
+  m_log.trace("tray: depth: %u, width: %u, height: %u", geom->depth, geom->width, geom->height);
 
   try {
-    client->query_xembed();
+    client.query_xembed();
   } catch (const xpp::x::error::window& err) {
     m_log.err("Failed to query _XEMBED_INFO, removing client... (%s)", err.what());
-    remove_client(win, true);
     return;
   }
 
-  m_log.trace("tray: xembed = %s", client->is_xembed_supported() ? "true" : "false");
+  m_log.trace("tray: xembed = %s", client.is_xembed_supported() ? "true" : "false");
-  if (client->is_xembed_supported()) {
+  if (client.is_xembed_supported()) {
-    m_log.trace("tray: version = 0x%x, flags = 0x%x, XEMBED_MAPPED = %s", client->get_xembed().get_version(),
+    m_log.trace("tray: version = 0x%x, flags = 0x%x, XEMBED_MAPPED = %s", client.get_xembed().get_version(),
-        client->get_xembed().get_flags(), client->get_xembed().is_mapped() ? "true" : "false");
+        client.get_xembed().get_flags(), client.get_xembed().is_mapped() ? "true" : "false");
   }
 
   try {
-    const unsigned int mask = XCB_CW_EVENT_MASK;
+    const uint32_t mask = XCB_CW_EVENT_MASK;
-    const unsigned int values[]{XCB_EVENT_MASK_PROPERTY_CHANGE | XCB_EVENT_MASK_STRUCTURE_NOTIFY};
+    const uint32_t value = XCB_EVENT_MASK_PROPERTY_CHANGE | XCB_EVENT_MASK_STRUCTURE_NOTIFY;
 
     m_log.trace("tray: Update client window");
-    m_connection.change_window_attributes_checked(client->window(), mask, values);
+    m_connection.change_window_attributes_checked(client.window(), mask, &value);
 
     m_log.trace("tray: Configure client size");
-    client->reconfigure(0, 0);
+    client.reconfigure(0, 0);
 
     m_log.trace("tray: Add client window to the save set");
-    m_connection.change_save_set_checked(XCB_SET_MODE_INSERT, client->window());
+    m_connection.change_save_set_checked(XCB_SET_MODE_INSERT, client.window());
 
     // TODO properly support tray icon backgrounds
     auto p = XCB_BACK_PIXMAP_NONE;
-    m_connection.change_window_attributes_checked(client->window(), XCB_CW_BACK_PIXMAP, &p);
+    m_connection.change_window_attributes_checked(client.window(), XCB_CW_BACK_PIXMAP, &p);
     m_log.trace("tray: Reparent client");
     m_connection.reparent_window_checked(
-        client->window(), m_tray, calculate_client_x(client->window()), calculate_client_y());
+        client.window(), m_tray, calculate_client_x(client.window()), calculate_client_y());
 
-    if (client->is_xembed_supported()) {
+    if (client.is_xembed_supported()) {
       m_log.trace("tray: Send embbeded notification to client");
-      xembed::notify_embedded(m_connection, client->window(), m_tray, client->get_xembed().get_version());
+      xembed::notify_embedded(m_connection, client.window(), m_tray, client.get_xembed().get_version());
     }
 
-    if (!client->is_xembed_supported() || client->get_xembed().is_mapped()) {
+    if (!client.is_xembed_supported() || client.get_xembed().is_mapped()) {
       m_log.trace("tray: Map client");
-      m_connection.map_window_checked(client->window());
+      m_connection.map_window_checked(client.window());
+    }
+  } catch (const std::exception& err) {
+    m_log.err("Failed to setup tray client '%s' (%s) removing... (%s)", ewmh_util::get_wm_name(win),
+        m_connection.id(win), err.what());
+    return;
   }
 
-  } catch (const std::exception& err) {
+  m_clients.emplace_back(std::move(client));
-    m_log.err("Failed to setup tray client removing... (%s)", err.what());
-    remove_client(win, false);
-  }
 }
 
 /**
@@ -774,8 +781,8 @@ int tray_manager::calculate_y() const {
 unsigned short int tray_manager::calculate_w() const {
   unsigned int width = m_opts.spacing;
   unsigned int count{0};
-  for (auto&& client : m_clients) {
+  for (auto& client : m_clients) {
-    if (client->mapped()) {
+    if (client.mapped()) {
       count++;
       width += m_opts.spacing + m_opts.client_size.w;
     }
@@ -795,7 +802,7 @@ unsigned short int tray_manager::calculate_h() const {
  */
 int tray_manager::calculate_client_x(const xcb_window_t& win) {
   for (unsigned int i = 0; i < m_clients.size(); i++) {
-    if (m_clients[i]->match(win)) {
+    if (m_clients[i].match(win)) {
       return m_opts.spacing + m_opts.client_size.w * i;
     }
   }
@@ -813,17 +820,17 @@ int tray_manager::calculate_client_y() {
  * Check if the given window is embedded
  */
 bool tray_manager::is_embedded(const xcb_window_t& win) const {
-  return m_clients.end() != std::find_if(m_clients.begin(), m_clients.end(),
+  return m_clients.end() !=
-                                [win](shared_ptr<tray_client> client) { return client->match(win); });
+         std::find_if(m_clients.begin(), m_clients.end(), [win](const auto& client) { return client.match(win); });
 }
 
 /**
  * Find tray client by window
  */
-shared_ptr<tray_client> tray_manager::find_client(const xcb_window_t& win) const {
+tray_client* tray_manager::find_client(const xcb_window_t& win) {
-  for (auto&& client : m_clients) {
+  for (auto& client : m_clients) {
-    if (client->match(win)) {
+    if (client.match(win)) {
-      return client;
+      return &client;
     }
   }
   return nullptr;
@@ -832,16 +839,16 @@ shared_ptr<tray_client> tray_manager::find_client(const xcb_window_t& win) const
 /**
  * Remove tray client
  */
-void tray_manager::remove_client(shared_ptr<tray_client>& client, bool reconfigure) {
+void tray_manager::remove_client(const tray_client& client, bool reconfigure) {
-  remove_client(client->window(), reconfigure);
+  remove_client(client.window(), reconfigure);
 }
 
 /**
  * Remove tray client by window
  */
 void tray_manager::remove_client(xcb_window_t win, bool reconfigure) {
-  m_clients.erase(std::remove_if(
+  m_clients.erase(
-      m_clients.begin(), m_clients.end(), [win](shared_ptr<tray_client> client) { return client->match(win); }));
+      std::remove_if(m_clients.begin(), m_clients.end(), [win](const auto& client) { return client.match(win); }));
 
   if (reconfigure) {
     tray_manager::reconfigure();
@@ -852,25 +859,11 @@ void tray_manager::remove_client(xcb_window_t win, bool reconfigure) {
  * Get number of mapped clients
  */
 int tray_manager::mapped_clients() const {
-  int mapped_clients = 0;
+  return std::count_if(m_clients.begin(), m_clients.end(), [](const auto& c) { return c.mapped(); });
-
-  for (auto&& client : m_clients) {
-    if (client->mapped()) {
-      mapped_clients++;
-    }
-  }
-
-  return mapped_clients;
 }
 
 bool tray_manager::has_mapped_clients() const {
-  for (auto&& client : m_clients) {
+  return std::find_if(m_clients.begin(), m_clients.end(), [](const auto& c) { return c.mapped(); }) != m_clients.end();
-    if (client->mapped()) {
-      return true;
-    }
-  }
-
-  return false;
 }
 
 /**

src/x11/xembed.cpp

@@ -1,7 +1,10 @@
 #include "x11/xembed.hpp"
 
+#include <cassert>
+
 #include "errors.hpp"
 #include "x11/atoms.hpp"
+#include "x11/ewmh.hpp"
 
 POLYBAR_NS
 
@@ -104,11 +107,14 @@ namespace xembed {
    * Unembed given window
    */
   void unembed(connection& conn, xcb_window_t win, xcb_window_t root) {
+    assert(win != XCB_NONE);
     try {
       conn.unmap_window_checked(win);
       conn.reparent_window_checked(win, root, 0, 0);
     } catch (const xpp::x::error::window& err) {
       // invalid window
+      // TODO
+      logger::make().err("tray: Failed to unembed window '%s' (%s)", ewmh_util::get_wm_name(win), conn.id(win));
     }
   }
 } // namespace xembed