Merge lp:~rtandy/compiz/lp1232299 into lp:compiz/0.9.11

[Ryan Tandy]

Fix gnome-flashback session starting Unity plugins. Change the profile back after processing settings upgrades. When changing profile, discard existing GSettings wrappers pointing to the old profile. Fixes: 1232299

This is my attempt to fix bug 1232299.

The scenario: you're logging into an environment other than Unity (so the profile is probably Default), and there are settings upgrades that haven't been applied yet. Easy to reproduce: install gnome-session-flashback and start a guest session with Flashback and Compiz. Packages for trusty are in ppa:rtandy/lp1232299

The settings upgrade machinery changes the profile to process upgrades. It needs to change it back afterwards.

The GSettings profile caches a wrapper for each schema. When it retrieves one by schema name, it needs to make sure that it actually does wrap the expected path, because the path includes the profile name, and exchange it for a new one if necessary.

I didn't measure the performance hit of checking the path name on each wrapper lookup. It might be better (more efficient) to clear the list and start from scratch when the profile changes.

I also don't know for sure whether freeing the wrapper is actually safe. I didn't find any places where it looked like other code was holding references to it, but that doesn't mean they don't exist.

I experienced one crash in unityshell while testing this (didn't have unity's dbgsym installed at the time, sadly) but I can't reproduce it now so I don't know whether or not my changes caused it.

This is almost certainly too risky for trusty at this time, but I'd love to see it in an SRU eventually.

[Ryan Tandy (rtandy)]

> It might be better (more efficient) to clear the list and start from scratch when the profile changes.

Thinking about that again this morning, that almost certainly makes more sense. I've re-pushed the branch doing it that way instead. Had to move one function earlier in the file so the declaration would be visible. Apologies for the noise.

[Brandon Schaefer (brandontschaefer)]

Quickly looking at the code, it looks like this will leak some memory:
60 + char *previousProfile = strdup(cPrivate->profile);

As for the review it self, Ill have to test this out. Im not super familiar with these parts of compiz :).

Thanks for the merge proposal!

Also you need a commit message :)

[Brandon Schaefer (brandontschaefer)]

Also dont worry about pushing new changes, if you see fit. We will see its been changed and we'll re-review it :)

[lp:~rtandy/compiz/lp1232299]

3850. By Ryan Tandy on 2014-04-07

Change profile back after processing upgrades.

3851. By Ryan Tandy on 2014-04-07

Drop saved gsettings wrappers when changing profile, to force creating new ones with the correct path.

[Ryan Tandy (rtandy)]

> Quickly looking at the code, it looks like this will leak some memory:
> 60 + char *previousProfile = strdup(cPrivate->profile);

Argh. Stupid mistake. Thanks for catching that.

Built, tested, re-pushed.

[Brandon Schaefer (brandontschaefer)]

LGTM

review: Approve

=== modified file 'compizconfig/gsettings/gsettings_backend_shared/ccs_gsettings_backend.c'
--- compizconfig/gsettings/gsettings_backend_shared/ccs_gsettings_backend.c	2013-05-13 13:23:20 +0000
+++ compizconfig/gsettings/gsettings_backend_shared/ccs_gsettings_backend.c	2014-04-07 23:24:42 +0000
@@ -28,6 +28,12 @@
     CCSGNOMEValueChangeData *valueChangeData;
 };
 
+static void
+ccsGSettingsWrapperDestroyNotify (gpointer o)
+{
+    ccsGSettingsWrapperUnref ((CCSGSettingsWrapper *) o);
+}
+
 void
 ccsGSettingsSetIntegration (CCSBackend *backend, CCSIntegration *integration)
 {
@@ -177,6 +183,8 @@
     const char *ccsProfile = ccsGetProfile (context);
     char *profile = NULL;
 
+    CCSGSettingsBackendPrivate *priv = (CCSGSettingsBackendPrivate *) ccsObjectGetPrivate (backend);
+
     if (!ccsProfile)
 	profile = strdup (DEFAULTPROF);
     else
@@ -189,7 +197,11 @@
     }
 
     if (g_strcmp0 (profile, currentProfile))
+    {
 	ccsGSettingsBackendUpdateCurrentProfileName (backend, profile);
+	g_list_free_full (priv->settingsList, ccsGSettingsWrapperDestroyNotify);
+	priv->settingsList = NULL;
+    }
 
     free (profile);
 
@@ -375,12 +387,6 @@
     return priv;
 }
 
-static void
-ccsGSettingsWrapperDestroyNotify (gpointer o)
-{
-    ccsGSettingsWrapperUnref ((CCSGSettingsWrapper *) o);
-}
-
 void
 ccsGSettingsBackendDetachFromBackend (CCSBackend *backend)
 {

=== modified file 'compizconfig/libcompizconfig/src/main.c'
--- compizconfig/libcompizconfig/src/main.c	2013-05-13 13:23:20 +0000
+++ compizconfig/libcompizconfig/src/main.c	2014-04-07 23:24:42 +0000
@@ -4910,6 +4910,8 @@
 {
     int			   i = 0;
     const char	  	   *path = CCS_UPGRADE_PATH;
+    CCSContextPrivate	   *cPrivate = GET_PRIVATE (CCSContextPrivate, context);
+    char		   *previousProfile = strdup(cPrivate->profile);
 
     for (i = 0; i < nFile; ++i)
     {
@@ -4926,6 +4928,13 @@
 
 	free (nameList[i]);
     }
+
+    if (strcmp (cPrivate->profile, previousProfile))
+    {
+	    ccsSetProfile (context, previousProfile);
+    }
+
+    free (previousProfile);
 }
 
 Bool