From 225022be844f4d3dd72acbf6ed4fe51679fb209c Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 11:58:35 +0200
Subject: [PATCH 01/12] tests: Move create_wibox() into a helper script

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 tests/_wibox_helper.lua  | 32 ++++++++++++++++++++++++++++++++
 tests/test-benchmark.lua | 35 ++---------------------------------
 2 files changed, 34 insertions(+), 33 deletions(-)
 create mode 100644 tests/_wibox_helper.lua

diff --git a/tests/_wibox_helper.lua b/tests/_wibox_helper.lua
new file mode 100644
index 000000000..5c19df914
--- /dev/null
+++ b/tests/_wibox_helper.lua
@@ -0,0 +1,32 @@
+local awful = require("awful")
+local cairo = require("lgi").cairo
+local wibox = require("wibox")
+
+return { create_wibox = function()
+    local img = cairo.ImageSurface(cairo.Format.ARGB32, 20, 20)
+
+    -- Widgets that are aligned to the left
+    local left_layout = wibox.layout.fixed.horizontal()
+    left_layout:add(awful.widget.launcher({ image = img, command = "bash" }))
+    left_layout:add(awful.widget.taglist(1, awful.widget.taglist.filter.all))
+    left_layout:add(awful.widget.prompt())
+
+    -- Widgets that are aligned to the right
+    local right_layout = wibox.layout.fixed.horizontal()
+    local textclock = awful.widget.textclock()
+    right_layout:add(textclock)
+    right_layout:add(awful.widget.layoutbox(1))
+
+    -- Now bring it all together (with the tasklist in the middle)
+    local layout = wibox.layout.align.horizontal()
+    layout:set_left(left_layout)
+    layout:set_middle(awful.widget.tasklist(1, awful.widget.tasklist.filter.currenttags))
+    layout:set_right(right_layout)
+
+    -- Create wibox
+    local wb = wibox({ width = 1024, height = 20, screen = 1 })
+    --wb.visible = true
+    wb:set_widget(layout)
+
+    return wb, textclock
+end }
diff --git a/tests/test-benchmark.lua b/tests/test-benchmark.lua
index 4b3c31905..ed7e9e493 100644
--- a/tests/test-benchmark.lua
+++ b/tests/test-benchmark.lua
@@ -2,10 +2,8 @@
 -- that we notice if they break.
 
 local awful = require("awful")
-local wibox = require("wibox")
-local lgi = require("lgi")
-local GLib = lgi.GLib
-local cairo = lgi.cairo
+local GLib = require("lgi").GLib
+local create_wibox = require("_wibox_helper").create_wibox
 
 local not_under_travis = not os.getenv("CI")
 
@@ -41,35 +39,6 @@ local function do_pending_repaint()
     awesome.emit_signal("refresh")
 end
 
-local function create_wibox()
-    local img = cairo.ImageSurface(cairo.Format.ARGB32, 20, 20)
-
-    -- Widgets that are aligned to the left
-    local left_layout = wibox.layout.fixed.horizontal()
-    left_layout:add(awful.widget.launcher({ image = img, command = "bash" }))
-    left_layout:add(awful.widget.taglist(1, awful.widget.taglist.filter.all))
-    left_layout:add(awful.widget.prompt())
-
-    -- Widgets that are aligned to the right
-    local right_layout = wibox.layout.fixed.horizontal()
-    local textclock = awful.widget.textclock()
-    right_layout:add(textclock)
-    right_layout:add(awful.widget.layoutbox(1))
-
-    -- Now bring it all together (with the tasklist in the middle)
-    local layout = wibox.layout.align.horizontal()
-    layout:set_left(left_layout)
-    layout:set_middle(awful.widget.tasklist(1, awful.widget.tasklist.filter.currenttags))
-    layout:set_right(right_layout)
-
-    -- Create wibox
-    local wb = wibox({ width = 1024, height = 20, screen = 1 })
-    wb.visible = true
-    wb:set_widget(layout)
-
-    return wb, textclock
-end
-
 local wb, textclock = create_wibox()
 
 local function relayout_textclock()

From 901c8f680ae797c7ea4adab2f9e85d186b3d232b Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 12:25:20 +0200
Subject: [PATCH 02/12] tests: Add some memory-leak tests

This creates some random collection of widgets and tests if they can be garbage
collected again.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 tests/test-leaks.lua | 49 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 tests/test-leaks.lua

