Merge pull request #1646 from psychon/systray_removal

Partly fix removal of systray from a wibox

lib/wibox/drawable.lua

@@ -25,6 +25,8 @@ local unpack = unpack or table.unpack -- luacheck: globals unpack (compatibility
 
 local visible_drawables = {}
 
+local systray_widget
+
 -- Get the widget context. This should always return the same table (if
 -- possible), so that our draw and fit caches can work efficiently.
 local function get_widget_context(self)
@@ -76,8 +78,15 @@ local function do_redraw(self)
     if self._need_relayout or self._need_complete_repaint then
         self._need_relayout = false
         if self._widget_hierarchy and self.widget then
+            local had_systray = systray_widget and self._widget_hierarchy:get_count(systray_widget) > 0
+
             self._widget_hierarchy:update(context,
                 self.widget, width, height, self._dirty_area)
+
+            local has_systray = systray_widget and self._widget_hierarchy:get_count(systray_widget) > 0
+            if had_systray and not has_systray then
+                systray_widget:_kickout(context)
+            end
         else
             self._need_complete_repaint = true
             if self.widget then
@@ -209,6 +218,11 @@ function drawable:find_widgets(x, y)
     return result
 end
 
+-- Private API. Not documented on purpose.
+function drawable._set_systray_widget(widget)
+    hierarchy.count_widget(widget)
+    systray_widget = widget
+end
 
 --- Set the widget that the drawable displays
 function drawable:set_widget(widget)

lib/wibox/hierarchy.lua

@@ -16,6 +16,16 @@ local no_parent = base.no_parent_I_know_what_I_am_doing
 
 local hierarchy = {}
 
+local widgets_to_count = setmetatable({}, { __mode = "k" })
+
+--- Add a widget to the list of widgets for which hierarchies should count their
+-- occurrences. Note that for correct operations, the widget must not yet be
+-- visible in any hierarchy.
+-- @param widget The widget that should be counted.
+function hierarchy.count_widget(widget)
+    widgets_to_count[widget] = true
+end
+
 local function hierarchy_new(redraw_callback, layout_callback, callback_arg)
     local result = {
         _matrix = matrix.identity,
@@ -37,7 +47,8 @@ local function hierarchy_new(redraw_callback, layout_callback, callback_arg)
             height = 0
         },
         _parent = nil,
-        _children = {}
+        _children = {},
+        _widget_counts = {},
     }
 
     function result._redraw()
@@ -144,6 +155,17 @@ function hierarchy_update(self, context, widget, width, height, region, matrix_t
         height = y2 - y1
     }
 
+    -- Update widget counts
+    self._widget_counts = {}
+    if widgets_to_count[widget] and width > 0 and height > 0 then
+        self._widget_counts[widget] = 1
+    end
+    for _, h in ipairs(self._children) do
+        for w, count in pairs(h._widget_counts) do
+            self._widget_counts[w] = (self._widget_counts[w] or 0) + count
+        end
+    end
+
     -- Check which part needs to be redrawn
 
     -- Are there any children which were removed? Their area needs a redraw.
@@ -260,6 +282,14 @@ function hierarchy:get_children()
     return self._children
 end
 
+--- Count how often this widget is visible inside this hierarchy. This function
+-- only works with widgets registered via `count_widget`.
+-- @param widget The widget that should be counted
+-- @return The number of times that this widget is contained in this hierarchy.
+function hierarchy:get_count(widget)
+    return self._widget_counts[widget] or 0
+end
+
 --- Does the given cairo context have an empty clip (aka "no drawing possible")?
 local function empty_clip(cr)
     local _, _, width, height = cr:clip_extents()

lib/wibox/widget/systray.lua

@@ -5,6 +5,7 @@
 ---------------------------------------------------------------------------
 
 local wbase = require("wibox.widget.base")
+local drawable = require("wibox.drawable")
 local beautiful = require("beautiful")
 local gtable = require("gears.table")
 local capi = {
@@ -72,6 +73,13 @@ function systray:draw(context, cr, width, height)
                          base, is_rotated, bg, reverse, spacing)
 end
 
+-- Private API. Does not appear in LDoc on purpose. This function is called
+-- some time after the systray is removed from some drawable. It's purpose is to
+-- really remove the systray.
+function systray:_kickout(context)
+    capi.awesome.systray(context.wibox.drawin)
+end
+
 function systray:fit(context, width, height)
     if not should_display_on(context.screen) then
         return 0, 0
@@ -153,7 +161,7 @@ end
 -- @function wibox.widget.systray
 
 local function new(revers)
-    local ret = wbase.make_widget()
+    local ret = wbase.make_widget(nil, nil, {enable_properties = true})
 
     gtable.crush(ret, systray, true)
 
