Merge lp:~vanvugt/compiz/fix-1002715-1002721 into lp:compiz/0.9.8

[Daniel van Vugt]

Don't drop plugins from the list to try and load before you've even tried to
load them. Doing so makes missing plugins silently ignored instead of an
error message (LP: #1002715). It also means valid plugins in more unusual, but
real locations in LD_LIBRARY_PATH will never get loaded (LP: #1002721).

[Sam Spilsbury (smspillaz)]

review: Approve

=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2012-05-15 15:41:59 +0000
+++ src/privatescreen.h	2012-05-23 05:22:20 +0000
@@ -267,13 +267,13 @@
 	bool isDirtyPluginList () const { return dirtyPluginList; }
 	void setDirtyPluginList () { dirtyPluginList = true; }
 
+	CompOption::Value::Vector mergedPluginList(CompOption::Value::Vector const& extraPluginsRequested) const;
+
     private:
 	CompOption::Value plugin;
 	bool	          dirtyPluginList;
 	typedef std::set<CompString> CompStringSet;
 	CompStringSet blacklist;
-
-	CompOption::Value::Vector mergedPluginList(CompOption::Value::Vector const& extraPluginsRequested);
 };
 
 class GrabList

=== modified file 'src/privatescreen/tests/test-privatescreen.cpp'
--- src/privatescreen/tests/test-privatescreen.cpp	2012-05-10 11:30:52 +0000
+++ src/privatescreen/tests/test-privatescreen.cpp	2012-05-23 05:22:20 +0000
@@ -619,6 +619,37 @@
     for (CompPlugin* p; (p = CompPlugin::pop ()) != 0; CompPlugin::unload (p));
 }
 
+// Verify plugin ordering, and verify that plugins not in availablePlugins
+// don't get dropped. Because availablePlugins is NOT a definitive list
+// of what the dynamic loader might be able to find in its path.
+TEST(privatescreen_PluginManagerTest, verify_plugin_ordering)
+{
+    using namespace testing;
+
+    cps::PluginManager ps;
+
+    initialPlugins.clear();
+    initialPlugins.push_back("alice");
+    initialPlugins.push_back("bob");
+    initialPlugins.push_back("charlie");
+
+    CompOption::Value::Vector extra;
+    extra.push_back("charlie");
+    extra.push_back("david");
+    extra.push_back("alice");
+    extra.push_back("eric");
+    
+    CompOption::Value::Vector merged = ps.mergedPluginList(extra);
+
+    ASSERT_EQ(merged.size(), 6);
+    ASSERT_EQ(merged[0].s(), "core");
+    ASSERT_EQ(merged[1].s(), "alice");
+    ASSERT_EQ(merged[2].s(), "bob");
+    ASSERT_EQ(merged[3].s(), "charlie");
+    ASSERT_EQ(merged[4].s(), "david");
+    ASSERT_EQ(merged[5].s(), "eric");
+}
+
 TEST(privatescreen_PluginManagerTest, calling_updatePlugins_with_additional_plugins)
 {
     using namespace testing;

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-05-15 15:41:59 +0000
+++ src/screen.cpp	2012-05-23 05:22:20 +0000
@@ -985,10 +985,8 @@
 
 
 CompOption::Value::Vector
-cps::PluginManager::mergedPluginList (CompOption::Value::Vector const& extraPluginsRequested)
+cps::PluginManager::mergedPluginList (CompOption::Value::Vector const& extraPluginsRequested) const
 {
-    std::list<CompString> availablePlugins(CompPlugin::availablePlugins ());
-
     CompOption::Value::Vector result;
 
     /* Must have core as first plugin */
@@ -1003,8 +1001,7 @@
 	if (blacklist.find (p) != blacklist.end ())
 	    continue;
 
-	if (availablePlugins.end() != std::find(availablePlugins.begin(), availablePlugins.end(), p))
-	    result.push_back(p);
+	result.push_back(p);
     }
 
     /* Add plugins not in the initial list */
@@ -1031,8 +1028,7 @@
 
 	if (!skip)
 	{
-	    if (availablePlugins.end() != std::find(availablePlugins.begin(), availablePlugins.end(), opt.s()))
-		result.push_back(opt.s());
+	    result.push_back(opt.s());
 	}
     }
     return result;