Skip to content

Commit

Permalink
Fix Multitasking-View remove_workspace callback which has an incorrec…
Browse files Browse the repository at this point in the history 
…t algorithm to remove destroyed workspaces
  • Loading branch information
Julian committed Nov 8, 2019
1 parent b9f9ff6 commit ffbc525
Showing 1 changed file with 4 additions and 13 deletions.
17 changes: 4 additions & 13 deletions src/Widgets/MultitaskingView.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,29 +389,20 @@ namespace Gala
// FIXME is there a better way to get the removed workspace?
#if HAS_MUTTER330
unowned Meta.WorkspaceManager manager = display.get_workspace_manager ();
unowned List<Workspace> existing_workspaces = null;
for (int i = 0; i < manager.get_n_workspaces (); i++) {
existing_workspaces.append (manager.get_workspace_by_index (i));
}
#else
unowned List<Meta.Workspace> existing_workspaces = screen.get_workspaces ();
#endif

foreach (var child in workspaces.get_children ()) {
unowned WorkspaceClone clone = (WorkspaceClone) child;
#if HAS_MUTTER330
for (int i = 0; i < manager.get_n_workspaces (); i++) {
if (manager.get_workspace_by_index (i) == clone.workspace) {
workspace = clone;
break;
}
}

if (workspace != null) {
break;
}
#else
if (existing_workspaces.index (clone.workspace) < 0) {
workspace = clone;
break;
}
#endif
}

if (workspace == null)
Expand Down

1 comment on commit ffbc525

@Tireg
Copy link
Contributor

@Tireg Tireg commented on ffbc525 Nov 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue:

Can't leave an emptied workspace without crashing gala.
Ok so now I can finally start my session, and open a window without crashing gala. But hey ! When I empty a workspace and I leave it, it still crashes !
It's giving me a stacktrace like:

Bug in window manager: Workspace does not exist to index!
==2815771== 
==2815771== Process terminating with default action of signal 6 (SIGABRT): dumping core
==2815771==    at 0x5CE7F5B: raise (in /lib64/libc-2.29.so)
==2815771==    by 0x5CD1534: abort (in /lib64/libc-2.29.so)
==2815771==    by 0x57A39F4: meta_bug (in /usr/lib64/libmutter-4.so.0.0.0)
==2815771==    by 0x57B000D: meta_workspace_index (in /usr/lib64/libmutter-4.so.0.0.0)
==2815771==    by 0x261C4E: gala_multitasking_view_update_positions (in /usr/bin/gala)
==2815771==    by 0x26232F: _gala_multitasking_view_remove_workspace_meta_workspace_manager_workspace_removed (in /usr/bin/gala)
==2815771==    by 0x4A8EF1C: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6000.7)
==2815771==    by 0x4AA2887: signal_emit_unlocked_R (in /usr/lib64/libgobject-2.0.so.0.6000.7)
==2815771==    by 0x4AABAAD: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6000.7)
==2815771==    by 0x4AAC156: g_signal_emit (in /usr/lib64/libgobject-2.0.so.0.6000.7)
==2815771==    by 0x579B5CE: meta_workspace_manager_remove_workspace (in /usr/lib64/libmutter-4.so.0.0.0)
==2815771==    by 0x254671: gala_workspace_manager_remove_workspace (in /usr/bin/gala)

Description of the fix:

So now, we have another bug related to workspace removal. It seems like this one, the Multitasking-View widget is giving us a hard time, even when we don't use it.
Ok so the update_positions () function of this widget seems to be unable to find one of the workspace it internally keeps: https://github.com/elementary/gala/blob/master/src/Widgets/MultitaskingView.vala#L315.

We should check that the workspaces removal works with mutter 3.3x. Lets see the differences:

void remove_workspace (int num)
{
	WorkspaceClone? workspace = null;
	unowned Meta.WorkspaceManager manager = display.get_workspace_manager ();

	foreach (var child in workspaces.get_children ()) {
		unowned WorkspaceClone clone = (WorkspaceClone) child;
		for (int i = 0; i < manager.get_n_workspaces (); i++) {
			if (manager.get_workspace_by_index (i) == clone.workspace) {
				workspace = clone;
				break;
			}
		}

		if (workspace != null) {
			break;
		}
	}

	[...]
}

This also differs from the old behaviour:

void remove_workspace (int num)
{
	WorkspaceClone? workspace = null;
	unowned List<Meta.Workspace> existing_workspaces = screen.get_workspaces ();

	foreach (var child in workspaces.get_children ()) {
		unowned WorkspaceClone clone = (WorkspaceClone) child;
		if (existing_workspaces.index (clone.workspace) < 0) {
			workspace = clone;
			break;
		}
	}

	[...]
}

So in the original version, we loop through each of the Multitasking-View workspaces and check whether they exists (==> we can find them) within the mutter workspaces. If we find one that does not exists, then we delete it.

In the new version, we try something very strange. We try to find the first workspace of the multitasking-view which has an equivalent within mutter, and then delete it. Let's just go back to the old behaviour.

Please sign in to comment.