diff --git a/tests/test-leaks.lua b/tests/test-leaks.lua
new file mode 100644
index 000000000..df0b99cda
--- /dev/null
+++ b/tests/test-leaks.lua
@@ -0,0 +1,49 @@
+-- Some memory leak checks as integration tests.
+
+local awful = require("awful")
+local cairo = require("lgi").cairo
+local create_wibox = require("_wibox_helper").create_wibox
+local gears = require("gears")
+local wibox = require("wibox")
+
+local errors = {}
+
+local prepare_for_collect = nil
+
+-- Test if some objects can be garbage collected
+local function collectable(a, b, c, d, e, f, g, h)
+    local objs = setmetatable({ a, b, c, d, e, f, g, h }, { __mode = "v" })
+    a, b, c, d, e, f, g, h = nil, nil, nil, nil, nil, nil, nil, nil
+    if prepare_for_collect then
+        prepare_for_collect()
+        prepare_for_collect = nil
+    end
+    collectgarbage("collect")
+    -- Check if the table is now empty
+    for k, v in pairs(objs) do
+        print("Some object was not garbage collected!")
+        error(v)
+    end
+end
+
+-- First test some basic widgets
+collectable(wibox.widget.base.make_widget())
+collectable(wibox.widget.textbox("foo"))
+collectable(wibox.layout.fixed.horizontal())
+collectable(wibox.layout.align.horizontal())
+
+-- Then some random widgets from awful
+collectable(awful.widget.launcher({ image = cairo.ImageSurface(cairo.Format.ARGB32, 20, 20), command = "bash" }))
+collectable(awful.widget.prompt())
+collectable(awful.widget.textclock())
+collectable(awful.widget.layoutbox(1))
+collectable(awful.widget.taglist(1, awful.widget.taglist.filter.all))
+
+-- And finally a full wibox
+function prepare_for_collect()
+    -- Only after doing the pending repaint can a wibox be GC'd.
+    awesome.emit_signal("refresh")
+end
+collectable(create_wibox())
+
+require("_runner").run_steps({ function() return true end })

From e9377c480857c3edbd6bdf1d97c25690f62273e7 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 12:38:57 +0200
Subject: [PATCH 03/12] Make textclock garbage-collectable

This fixes the textclock-specific part of the test that the previous commit
added.

To fix this, gears.timer.weak_start_new() is used. This function creates a timer
that is automatically stopped when its callback function is garbage collected.
The callback function is saved as a member of the texbox widget that is the
"widget behind the textclock". Thus, the timer can only be stopped after the
widget is garbage-collected, but the timer does not keep the widget alive.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 lib/awful/widget/textclock.lua | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/awful/widget/textclock.lua b/lib/awful/widget/textclock.lua
index 22989a843..6b5d708ee 100644
--- a/lib/awful/widget/textclock.lua
+++ b/lib/awful/widget/textclock.lua
@@ -31,13 +31,14 @@ function textclock.new(format, timeout)
     local timeout = timeout or 60
 
     local w = textbox()
-    local t = timer { timeout = timeout }
-    t:connect_signal("timeout", function()
+    local t
+    function w._textclock_update_cb()
         w:set_markup(DateTime.new_now_local():format(format))
         t.timeout = calc_timeout(timeout)
         t:again()
-    end)
-    t:start()
+        return true -- Continue the timer
+    end
+    t = timer.weak_start_new(timeout, w._textclock_update_cb)
     t:emit_signal("timeout")
     return w
 end

From 6f2e7bba25d2d6ccab635e86d16cdd5584b5fb31 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 13:19:12 +0200
Subject: [PATCH 04/12] Make layoutbox kind-of garbage-collectable

Instead of connecting to the needed tag-update-signal again for every layoutbox,
this now just creates a single connection and updates all layoutboxes from here.
A new weak table is used to find the layoutboxes from these callbacks.
Additionally, layoutboxes are now per-screen unique. So even if you try to
create three layoutboxes for screen 1, the code will now always return the same
instance.

This kind-of fixes the leak test for layoutboxes. The problem is that the
default config also creates a layoutbox and adds it to a wibox. Since this is
now the same layoutbox, the test still fails. Just removing the layoutbox-part
from the default config makes this problem go away.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 lib/awful/widget/layoutbox.lua | 37 +++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/lib/awful/widget/layoutbox.lua b/lib/awful/widget/layoutbox.lua
index 2c0a698c2..2434ab77d 100644
--- a/lib/awful/widget/layoutbox.lua
+++ b/lib/awful/widget/layoutbox.lua
@@ -18,29 +18,46 @@ local imagebox = require("wibox.widget.imagebox")
 
 local layoutbox = { mt = {} }
 
-local function update(w, screen, tooltip)
+local boxes = nil
+
+local function update(w, screen)
     local layout = layout.getname(layout.get(screen))
