diff --git a/lib/signals.js b/lib/signals.js index cdaaf27..516f319 100644 --- a/lib/signals.js +++ b/lib/signals.js @@ -24,6 +24,7 @@ export class SignalListener { this.controller = controller; this._signals = []; + this._pendingIdles = new Set(); } bind() { @@ -85,15 +86,21 @@ export class SignalListener { _addWindow(window) { if (!window) return; - GLib.idle_add(GLib.PRIORITY_DEFAULT, () => { + const sourceId = GLib.idle_add(GLib.PRIORITY_DEFAULT, () => { + this._pendingIdles.delete(sourceId); + if (!SignalListener.activeInstance) return GLib.SOURCE_REMOVE; if (this._shouldTile(window)) { this.controller.tilingRequest(window); } return GLib.SOURCE_REMOVE; }); + this._pendingIdles.add(sourceId); } unbind() { + this._pendingIdles.forEach(id => GLib.source_remove(id)); + this._pendingIdles.clear(); + this._signals.forEach(({ obj, id }) => { try { if (obj) { diff --git a/lib/window.js b/lib/window.js index 36b84a2..fed194e 100644 --- a/lib/window.js +++ b/lib/window.js @@ -15,6 +15,7 @@ export class WindowWrapper { this.monitorId = null; this._sizeChangedHandled = false; + this._pendingLaters = []; } get unmanaged() { @@ -81,6 +82,12 @@ export class WindowWrapper { } destroy() { + const laters = global.compositor.get_laters(); + for (const id of this._pendingLaters) { + try { laters.remove(id); } catch (e) { /* already fired */ } + } + this._pendingLaters = []; + const keys = Array.from(this.signals.keys()); for (const name of keys) { this.disconnectSignal(name); @@ -99,12 +106,12 @@ export class WindowWrapper { if (this.window.maximized_horizontally || this.window.maximized_vertically) { this.window.unmaximize(); - // Delay unmaximize via compositor. - global.compositor.get_laters().add(Meta.LaterType.BEFORE_REDRAW, () => { + const laterId = global.compositor.get_laters().add(Meta.LaterType.BEFORE_REDRAW, () => { if (this.unmanaged) return false; this._doResize(rect); return false; }); + this._pendingLaters.push(laterId); } else { this._doResize(rect); } @@ -123,10 +130,11 @@ export class WindowWrapper { Math.round(rect.width), Math.round(rect.height) ); - global.compositor.get_laters().add(Meta.LaterType.BEFORE_REDRAW, () => { + const laterId = global.compositor.get_laters().add(Meta.LaterType.BEFORE_REDRAW, () => { this._isResizing = false; return false; }); + this._pendingLaters.push(laterId); } catch (e) { this._isResizing = false; Logger.warn(`Resize failed for "${this.title}"`, e); diff --git a/tests/setup.js b/tests/setup.js index 7df516f..89a5887 100644 --- a/tests/setup.js +++ b/tests/setup.js @@ -1,22 +1,28 @@ import { vi } from 'vitest'; // Mock GNOME's 'gi://' imports which don't exist in Node environment -vi.mock('gi://GLib', () => ({ - default: { - idle_add: vi.fn((priority, callback) => { - callback(); // Execute immediately in tests - return 1; - }), - timeout_add: vi.fn((priority, interval, callback) => { - callback(); // Execute immediately in tests - return 1; - }), - source_remove: vi.fn(), - SOURCE_REMOVE: false, - PRIORITY_DEFAULT: 0, - PRIORITY_DEFAULT_IDLE: 0 - } -})); +vi.mock('gi://GLib', () => { + let _idleCounter = 0; + let _timeoutCounter = 0; + return { + default: { + idle_add: vi.fn((priority, callback) => { + const id = ++_idleCounter; + callback(); + return id; + }), + timeout_add: vi.fn((priority, interval, callback) => { + const id = ++_timeoutCounter; + callback(); + return id; + }), + source_remove: vi.fn(), + SOURCE_REMOVE: false, + PRIORITY_DEFAULT: 0, + PRIORITY_DEFAULT_IDLE: 0 + } + }; +}); vi.mock('gi://St', () => ({ default: { @@ -78,11 +84,13 @@ global.workspace_manager = { list_windows: vi.fn(() => []) })) }; +let _laterCounter = 0; global.compositor = { get_laters: vi.fn(() => ({ add: vi.fn((type, callback) => { + const id = ++_laterCounter; callback(); - return 1; + return id; }), remove: vi.fn() })) diff --git a/tests/signals.test.js b/tests/signals.test.js new file mode 100644 index 0000000..251d4ac --- /dev/null +++ b/tests/signals.test.js @@ -0,0 +1,85 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import GLib from 'gi://GLib'; +import { SignalListener, TILABLE_WINDOW_TYPES } from '../lib/signals.js'; +import Meta from 'gi://Meta'; + +describe('SignalListener', () => { + let mockController; + + beforeEach(() => { + SignalListener.activeInstance = null; + vi.clearAllMocks(); + + mockController = { + tilingRequest: vi.fn(), + handleMonitorsChanged: vi.fn(), + startDragTracking: vi.fn(), + endDragTracking: vi.fn(), + hydrate: vi.fn(), + }; + }); + + afterEach(() => { + SignalListener.activeInstance = null; + }); + + describe('_pendingIdles tracking', () => { + it('should initialize _pendingIdles as empty Set', () => { + const listener = new SignalListener(mockController); + expect(listener._pendingIdles).toBeInstanceOf(Set); + expect(listener._pendingIdles.size).toBe(0); + }); + + it('should track idle source IDs when _addWindow is called', () => { + // Override idle_add to NOT execute callback (simulate async) + GLib.idle_add = vi.fn((priority, cb) => 42); + + const listener = new SignalListener(mockController); + const mockWindow = { + get_window_type: () => Meta.WindowType.NORMAL, + is_skip_taskbar: () => false, + }; + + listener._addWindow(mockWindow); + expect(listener._pendingIdles.has(42)).toBe(true); + }); + + it('should clear pending idles on unbind and call source_remove', () => { + GLib.idle_add = vi.fn((priority, cb) => 77); + + const listener = new SignalListener(mockController); + const mockWindow = { + get_window_type: () => Meta.WindowType.NORMAL, + is_skip_taskbar: () => false, + }; + + listener._addWindow(mockWindow); + expect(listener._pendingIdles.size).toBe(1); + + listener.unbind(); + expect(GLib.source_remove).toHaveBeenCalledWith(77); + expect(listener._pendingIdles.size).toBe(0); + }); + + it('should reject idle callback when activeInstance is null', () => { + let capturedCb; + GLib.idle_add = vi.fn((priority, cb) => { + capturedCb = cb; + return 88; + }); + + const listener = new SignalListener(mockController); + const mockWindow = { + get_window_type: () => Meta.WindowType.NORMAL, + is_skip_taskbar: () => false, + }; + + listener._addWindow(mockWindow); + listener.unbind(); // sets activeInstance = null + + // Simulate late callback firing + capturedCb(); + expect(mockController.tilingRequest).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/tests/window.test.js b/tests/window.test.js index fcd4c35..5a7d08f 100644 --- a/tests/window.test.js +++ b/tests/window.test.js @@ -118,4 +118,32 @@ describe('WindowWrapper', () => { const wrapper = new WindowWrapper(mockWindow, mockController); wrapper.applyGeometry({ x: 10, y: 10, width: 100, height: 100 }); // should not throw }); + + describe('_pendingLaters tracking', () => { + it('should track and remove compositor laters on destroy', () => { + const wrapper = new WindowWrapper(mockWindow, mockController); + const mockLaters = { add: vi.fn(() => 42), remove: vi.fn() }; + global.compositor.get_laters = vi.fn(() => mockLaters); + + wrapper.applyGeometry({ x: 10, y: 10, width: 100, height: 100 }); + expect(wrapper._pendingLaters.length).toBeGreaterThan(0); + expect(wrapper._pendingLaters.includes(42)).toBe(true); + + wrapper.destroy(); + expect(mockLaters.remove).toHaveBeenCalledWith(42); + expect(wrapper._pendingLaters.length).toBe(0); + }); + + it('should catch errors when removing already-fired laters in destroy', () => { + const wrapper = new WindowWrapper(mockWindow, mockController); + const mockLaters = { + add: vi.fn(() => 42), + remove: vi.fn(() => { throw new Error('Already removed'); }) + }; + global.compositor.get_laters = vi.fn(() => mockLaters); + + wrapper.applyGeometry({ x: 10, y: 10, width: 100, height: 100 }); + expect(() => wrapper.destroy()).not.toThrow(); + }); + }); });