@@ -171,6 +179,8 @@ local function new(revers)
         end
     end)
 
+    drawable._set_systray_widget(ret)
+
     return ret
 end
 
@@ -181,6 +191,10 @@ function systray.mt:__call(...)
     return instance
 end
 
+--@DOC_widget_COMMON@
+
+--@DOC_object_COMMON@
+
 return setmetatable(systray, systray.mt)
 
 -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80

objects/drawin.c

@@ -166,6 +166,12 @@ drawin_systray_kickout(drawin_t *w)
     }
 }
 
+void
+luaA_drawin_systray_kickout(lua_State *L)
+{
+    drawin_systray_kickout(luaA_checkudata(L, 1, &drawin_class));
+}
+
 static void
 drawin_wipe(drawin_t *w)
 {

objects/drawin.h

@@ -47,8 +47,8 @@ struct drawin_t
 ARRAY_FUNCS(drawin_t *, drawin, DO_NOTHING)
 
 drawin_t * drawin_getbywin(xcb_window_t);
-
 void drawin_refresh_pixmap_partial(drawin_t *, int16_t, int16_t, uint16_t, uint16_t);
+void luaA_drawin_systray_kickout(lua_State *);
 
 void drawin_class_setup(lua_State *);
 

spec/wibox/hierarchy_spec.lua

@@ -259,6 +259,67 @@ describe("wibox.hierarchy", function()
             assert.is.same({ rect.x, rect.y, rect.width, rect.height }, { 4, 0, 5, 2 })
         end)
     end)
+
+    describe("widget counts", function()
+        local child, intermediate, parent
+        local unrelated
+        local context, instance
+        before_each(function()
+            local function nop() end
+            context = {}
+            child = make_widget(nil)
+            intermediate = make_widget({
+                make_child(child, 10, 20, matrix.identity)
+            })
+            parent = make_widget({
+                make_child(intermediate, 10, 20, matrix.identity),
+                make_child(child, 0, 20, matrix.identity)
+            })
+            unrelated = make_widget(nil)
+
+            hierarchy.count_widget(child)
+            hierarchy.count_widget(parent)
+            hierarchy.count_widget(unrelated)
+            instance = hierarchy.new(context, parent, 10, 20, nop, nop)
+        end)
+
+        it("basic counts", function()
+            local unrelated_other = make_widget(nil)
+            assert.is.equal(1, instance:get_count(child))
+            -- intermediate was not passed to hierarchy.count_widget()!
+            assert.is.equal(0, instance:get_count(intermediate))
+            assert.is.equal(1, instance:get_count(parent))
+            assert.is.equal(0, instance:get_count(unrelated))
+            assert.is.equal(0, instance:get_count(unrelated_other))
+        end)
+
+        it("after update", function()
+            -- Replace child and intermediate by just a new_child
+            local new_child = make_widget(nil)
+            parent.layout = function()
+                return { make_child(new_child, 10, 20, matrix.identity) }
+            end
+            parent:emit_signal("widget::layout_changed")
+            instance:update(context, parent, 10, 20)
+
+            assert.is.equal(0, instance:get_count(child))
+            -- new_child was not passed to hierarchy.count_widget()!
+            assert.is.equal(0, instance:get_count(new_child))
+            assert.is.equal(1, instance:get_count(parent))
+            assert.is.equal(0, instance:get_count(unrelated))
+        end)
+
+        it("collectible", function()
+            -- This test that hierarchy.count_widget() does not prevent garbage collection of the widget.
+            local weak = setmetatable({}, { __mode = "v"})
+            weak[1], weak[2], weak[3], weak[4] = child, intermediate, parent, instance
+            child, intermediate, parent, instance = nil, nil, nil, nil
+
+            assert.is.equal(4, #weak)
+            collectgarbage("collect")
+            assert.is.equal(0, #weak)
+        end)
+    end)
 end)
 
 -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80

spec/wibox/test_utils.lua

@@ -96,6 +96,12 @@ return {
             end
         end
         w._private.widget_caches = {}
+        w:connect_signal("widget::layout_changed", function()
+            -- TODO: This is not completely correct, since our parent's caches
+            -- are not cleared. For the time being, tests just have to handle
+            -- this clearing-part themselves.
+            w._private.widget_caches = {}
+        end)
 
         return w
     end,

systray.c

@@ -337,7 +337,10 @@ luaA_systray(lua_State *L)
 {
     systray_register();
 
-    if(lua_gettop(L) != 0)
+    if(lua_gettop(L) == 1)
+        luaA_drawin_systray_kickout(L);
+
+    if(lua_gettop(L) > 1)
     {
         size_t bg_len;
         drawin_t *w = luaA_checkudata(L, 1, &drawin_class);