-    tooltip:set_text(layout or "[no name]")
+    w._layoutbox_tooltip:set_text(layout or "[no name]")
     w:set_image(layout and beautiful["layout_" .. layout])
 end
 
+local function update_from_tag(t)
+    local screen = tag.getscreen(t)
+    local w = boxes[screen]
+    if w then
+        update(w, screen)
+    end
+end
+
 --- Create a layoutbox widget. It draws a picture with the current layout
 -- symbol of the current tag.
 -- @param screen The screen number that the layout will be represented for.
 -- @return An imagebox widget configured as a layoutbox.
 function layoutbox.new(screen)
     local screen = screen or 1
-    local w = imagebox()
-    local tooltip = tooltip({ objects = {w}, delay_show = 1 })
 
-    update(w, screen, tooltip)
-
-    local function update_on_tag_selection(t)
-        return update(w, tag.getscreen(t), tooltip)
+    -- Do we already have the update callbacks registered?
+    if boxes == nil then
+        boxes = setmetatable({}, { __mode = "v" })
+        tag.attached_connect_signal(nil, "property::selected", update_from_tag)
+        tag.attached_connect_signal(nil, "property::layout", update_from_tag)
+        layoutbox.boxes = boxes
     end
 
-    tag.attached_connect_signal(screen, "property::selected", update_on_tag_selection)
-    tag.attached_connect_signal(screen, "property::layout", update_on_tag_selection)
+    -- Do we already have a layoutbox for this screen?
+    local w = boxes[screen]
+    if not w then
+        w = imagebox()
+        w._layoutbox_tooltip = tooltip({ objects = {w}, delay_show = 1 })
+
+        update(w, screen)
+        boxes[screen] = w
+    end
 
     return w
 end

From 40db0b7337359118e53ac3e3f8b8ec65dc3f24e3 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 14:03:59 +0200
Subject: [PATCH 05/12] Make taglists garbage-collectable

Similar to what the previous commit does for layoutboxes, this changes the code
for the taglist so that there is only a single, global connection to the various
signals and these update all taglists via weak tables.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 lib/awful/widget/taglist.lua | 70 ++++++++++++++++++++++--------------
 tests/test-leaks.lua         |  4 +++
 2 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/lib/awful/widget/taglist.lua b/lib/awful/widget/taglist.lua
index 843aea380..44e6e0fc6 100644
--- a/lib/awful/widget/taglist.lua
+++ b/lib/awful/widget/taglist.lua
@@ -27,6 +27,8 @@ local timer = require("gears.timer")
 local taglist = { mt = {} }
 taglist.filter = {}
 
+local instances = nil
+
 function taglist.taglist_label(t, args)
     if not args then args = {} end
     local theme = beautiful.get()
@@ -159,39 +161,53 @@ function taglist.new(screen, filter, buttons, style, update_function, base_widge
     local data = setmetatable({}, { __mode = 'k' })
 
     local queued_update = {}
-    local u = function (s)
-        if s ~= screen then return end
-
+    function w._do_taglist_update()
         -- Add a delayed callback for the first update.
-        if not queued_update[s] then
+        if not queued_update[screen] then
             timer.delayed_call(function()
-                taglist_update(s, w, buttons, filter, data, style, uf)
-                queued_update[s] = false
+                taglist_update(screen, w, buttons, filter, data, style, uf)
+                queued_update[screen] = false
             end)
-            queued_update[s] = true
+            queued_update[screen] = true
         end
     end
-    local uc = function (c) return u(c.screen) end
-    local ut = function (t) return u(tag.getscreen(t)) end
-    capi.client.connect_signal("focus", uc)
-    capi.client.connect_signal("unfocus", uc)
-    tag.attached_connect_signal(screen, "property::selected", ut)
-    tag.attached_connect_signal(screen, "property::icon", ut)
-    tag.attached_connect_signal(screen, "property::hide", ut)
-    tag.attached_connect_signal(screen, "property::name", ut)
-    tag.attached_connect_signal(screen, "property::activated", ut)
-    tag.attached_connect_signal(screen, "property::screen", ut)
-    tag.attached_connect_signal(screen, "property::index", ut)
-    tag.attached_connect_signal(screen, "property::urgent", ut)
-    capi.client.connect_signal("property::screen", function(c, old_screen)
-        if screen == c.screen or screen == old_screen then
-            u(screen)
+    if instances == nil then
+        instances = {}
+        local function u(s)
+            local i = instances[s]
+            if i then
+                for _, tlist in pairs(i) do
+                    tlist._do_taglist_update()
+                end
+            end
         end
-    end)
-    capi.client.connect_signal("tagged", uc)
-    capi.client.connect_signal("untagged", uc)
-    capi.client.connect_signal("unmanage", uc)
-    u(screen)
+        local uc = function (c) return u(c.screen) end
+        local ut = function (t) return u(tag.getscreen(t)) end
+        capi.client.connect_signal("focus", uc)
+        capi.client.connect_signal("unfocus", uc)
+        tag.attached_connect_signal(screen, "property::selected", ut)
+        tag.attached_connect_signal(screen, "property::icon", ut)
+        tag.attached_connect_signal(screen, "property::hide", ut)
+        tag.attached_connect_signal(screen, "property::name", ut)
+        tag.attached_connect_signal(screen, "property::activated", ut)
+        tag.attached_connect_signal(screen, "property::screen", ut)
+        tag.attached_connect_signal(screen, "property::index", ut)
+        tag.attached_connect_signal(screen, "property::urgent", ut)
+        capi.client.connect_signal("property::screen", function(c, old_screen)
+            u(c.screen)
+            u(old_screen)
+        end)
+        capi.client.connect_signal("tagged", uc)
+        capi.client.connect_signal("untagged", uc)
+        capi.client.connect_signal("unmanage", uc)
+    end
+    w._do_taglist_update()
+    local list = instances[s]
+    if not list then
+        list = setmetatable({}, { __mode = "v" })
+        instances[screen] = list
+    end
+    table.insert(list, w)
     return w
 end
 
diff --git a/tests/test-leaks.lua b/tests/test-leaks.lua
index df0b99cda..b7639642e 100644
--- a/tests/test-leaks.lua
+++ b/tests/test-leaks.lua
@@ -37,6 +37,10 @@ collectable(awful.widget.launcher({ image = cairo.ImageSurface(cairo.Format.ARGB
 collectable(awful.widget.prompt())
 collectable(awful.widget.textclock())
 collectable(awful.widget.layoutbox(1))
+function prepare_for_collect()
+    -- Only after doing the pending update can a taglist be GC'd.
+    awesome.emit_signal("refresh")
+end
 collectable(awful.widget.taglist(1, awful.widget.taglist.filter.all))
 
 -- And finally a full wibox

From abc5a552a494b5cab0a842afb50cebf7a4fa4c52 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 14:10:45 +0200
Subject: [PATCH 06/12] Make tasklist garbage-collectable

Again, instead of directly connecting to various signals for updating a
tasklist, this commit changes the code so that there is just a single, global
connections and based on this a weak table with all tasklist instances is used
do the updates.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 lib/awful/widget/tasklist.lua | 80 +++++++++++++++++++++++------------
 tests/test-leaks.lua          | 10 ++++-
 2 files changed, 61 insertions(+), 29 deletions(-)

diff --git a/lib/awful/widget/tasklist.lua b/lib/awful/widget/tasklist.lua
index 401fe531c..294f0305e 100644
--- a/lib/awful/widget/tasklist.lua
+++ b/lib/awful/widget/tasklist.lua
@@ -23,6 +23,8 @@ local timer = require("gears.timer")
 
 local tasklist = { mt = {} }
 
+local instances
+
 -- Public structures
 tasklist.filter = {}
 
@@ -169,7 +171,7 @@ function tasklist.new(screen, filter, buttons, style, update_function, base_widg
     local data = setmetatable({}, { __mode = 'k' })
 
     local queued_update = false
-    local u = function ()
+    function w._do_tasklist_update()
         -- Add a delayed callback for the first update.
         if not queued_update then
             timer.delayed_call(function()
@@ -179,34 +181,56 @@ function tasklist.new(screen, filter, buttons, style, update_function, base_widg
             queued_update = true
         end
     end
-    tag.attached_connect_signal(screen, "property::selected", u)
-    tag.attached_connect_signal(screen, "property::activated", u)
-    capi.client.connect_signal("property::urgent", u)
-    capi.client.connect_signal("property::sticky", u)
-    capi.client.connect_signal("property::ontop", u)
-    capi.client.connect_signal("property::above", u)
-    capi.client.connect_signal("property::below", u)
-    capi.client.connect_signal("property::floating", u)
-    capi.client.connect_signal("property::maximized_horizontal", u)
-    capi.client.connect_signal("property::maximized_vertical", u)
-    capi.client.connect_signal("property::minimized", u)
-    capi.client.connect_signal("property::name", u)
-    capi.client.connect_signal("property::icon_name", u)
-    capi.client.connect_signal("property::icon", u)
-    capi.client.connect_signal("property::skip_taskbar", u)
-    capi.client.connect_signal("property::screen", function(c, old_screen)
-        if screen == c.screen or screen == old_screen then
-            u()
+    if instances == nil then
+        instances = {}
+        local function us(s)
+            local i = instances[s]
+            if i then
+                for _, tlist in pairs(i) do
+                    tlist._do_tasklist_update()
+                end
+            end
         end
-    end)
-    capi.client.connect_signal("property::hidden", u)
-    capi.client.connect_signal("tagged", u)
-    capi.client.connect_signal("untagged", u)
-    capi.client.connect_signal("unmanage", u)
-    capi.client.connect_signal("list", u)
-    capi.client.connect_signal("focus", u)
-    capi.client.connect_signal("unfocus", u)
-    u()
+        local function u()
+            for s in ipairs(instances) do
+                us(s)
+            end
+        end
+
+        tag.attached_connect_signal(nil, "property::selected", u)
+        tag.attached_connect_signal(nil, "property::activated", u)
+        capi.client.connect_signal("property::urgent", u)
+        capi.client.connect_signal("property::sticky", u)
+        capi.client.connect_signal("property::ontop", u)
+        capi.client.connect_signal("property::above", u)
+        capi.client.connect_signal("property::below", u)
+        capi.client.connect_signal("property::floating", u)
+        capi.client.connect_signal("property::maximized_horizontal", u)
+        capi.client.connect_signal("property::maximized_vertical", u)
+        capi.client.connect_signal("property::minimized", u)
+        capi.client.connect_signal("property::name", u)
+        capi.client.connect_signal("property::icon_name", u)
+        capi.client.connect_signal("property::icon", u)
+        capi.client.connect_signal("property::skip_taskbar", u)
+        capi.client.connect_signal("property::screen", function(c, old_screen)
+            us(c.screen)
+            us(old_screen)
+        end)
+        capi.client.connect_signal("property::hidden", u)
+        capi.client.connect_signal("tagged", u)
+        capi.client.connect_signal("untagged", u)
+        capi.client.connect_signal("unmanage", u)
+        capi.client.connect_signal("list", u)
+        capi.client.connect_signal("focus", u)
+        capi.client.connect_signal("unfocus", u)
+    end
+    w._do_tasklist_update()
+    local list = instances[s]
+    if not list then
+        list = setmetatable({}, { __mode = "v" })
+        instances[screen] = list
+    end
+    table.insert(list, w)
     return w
 end
 
diff --git a/tests/test-leaks.lua b/tests/test-leaks.lua
index b7639642e..dd36ab9a7 100644
--- a/tests/test-leaks.lua
+++ b/tests/test-leaks.lua
@@ -11,7 +11,8 @@ local errors = {}
 local prepare_for_collect = nil
 
 -- Test if some objects can be garbage collected
-local function collectable(a, b, c, d, e, f, g, h)
+local function collectable(a, b, c, d, e, f, g, h, last)
+    assert(last == nil, "got more arguments than supported")
     local objs = setmetatable({ a, b, c, d, e, f, g, h }, { __mode = "v" })
     a, b, c, d, e, f, g, h = nil, nil, nil, nil, nil, nil, nil, nil
     if prepare_for_collect then
@@ -37,12 +38,19 @@ collectable(awful.widget.launcher({ image = cairo.ImageSurface(cairo.Format.ARGB
 collectable(awful.widget.prompt())
 collectable(awful.widget.textclock())
 collectable(awful.widget.layoutbox(1))
+
 function prepare_for_collect()
     -- Only after doing the pending update can a taglist be GC'd.
     awesome.emit_signal("refresh")
 end
 collectable(awful.widget.taglist(1, awful.widget.taglist.filter.all))
 
+function prepare_for_collect()
+    -- Only after doing the pending update can a taglist be GC'd.
+    awesome.emit_signal("refresh")
+end
+collectable(awful.widget.tasklist(1, awful.widget.tasklist.filter.currenttags))
+
 -- And finally a full wibox
 function prepare_for_collect()
     -- Only after doing the pending repaint can a wibox be GC'd.

From bd47edb4ef692e04a949c39c8a90f552382d589f Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 14:42:39 +0200
Subject: [PATCH 07/12] leak test: Make the layoutbox test pass

This commit does two things: It gets rid of the reference to the layoutbox that
the default config created and it changes the widget dependency cache to not
keep widgets alive unnecessarily.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 lib/wibox/widget/base.lua | 2 +-
 tests/test-leaks.lua      | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/wibox/widget/base.lua b/lib/wibox/widget/base.lua
index 26203581f..6dbd05da9 100644
--- a/lib/wibox/widget/base.lua
+++ b/lib/wibox/widget/base.lua
@@ -18,7 +18,7 @@ local base = {}
 -- {{{ Caches
 
 -- Indexes are widgets, allow them to be garbage-collected
-local widget_dependencies = setmetatable({}, { __mode = "k" })
+local widget_dependencies = setmetatable({}, { __mode = "kv" })
 
 -- Get the cache of the given kind for this widget. This returns a gears.cache
 -- that calls the callback of kind `kind` on the widget.
diff --git a/tests/test-leaks.lua b/tests/test-leaks.lua
index dd36ab9a7..e91727a80 100644
--- a/tests/test-leaks.lua
+++ b/tests/test-leaks.lua
@@ -10,6 +10,12 @@ local errors = {}
 
 local prepare_for_collect = nil
 
+-- Make the layoutbox in the default config GC'able
+mywibox[1].visible = false
+mywibox = nil
+mylayoutbox = nil
+awesome.emit_signal("refresh")
+
 -- Test if some objects can be garbage collected
 local function collectable(a, b, c, d, e, f, g, h, last)
     assert(last == nil, "got more arguments than supported")

From dc2147208f0280a7ef974f8aa8a0296164a84d43 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 14:44:19 +0200
Subject: [PATCH 08/12] tests: Return more widgets from create_wibox()

All of these are checked for being GC'able in test-leaks.lua.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 tests/_wibox_helper.lua | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/_wibox_helper.lua b/tests/_wibox_helper.lua
index 5c19df914..3612374ad 100644
--- a/tests/_wibox_helper.lua
+++ b/tests/_wibox_helper.lua
@@ -28,5 +28,5 @@ return { create_wibox = function()
     --wb.visible = true
     wb:set_widget(layout)
 
-    return wb, textclock
+    return wb, textclock, img, left_layout, right_layout, layout
 end }

From cd1ad8b753b554cfb259e8bc0ca92403d3c08b19 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 15:07:22 +0200
Subject: [PATCH 09/12] Don't measure time for creating wiboxes

Apparently some of the last commits speeds up create_wibox() a lot. This
highlights that this is a bad test: After creating thousands of wiboxes, awesome
needed 15 seconds to draw all of them and in the end some dbus timeout aborted
the test run.

However, it's irrelevant how quickly we can create wibox. The interesting number
is how quickly we can display a new wibox. Thus, this commits changes the code
so that it also measures the time that is needed to update the wibox. This way,
we don't accumulate a huge number of pending repaints and everything's fine.

Some results (but there is nothing to compare this with):
   create&draw wibox: 0.0373947  sec/iter ( 28 iters, 1.59 sec for benchmark)
    update textclock: 0.00198174 sec/iter (515 iters, 1.937 sec for benchmark)
  relayout textclock: 0.000614439 sec/iter (1710 iters, 1.051 sec for benchmark)
    redraw textclock: 0.00116882 sec/iter (865 iters, 2.962 sec for benchmark)
          tag switch: 0.000705579 sec/iter (1498 iters, 3.703 sec for benchmark)

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 tests/test-benchmark.lua | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/test-benchmark.lua b/tests/test-benchmark.lua
index ed7e9e493..065b8e47e 100644
--- a/tests/test-benchmark.lua
+++ b/tests/test-benchmark.lua
@@ -39,6 +39,11 @@ local function do_pending_repaint()
     awesome.emit_signal("refresh")
 end
 
+local function create_and_draw_wibox()
+    create_wibox()
+    do_pending_repaint()
+end
+
 local wb, textclock = create_wibox()
 
 local function relayout_textclock()
@@ -61,7 +66,7 @@ local function e2e_tag_switch()
     do_pending_repaint()
 end
 
-benchmark(create_wibox, "create wibox")
+benchmark(create_and_draw_wibox, "create&draw wibox")
 benchmark(update_textclock, "update textclock")
 benchmark(relayout_textclock, "relayout textclock")
 benchmark(redraw_textclock, "redraw textclock")

From f7799cbb7fb21483ba2df825aff5a4eb3a29f496 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 15:30:04 +0200
Subject: [PATCH 10/12] Refactior test-leaks a bit

I think it looks nicer with this helper function.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 tests/test-leaks.lua | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/tests/test-leaks.lua b/tests/test-leaks.lua
index e91727a80..32797f3ef 100644
--- a/tests/test-leaks.lua
+++ b/tests/test-leaks.lua
@@ -9,12 +9,15 @@ local wibox = require("wibox")
 local errors = {}
 
 local prepare_for_collect = nil
+local function emit_refresh()
+    awesome.emit_signal("refresh")
+end
 
 -- Make the layoutbox in the default config GC'able
 mywibox[1].visible = false
 mywibox = nil
 mylayoutbox = nil
-awesome.emit_signal("refresh")
+emit_refresh()
 
 -- Test if some objects can be garbage collected
 local function collectable(a, b, c, d, e, f, g, h, last)
@@ -45,23 +48,14 @@ collectable(awful.widget.prompt())
 collectable(awful.widget.textclock())
 collectable(awful.widget.layoutbox(1))
 
-function prepare_for_collect()
-    -- Only after doing the pending update can a taglist be GC'd.
-    awesome.emit_signal("refresh")
-end
+-- Some widgets do things via timer.delayed_call
+prepare_for_collect = emit_refresh
 collectable(awful.widget.taglist(1, awful.widget.taglist.filter.all))
 
-function prepare_for_collect()
-    -- Only after doing the pending update can a taglist be GC'd.
-    awesome.emit_signal("refresh")
-end
+prepare_for_collect = emit_refresh
 collectable(awful.widget.tasklist(1, awful.widget.tasklist.filter.currenttags))
 
--- And finally a full wibox
-function prepare_for_collect()
-    -- Only after doing the pending repaint can a wibox be GC'd.
-    awesome.emit_signal("refresh")
-end
+prepare_for_collect = emit_refresh
 collectable(create_wibox())
 
 require("_runner").run_steps({ function() return true end })

From 94271e8c913bac1245ebc94a5654919ccb407b5a Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 15:31:42 +0200
Subject: [PATCH 11/12] test-leaks.lua: Also test tooltips

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 tests/test-leaks.lua | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/test-leaks.lua b/tests/test-leaks.lua
index 32797f3ef..4bae79c71 100644
--- a/tests/test-leaks.lua
+++ b/tests/test-leaks.lua
@@ -36,6 +36,20 @@ local function collectable(a, b, c, d, e, f, g, h, last)
     end
 end
 
+-- Use the layoutbox for testing delayed tooltips
+local function tooltip_delayed()
+    local l = awful.widget.layoutbox(1)
+    local t = l._layoutbox_tooltip
+    assert(t)
+    return l, t
+end
+
+local function tooltip_now()
+    local w = wibox.widget.textbox("some textbox")
+    local t = awful.tooltip({ objects = {w} })
+    return w, t
+end
+
 -- First test some basic widgets
 collectable(wibox.widget.base.make_widget())
 collectable(wibox.widget.textbox("foo"))
@@ -49,6 +63,12 @@ collectable(awful.widget.textclock())
 collectable(awful.widget.layoutbox(1))
 
 -- Some widgets do things via timer.delayed_call
+prepare_for_collect = emit_refresh
+collectable(tooltip_delayed())
+
+prepare_for_collect = emit_refresh
+collectable(tooltip_now())
+
 prepare_for_collect = emit_refresh
 collectable(awful.widget.taglist(1, awful.widget.taglist.filter.all))
 

From 3e9fdea650e5b65cc70901f563a3ecaa6f0721ab Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sun, 27 Sep 2015 16:04:20 +0200
Subject: [PATCH 12/12] test-leaks: Fix with Lua 5.1

I have no idea why this needs collectgarbage() to be called twice.

On the other hand, I can explain the change in tooltip.lua. Lua 5.2 introduced
"ephermeron tables". This means that in the following sitation, lua 5.2 can
collect the entry from the table, while 5.1 keeps the entry alive, because the
table has a strong reference to the value and that in turn has a strong
reference to the key:

  t = setmetatable({}, { __mode = "k"})
  do
    local k = {}
    t[k] = function() print(k) end
  end
  collectgarbage("collect")
  print(next(t, nil))

To handle this incompatibility, this commit just removes the whole indirection
through the module-level variable "data".

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 lib/awful/tooltip.lua | 79 +++++++++++++++++++------------------------
 tests/test-leaks.lua  |  1 +
 2 files changed, 35 insertions(+), 45 deletions(-)

diff --git a/lib/awful/tooltip.lua b/lib/awful/tooltip.lua
index 91e377245..653a1823c 100644
--- a/lib/awful/tooltip.lua
+++ b/lib/awful/tooltip.lua
@@ -59,17 +59,6 @@ local ipairs = ipairs
 -- @tfield boolean visible True if tooltip is visible.
 local tooltip = { mt = {}  }
 
---- Tooltip private data.
--- @local
--- @table tooltip.data
--- @tfield string fg tooltip foreground color.
--- @tfield string font Tooltip font.
--- @tfield function hide The hide() function.
--- @tfield function show The show() function.
--- @tfield gears.timer timer The text update timer.
--- @tfield function timer_function The text update timer function.
-local data = setmetatable({}, { __mode = 'k' })
-
 -- Place the tooltip on the screen.
 -- @tparam tooltip self A tooltip object.
 local function place(self)
@@ -98,10 +87,10 @@ end
 local function show(self)
     -- do nothing if the tooltip is already shown
     if self.visible then return end
-    if data[self].timer then
-        if not data[self].timer.started then
-            data[self].timer_function()
-            data[self].timer:start()
+    if self.timer then
+        if not self.timer.started then
+            self.timer_function()
+            self.timer:start()
         end
     end
     set_geometry(self)
@@ -116,9 +105,9 @@ end
 local function hide(self)
     -- do nothing if the tooltip is already hidden
     if not self.visible then return end
-    if data[self].timer then
-        if data[self].timer.started then
-            data[self].timer:stop()
+    if self.timer then
+        if self.timer.started then
+            self.timer:stop()
         end
     end
     self.visible = false
@@ -154,8 +143,8 @@ end
 -- @tparam tooltip self A tooltip object.
 -- @tparam number timeout The timeout value.
 tooltip.set_timeout = function(self, timeout)
-    if data[self].timer then
-        data[self].timer.timeout = timeout
+    if self.timer then
+        self.timer.timeout = timeout
     end
 end
 
@@ -165,8 +154,8 @@ end
 -- @tparam gears.object object An object with `mouse::enter` and
 --   `mouse::leave` signals.
 tooltip.add_to_object = function(self, object)
-    object:connect_signal("mouse::enter", data[self].show)
-    object:connect_signal("mouse::leave", data[self].hide)
+    object:connect_signal("mouse::enter", self.show)
+    object:connect_signal("mouse::leave", self.hide)
 end
 
 --- Remove tooltip from an object.
@@ -175,8 +164,8 @@ end
 -- @tparam gears.object object An object with `mouse::enter` and
 --   `mouse::leave` signals.
 tooltip.remove_from_object = function(self, object)
-    object:disconnect_signal("mouse::enter", data[self].show)
-    object:disconnect_signal("mouse::leave", data[self].hide)
+    object:disconnect_signal("mouse::enter", self.show)
+    object:disconnect_signal("mouse::leave", self.hide)
 end
 
 
@@ -213,24 +202,24 @@ tooltip.new = function(args)
             delay_timeout:stop()
         end)
 
-        data[self] = {
-            show = function()
-                if not delay_timeout.started then
-                    delay_timeout:start()
-                end
-            end,
-            hide = function()
-                if delay_timeout.started then
-                    delay_timeout:stop()
-                end
-                hide(self)
-            end,
-        }
+        function self.show()
+            if not delay_timeout.started then
+                delay_timeout:start()
+            end
+        end
+        function self.hide()
+            if delay_timeout.started then
+                delay_timeout:stop()
+            end
+            hide(self)
+        end
     else
-        data[self] = {
-            show = function() show(self) end,
-            hide = function() hide(self) end,
-        }
+        function self.show()
+            show(self)
+        end
+        function self.hide()
+            hide(self)
+        end
     end
 
     -- export functions
@@ -242,11 +231,11 @@ tooltip.new = function(args)
 
     -- setup the timer action only if needed
     if args.timer_function then
-        data[self].timer = timer { timeout = args.timeout and args.timeout or 1 }
-        data[self].timer_function = function()
+        self.timer = timer { timeout = args.timeout and args.timeout or 1 }
+        self.timer_function = function()
                 self:set_markup(args.timer_function())
             end
-        data[self].timer:connect_signal("timeout", data[self].timer_function)
+        self.timer:connect_signal("timeout", self.timer_function)
     end
 
     -- Set default properties
@@ -274,7 +263,7 @@ tooltip.new = function(args)
 
     -- Close the tooltip when clicking it.  This gets done on release, to not
     -- emit the release event on an underlying object, e.g. the titlebar icon.
-    self.wibox:buttons(abutton({}, 1, nil, function() data[self].hide() end))
+    self.wibox:buttons(abutton({}, 1, nil, function() self.hide() end))
 
     -- Re-place when the geometry of the wibox changes.
     self.wibox:connect_signal("property::width",  function() place(self) end)
diff --git a/tests/test-leaks.lua b/tests/test-leaks.lua
index 4bae79c71..f8b06bce8 100644
--- a/tests/test-leaks.lua
+++ b/tests/test-leaks.lua
@@ -29,6 +29,7 @@ local function collectable(a, b, c, d, e, f, g, h, last)
         prepare_for_collect = nil
     end
     collectgarbage("collect")
+    collectgarbage("collect")
     -- Check if the table is now empty
     for k, v in pairs(objs) do
         print("Some object was not garbage collected!")