Merge lp:~alan-griffiths/compiz-core/rework-updatePlugins into lp:compiz-core

Proposed by Alan Griffiths
Status: Merged
Merged at revision: 3075
Proposed branch: lp:~alan-griffiths/compiz-core/rework-updatePlugins
Merge into: lp:compiz-core
Diff against target: 686 lines (+439/-107) (has conflicts)
4 files modified
include/core/plugin.h (+1/-1)
src/privatescreen.h (+6/-0)
src/privatescreen/tests/test-privatescreen.cpp (+352/-13)
src/screen.cpp (+80/-93)
Text conflict in src/privatescreen/tests/test-privatescreen.cpp
To merge this branch: bzr merge lp:~alan-griffiths/compiz-core/rework-updatePlugins
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
Review via email: mp+99981@code.launchpad.net

This proposal supersedes a proposal from 2012-03-28.

Description of the change

Avoid graphics corruption and hangs on compiz start-up by ensuring that
plugins don't get initialized, de-initialized and re-initialzed during
start-up. (LP: #963093) (LP: #963633)

The multiple init/fini/init calls occured when compiz was asked to load an
invalid plugin name. This occurred most recently as plugins "bailer" and
"detection" were left in some peoples' configs while the plugins themselves
no longer exist in the current compiz release.

The source of the graphics corruption and hangs has been found to be a
problem in the composite and/or opengl plugins. One or both of them are unsafe
to init/fini multiple times without a full compiz restart. So the root cause
is not exactly known yet. However composite and opengl are not alone; many
plugins have bugs with init/fini/init sequences, so it is valuable to fix
the start-up plugin ordering as this does.

Essentially this fix works by remembering which plugins don't exist at all
and putting them on a black list. Then subsequent updates check the blacklist
and know they should never include those failed plugins in testing whether
the active plugin lists have changed.

The final part of the fix is to remove a rendundant call to updatePlugins from
EventManager::init. It is not required as main tells PluginManager when to
load plugins on startup.

IMPORTANT NOTE FOR UBUNTU PACKAGING
In downstream ubuntu branches the DEFAULT_PLUGINS list in "debian/rules" also causes multiple plugin loads on start-up and prevents this fix from working! The solution is to change DEFAULT_PLUGINS to just "ccp".

ORIGINAL DESCRIPTION:
Some test cases to cover what we need.

Combined Daniel's "blacklist" with some cleanup of the code and ignoring any non-existent plugins entirely.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Re: "Just because some plugins are "popped" to insert a new one, doesn't require them to be finalized."
This is untrue. Some plugins have loose dependencies on others and change their behaviour/features depending on what other plugins are loaded. Hence popped plugins must be fini'd and reinitialized.

Also, even if this branch works perfectly I would still be unhappy with the risk associated with such a large change in critical logic. Please don't hate me...

I'm looking at a much smaller/simpler solution. In fact I was designing it in my head as you designed this code. Will let you know if/when I think I have something that works.

In other news, the two critical bugs we're aiming to fix have had ZERO duplicates in the past two days. It sounds like the workarounds are working.

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I've tested it now. The big startup bugs are avoided, however it causes a regression: Disabling plugins in ccsm has no effect. Those plugins remain active and running even after they are removed from the active_plugins list. That's one reason why I proposed the new logging last night. It's useful :)

Arguably, it's an acceptable compromise to fix the critical issues and introduce a less severe bug. However the the size of the new code in updatePlugins still makes me a little scared. Also, I managed an equally successful hack yesterday with just a few lines changed, but similar regressions.

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Replaced by: https://code.launchpad.net/~vanvugt/compiz-core/fix-startup-plugins/+merge/99886
Which has no such regressions. And is smaller, simpler.

review: Disapprove
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Manual testing in precise is all good. The bugs are fixed.

But...
1. Please don't change formatting unnecessarily from func (x) to func(x). I agree with the latter but it's not what compiz uses.
2. You alternative between both styles inconsistently: func (x) and func(x)
3. The critical bugs this branch fixes are not mentioned anywhere and we don't have a usable commit message. But I can add those.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Also, please try to avoid renaming variables (even though you're right) this late in a stable release cycle. It increases the sizes of diffs and makes critical bug fixes harder to review.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Updated the description and linked relevant bugs.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Also, repeatedly calling: desiredPlugins[desireIndex].s ()
is inefficient and harder to read.

We should just do: const CompString &name = desiredPlugins[desireIndex].s ();
and then use "name".

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Damn. It did contain real conflicts, but I've fixed them.

3075. By Alan Griffiths

Avoid graphics corruption and hangs on compiz start-up by ensuring that
plugins don't get initialized, de-initialized and re-initialzed during
start-up. (LP: #963093) (LP: #963633)

The multiple init/fini/init calls occured when compiz was asked to load an
invalid plugin name. This occurred most recently as plugins "bailer" and
"detection" were left in some peoples' configs while the plugins themselves
no longer exist in the current compiz release.

The source of the graphics corruption and hangs has been found to be a
problem in the composite and/or opengl plugins. One or both of them are unsafe
to init/fini multiple times without a full compiz restart. So the root cause
is not exactly known yet. However composite and opengl are not alone; many
plugins have bugs with init/fini/init sequences, so it is valuable to fix
the start-up plugin ordering as this does.

Essentially this fix works by remembering which plugins don't exist at all
and putting them on a black list. Then subsequent updates check the blacklist
and know they should never include those failed plugins in testing whether
the active plugin lists have changed.

The final part of the fix is to remove a rendundant call to updatePlugins from
EventManager::init. It is not required as main tells PluginManager when to
load plugins on startup.

IMPORTANT NOTE FOR UBUNTU PACKAGING
In downstream ubuntu branches the DEFAULT_PLUGINS list in "debian/rules" also
causes multiple plugin loads on start-up and prevents this fix from working!
The solution is to change DEFAULT_PLUGINS to just "ccp".

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/core/plugin.h'
--- include/core/plugin.h 2012-02-16 05:31:28 +0000
+++ include/core/plugin.h 2012-03-29 17:16:19 +0000
@@ -207,7 +207,7 @@
207 * CompPlugin::pop()'d.207 * CompPlugin::pop()'d.
208 */208 */
209 static List & getPlugins ();209 static List & getPlugins ();
210 210
211 /**211 /**
212 * Gets a list of the names of all the known plugins, including plugins that may212 * Gets a list of the names of all the known plugins, including plugins that may
213 * have already been loaded.213 * have already been loaded.
214214
=== modified file 'src/privatescreen.h'
--- src/privatescreen.h 2012-03-24 09:49:30 +0000
+++ src/privatescreen.h 2012-03-29 17:16:19 +0000
@@ -45,6 +45,8 @@
4545
46#include "core_options.h"46#include "core_options.h"
4747
48#include <set>
49
48/**50/**
49 * A wrapping of the X display screen. This takes care of communication to the51 * A wrapping of the X display screen. This takes care of communication to the
50 * X server.52 * X server.
@@ -574,6 +576,10 @@
574 private:576 private:
575 CompOption::Value plugin;577 CompOption::Value plugin;
576 bool dirtyPluginList;578 bool dirtyPluginList;
579 typedef std::set<CompString> CompStringSet;
580 CompStringSet blacklist;
581
582 CompOption::Value::Vector mergedPluginList();
577};583};
578584
579class GrabList585class GrabList
580586
=== modified file 'src/privatescreen/tests/test-privatescreen.cpp'
--- src/privatescreen/tests/test-privatescreen.cpp 2012-03-29 13:33:39 +0000
+++ src/privatescreen/tests/test-privatescreen.cpp 2012-03-29 17:16:19 +0000
@@ -175,6 +175,29 @@
175 MOCK_CONST_METHOD0(createFailed, bool ());175 MOCK_CONST_METHOD0(createFailed, bool ());
176};176};
177177
178class StubActivePluginsOption
179{
180public:
181 StubActivePluginsOption(CoreOptions& co) : co(co)
182 {
183 CompOption::Vector& mOptions = co.getOptions ();
184 CompOption::Value::Vector list;
185 CompOption::Value value;
186
187 // active_plugins
188 mOptions[CoreOptions::ActivePlugins].setName ("active_plugins", CompOption::TypeList);
189 list.clear ();
190 value.set(CompString ("core"));
191 list.push_back (value);
192 }
193
194 bool setActivePlugins(const char*, const char* key, CompOption::Value & value)
195 {
196 return co.setOption(key, value);
197 }
198private:
199 CoreOptions& co;
200};
178} // (anon) namespace201} // (anon) namespace
179202
180namespace {203namespace {
@@ -254,6 +277,17 @@
254 }277 }
255 return true;278 return true;
256 }279 }
280
281 CompStringList mockListPlugins (const char *path)
282 {
283 CompStringList list;
284 list.push_back("one");
285 list.push_back("two");
286 list.push_back("three");
287 list.push_back("four");
288
289 return list;
290 }
257};291};
258292
259293
@@ -349,19 +383,320 @@
349 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));383 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
350 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("two"))).384 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("two"))).
351 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));385 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
352 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("three"))).386<<<<<<< TREE
353 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));387 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("three"))).
354388 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
355 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(1); // Once for "three" which doesn't load389
356390 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(1); // Once for "three" which doesn't load
357 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).391
358 WillOnce(Return(false));392 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
359393 WillOnce(Return(false));
360 ps.updatePlugins();394
361395 ps.updatePlugins();
362 // TODO Some cleanup that probably ought to be automatic.396
363 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(2);397 // TODO Some cleanup that probably ought to be automatic.
364 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(2);398 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(2);
399 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(2);
400=======
401 EXPECT_CALL(mockfs.mockVtableTwo, init()).WillOnce(Return(true));
402
403 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("three"))).
404 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
405 EXPECT_CALL(mockfs.mockVtableThree, init()).WillOnce(Return(false));
406
407 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(1); // Once for "three" which doesn't load
408
409 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
410 WillOnce(Return(false));
411
412 EXPECT_CALL(mockfs, ListPlugins(_)).
413 WillRepeatedly(Invoke(&mockfs, &MockPluginFilesystem::mockListPlugins));
414
415 ps.updatePlugins();
416
417 Mock::VerifyAndClearExpectations(&mockfs);
418 Mock::VerifyAndClearExpectations(&mockfs.mockVtableOne);
419 Mock::VerifyAndClearExpectations(&mockfs.mockVtableTwo);
420 Mock::VerifyAndClearExpectations(&mockfs.mockVtableThree);
421 Mock::VerifyAndClearExpectations(&mockfs.mockVtableFour);
422
423 // TODO Some cleanup that probably ought to be automatic.
424 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(2);
425 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(2);
426 EXPECT_CALL(mockfs.mockVtableOne, finiScreen(Ne((void*)0))).Times(1);
427 EXPECT_CALL(mockfs.mockVtableOne, fini()).Times(1);
428 EXPECT_CALL(mockfs.mockVtableTwo, finiScreen(Ne((void*)0))).Times(1);
429 EXPECT_CALL(mockfs.mockVtableTwo, fini()).Times(1);
430
431 for (CompPlugin* p; (p = CompPlugin::pop ()) != 0; CompPlugin::unload (p));
432}
433
434TEST(privatescreen_PluginManagerTest, updating_when_failing_to_load_plugin_in_middle_of_list)
435{
436 using namespace testing;
437
438 MockCompScreen comp_screen;
439
440 cps::PluginManager ps(&comp_screen);
441 StubActivePluginsOption sapo(ps);
442
443 CompOption::Value::Vector values;
444 values.push_back ("core");
445 ps.setPlugins (values);
446 ps.setDirtyPluginList ();
447
448 std::list <CompString> plugins;
449 plugins.push_back ("one");
450 plugins.push_back ("three");
451 plugins.push_back ("four");
452 initialPlugins = plugins;
453
454 MockPluginFilesystem mockfs;
455
456 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("one"))).
457 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
458 EXPECT_CALL(mockfs.mockVtableOne, init()).WillOnce(Return(true));
459
460 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("three"))).
461 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
462 EXPECT_CALL(mockfs.mockVtableThree, init()).WillOnce(Return(false));
463
464 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("four"))).
465 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
466 EXPECT_CALL(mockfs.mockVtableFour, init()).Times(1).WillRepeatedly(Return(true));
467
468 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(1); // Once for "three" which doesn't load
469
470 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
471 WillOnce(Return(true));
472
473 EXPECT_CALL(mockfs, ListPlugins(_)).
474 WillRepeatedly(Invoke(&mockfs, &MockPluginFilesystem::mockListPlugins));
475
476 ps.updatePlugins();
477
478 Mock::VerifyAndClearExpectations(&mockfs);
479 Mock::VerifyAndClearExpectations(&mockfs.mockVtableOne);
480 Mock::VerifyAndClearExpectations(&mockfs.mockVtableTwo);
481 Mock::VerifyAndClearExpectations(&mockfs.mockVtableThree);
482 Mock::VerifyAndClearExpectations(&mockfs.mockVtableFour);
483
484 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
485 WillOnce(Invoke(&sapo, &StubActivePluginsOption::setActivePlugins));
486
487 EXPECT_CALL(mockfs, ListPlugins(_)).
488 WillRepeatedly(Invoke(&mockfs, &MockPluginFilesystem::mockListPlugins));
489
490 ps.updatePlugins();
491
492 Mock::VerifyAndClearExpectations(&mockfs);
493 Mock::VerifyAndClearExpectations(&mockfs.mockVtableOne);
494 Mock::VerifyAndClearExpectations(&mockfs.mockVtableTwo);
495 Mock::VerifyAndClearExpectations(&mockfs.mockVtableThree);
496 Mock::VerifyAndClearExpectations(&mockfs.mockVtableFour);
497
498 // TODO Some cleanup that probably ought to be automatic.
499 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(2);
500 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(2);
501 EXPECT_CALL(mockfs.mockVtableOne, finiScreen(Ne((void*)0))).Times(1);
502 EXPECT_CALL(mockfs.mockVtableOne, fini()).Times(1);
503 EXPECT_CALL(mockfs.mockVtableFour, finiScreen(Ne((void*)0))).Times(1);
504 EXPECT_CALL(mockfs.mockVtableFour, fini()).Times(1);
505 for (CompPlugin* p; (p = CompPlugin::pop ()) != 0; CompPlugin::unload (p));
506}
507
508TEST(privatescreen_PluginManagerTest, calling_updatePlugins_with_fewer_plugins)
509{
510 using namespace testing;
511
512 MockCompScreen comp_screen;
513
514 cps::PluginManager ps(&comp_screen);
515
516 StubActivePluginsOption sapo(ps);
517
518 // Stuff that has to be done before calling updatePlugins()
519 initialPlugins = std::list <CompString>();
520 CompOption::Value::Vector values;
521 values.push_back ("core");
522 ps.setPlugins (values);
523 ps.setDirtyPluginList ();
524
525 {
526 CompOption::Value::Vector plugins;
527 plugins.push_back ("one");
528 plugins.push_back ("two");
529 plugins.push_back ("three");
530 CompOption::Value v(plugins);
531 sapo.setActivePlugins("core", "active_plugins", v);
532 }
533
534 MockPluginFilesystem mockfs;
535
536 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("one"))).
537 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
538 EXPECT_CALL(mockfs.mockVtableOne, init()).WillOnce(Return(true));
539
540 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("two"))).
541 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
542 EXPECT_CALL(mockfs.mockVtableTwo, init()).WillOnce(Return(true));
543
544 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("three"))).
545 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
546 EXPECT_CALL(mockfs.mockVtableThree, init()).WillOnce(Return(true));
547
548 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
549 WillOnce(Invoke(&sapo, &StubActivePluginsOption::setActivePlugins));
550
551 EXPECT_CALL(mockfs, ListPlugins(_)).
552 WillRepeatedly(Invoke(&mockfs, &MockPluginFilesystem::mockListPlugins));
553
554 ps.updatePlugins();
555
556 Mock::VerifyAndClearExpectations(&mockfs);
557 Mock::VerifyAndClearExpectations(&mockfs.mockVtableOne);
558 Mock::VerifyAndClearExpectations(&mockfs.mockVtableTwo);
559 Mock::VerifyAndClearExpectations(&mockfs.mockVtableThree);
560 Mock::VerifyAndClearExpectations(&mockfs.mockVtableFour);
561
562 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(2);
563 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(1);
564 EXPECT_CALL(mockfs.mockVtableTwo, finiScreen(Ne((void*)0))).Times(1);
565 EXPECT_CALL(mockfs.mockVtableTwo, fini()).Times(1);
566 EXPECT_CALL(mockfs.mockVtableThree, fini()).Times(1);
567 EXPECT_CALL(mockfs.mockVtableThree, finiScreen(Ne((void*)0))).Times(1);
568 EXPECT_CALL(mockfs.mockVtableThree, init()).WillOnce(Return(true));
569 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
570 WillOnce(Invoke(&sapo, &StubActivePluginsOption::setActivePlugins));
571
572 {
573 CompOption::Value::Vector plugins;
574 plugins.push_back ("one");
575 plugins.push_back ("three");
576 CompOption::Value v(plugins);
577 sapo.setActivePlugins("core", "active_plugins", v);
578 }
579
580 EXPECT_CALL(mockfs, ListPlugins(_)).
581 WillRepeatedly(Invoke(&mockfs, &MockPluginFilesystem::mockListPlugins));
582
583 ps.updatePlugins();
584
585 Mock::VerifyAndClearExpectations(&mockfs);
586 Mock::VerifyAndClearExpectations(&mockfs.mockVtableOne);
587 Mock::VerifyAndClearExpectations(&mockfs.mockVtableTwo);
588 Mock::VerifyAndClearExpectations(&mockfs.mockVtableThree);
589 Mock::VerifyAndClearExpectations(&mockfs.mockVtableFour);
590
591 // TODO Some cleanup that probably ought to be automatic.
592 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(2);
593 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(2);
594 EXPECT_CALL(mockfs.mockVtableOne, finiScreen(Ne((void*)0))).Times(1);
595 EXPECT_CALL(mockfs.mockVtableOne, fini()).Times(1);
596 EXPECT_CALL(mockfs.mockVtableThree, finiScreen(Ne((void*)0))).Times(1);
597 EXPECT_CALL(mockfs.mockVtableThree, fini()).Times(1);
598
599 for (CompPlugin* p; (p = CompPlugin::pop ()) != 0; CompPlugin::unload (p));
600}
601
602TEST(privatescreen_PluginManagerTest, calling_updatePlugins_with_additional_plugins)
603{
604 using namespace testing;
605
606 MockCompScreen comp_screen;
607
608 cps::PluginManager ps(&comp_screen);
609
610 StubActivePluginsOption sapo(ps);
611
612 // Stuff that has to be done before calling updatePlugins()
613 initialPlugins = std::list <CompString>();
614 CompOption::Value::Vector values;
615 values.push_back ("core");
616 ps.setPlugins (values);
617 ps.setDirtyPluginList ();
618
619 {
620 CompOption::Value::Vector plugins;
621 plugins.push_back ("one");
622 plugins.push_back ("two");
623 plugins.push_back ("four");
624 CompOption::Value v(plugins);
625 sapo.setActivePlugins("core", "active_plugins", v);
626 }
627
628 MockPluginFilesystem mockfs;
629
630 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("one"))).
631 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
632 EXPECT_CALL(mockfs.mockVtableOne, init()).WillOnce(Return(true));
633
634 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("two"))).
635 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
636 EXPECT_CALL(mockfs.mockVtableTwo, init()).WillOnce(Return(true));
637
638 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("four"))).
639 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
640 EXPECT_CALL(mockfs.mockVtableFour, init()).WillOnce(Return(true));
641
642 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
643 WillOnce(Invoke(&sapo, &StubActivePluginsOption::setActivePlugins));
644
645 EXPECT_CALL(mockfs, ListPlugins(_)).
646 WillRepeatedly(Invoke(&mockfs, &MockPluginFilesystem::mockListPlugins));
647
648 ps.updatePlugins();
649
650 Mock::VerifyAndClearExpectations(&mockfs);
651 Mock::VerifyAndClearExpectations(&mockfs.mockVtableOne);
652 Mock::VerifyAndClearExpectations(&mockfs.mockVtableTwo);
653 Mock::VerifyAndClearExpectations(&mockfs.mockVtableThree);
654 Mock::VerifyAndClearExpectations(&mockfs.mockVtableFour);
655
656 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(1);
657 EXPECT_CALL(mockfs.mockVtableFour, fini()).Times(1);
658 EXPECT_CALL(mockfs.mockVtableFour, finiScreen(Ne((void*)0))).Times(1);
659 EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("three"))).
660 WillOnce(Invoke(&mockfs, &MockPluginFilesystem::DummyLoader));
661 EXPECT_CALL(mockfs.mockVtableThree, init()).WillOnce(Return(true));
662 EXPECT_CALL(mockfs.mockVtableFour, init()).WillOnce(Return(true));
663 EXPECT_CALL(comp_screen, _setOptionForPlugin(StrEq("core"), StrEq("active_plugins"), _)).
664 WillOnce(Invoke(&sapo, &StubActivePluginsOption::setActivePlugins));
665
666 {
667 CompOption::Value::Vector plugins;
668 plugins.push_back ("one");
669 plugins.push_back ("two");
670 plugins.push_back ("three");
671 plugins.push_back ("four");
672 CompOption::Value v(plugins);
673 sapo.setActivePlugins("core", "active_plugins", v);
674 }
675
676 EXPECT_CALL(mockfs, ListPlugins(_)).
677 WillRepeatedly(Invoke(&mockfs, &MockPluginFilesystem::mockListPlugins));
678
679 ps.updatePlugins();
680
681 Mock::VerifyAndClearExpectations(&mockfs);
682 Mock::VerifyAndClearExpectations(&mockfs.mockVtableOne);
683 Mock::VerifyAndClearExpectations(&mockfs.mockVtableTwo);
684 Mock::VerifyAndClearExpectations(&mockfs.mockVtableThree);
685 Mock::VerifyAndClearExpectations(&mockfs.mockVtableFour);
686
687 // TODO Some cleanup that probably ought to be automatic.
688 EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(4);
689 EXPECT_CALL(comp_screen, _finiPluginForScreen(Ne((void*)0))).Times(4);
690 EXPECT_CALL(mockfs.mockVtableFour, finiScreen(Ne((void*)0))).Times(1);
691 EXPECT_CALL(mockfs.mockVtableFour, fini()).Times(1);
692 EXPECT_CALL(mockfs.mockVtableThree, finiScreen(Ne((void*)0))).Times(1);
693 EXPECT_CALL(mockfs.mockVtableThree, fini()).Times(1);
694 EXPECT_CALL(mockfs.mockVtableTwo, finiScreen(Ne((void*)0))).Times(1);
695 EXPECT_CALL(mockfs.mockVtableTwo, fini()).Times(1);
696 EXPECT_CALL(mockfs.mockVtableOne, finiScreen(Ne((void*)0))).Times(1);
697 EXPECT_CALL(mockfs.mockVtableOne, fini()).Times(1);
698
699>>>>>>> MERGE-SOURCE
365 for (CompPlugin* p; (p = CompPlugin::pop ()) != 0; CompPlugin::unload (p));700 for (CompPlugin* p; (p = CompPlugin::pop ()) != 0; CompPlugin::unload (p));
366}701}
367702
@@ -398,5 +733,9 @@
398 comp_screen.priv.reset(new PrivateScreen(&comp_screen));733 comp_screen.priv.reset(new PrivateScreen(&comp_screen));
399 cps::EventManager& em(*comp_screen.priv.get());734 cps::EventManager& em(*comp_screen.priv.get());
400735
736<<<<<<< TREE
737=======
738 em.setPlugins (values);
739>>>>>>> MERGE-SOURCE
401 em.init(0);740 em.init(0);
402}741}
403742
=== modified file 'src/screen.cpp'
--- src/screen.cpp 2012-03-24 09:49:30 +0000
+++ src/screen.cpp 2012-03-29 17:16:19 +0000
@@ -893,83 +893,45 @@
893 }893 }
894}894}
895895
896void896CompOption::Value::Vector
897cps::PluginManager::updatePlugins ()897cps::PluginManager::mergedPluginList ()
898{898{
899 unsigned int pListCount = 1;899 std::list<CompString> availablePlugins(CompPlugin::availablePlugins ());
900900
901 possibleTap = NULL;901 CompOption::Value::Vector result;
902 dirtyPluginList = false;902
903903 CompOption::Value::Vector const& extraPluginsRequested = optionGetActivePlugins();
904 CompOption::Value::Vector &list = optionGetActivePlugins ();
905
906 /* Determine the number of plugins, which is core +
907 * initial plugins + plugins in option list in addition
908 * to initial plugins */
909 foreach (CompString &pn, initialPlugins)
910 {
911 if (pn != "core")
912 pListCount++;
913 }
914
915 CompString lpName;
916
917 foreach (CompOption::Value &lp, list)
918 {
919 bool skip = false;
920
921 lpName = lp.s();
922
923 if (lpName == "core")
924 continue;
925
926 foreach (CompString &p, initialPlugins)
927 {
928 if (p == lpName)
929 {
930 skip = true;
931 break;
932 }
933 }
934
935 /* plugin not in initial list */
936 if (!skip)
937 pListCount++;
938 }
939
940 /* pListCount is now the number of plugisn contained in both the
941 * initial and new plugins list */
942 CompOption::Value::Vector pList(pListCount);
943
944 if (pList.empty ())
945 {
946 screen->setOptionForPlugin ("core", "active_plugins", plugin);
947 return;
948 }
949904
950 /* Must have core as first plugin */905 /* Must have core as first plugin */
951 pList.at (0) = "core";906 result.push_back("core");
952 unsigned int j = 1;
953907
954 /* Add initial plugins */908 /* Add initial plugins */
955 foreach (CompString &p, initialPlugins)909 foreach(CompString & p, initialPlugins)
956 {910 {
957 if (p == "core")911 if (p == "core")
958 continue;912 continue;
959 pList.at (j).set (p);913
960 j++;914 if (blacklist.find (p) != blacklist.end ())
915 continue;
916
917 if (availablePlugins.end() != std::find(availablePlugins.begin(), availablePlugins.end(), p))
918 result.push_back(p);
961 }919 }
962920
963 /* Add plugins not in the initial list */921 /* Add plugins not in the initial list */
964 foreach (CompOption::Value &opt, list)922 foreach(CompOption::Value const& opt, extraPluginsRequested)
965 {923 {
966 std::list <CompString>::iterator it = initialPlugins.begin ();924 if (opt.s() == "core")
967 bool skip = false;925 continue;
968 926
969 if (opt.s () == "core")927 if (blacklist.find (opt.s()) != blacklist.end ())
970 continue;928 continue;
971929
972 for (; it != initialPlugins.end (); it++)930 typedef std::list<CompString>::iterator iterator;
931 bool skip = false;
932
933 for (iterator it = initialPlugins.begin(); it != initialPlugins.end();
934 it++)
973 {935 {
974 if ((*it) == opt.s())936 if ((*it) == opt.s())
975 {937 {
@@ -980,50 +942,71 @@
980942
981 if (!skip)943 if (!skip)
982 {944 {
983 pList.at (j++).set (opt.s ());945 if (availablePlugins.end() != std::find(availablePlugins.begin(), availablePlugins.end(), opt.s()))
946 result.push_back(opt.s());
984 }947 }
985 }948 }
986949 return result;
987 assert (j == pList.size ());950}
988951
989 /* j is initialized to 1 to make sure we never pop the core plugin */952
990 unsigned int i;953void
991 for (i = j = 1; j < plugin.list ().size () && i < pList.size (); i++, j++)954cps::PluginManager::updatePlugins ()
955{
956 possibleTap = NULL;
957 dirtyPluginList = false;
958
959 CompOption::Value::Vector const desiredPlugins(mergedPluginList());
960
961 unsigned int pluginIndex;
962 for (pluginIndex = 1;
963 pluginIndex < plugin.list ().size () && pluginIndex < desiredPlugins.size ();
964 pluginIndex++)
992 {965 {
993 if (plugin.list ().at (j).s () != pList.at (i).s ())966 if (plugin.list ().at (pluginIndex).s () != desiredPlugins.at (pluginIndex).s ())
994 break;967 break;
995 }968 }
996969
997 CompPlugin::List pop;970 unsigned int desireIndex = pluginIndex;
998971
999 if (unsigned int const nPop = plugin.list ().size () - j)972 // We have pluginIndex pointing at first difference (or end).
973 // Now pop plugins off stack to this point, but keep track that they are loaded
974 CompPlugin::List alreadyLoaded;
975 if (const unsigned int nPop = plugin.list().size() - pluginIndex)
1000 {976 {
1001 for (j = 0; j < nPop; j++)977 for (pluginIndex = 0; pluginIndex < nPop; pluginIndex++)
1002 {978 {
1003 pop.push_back (CompPlugin::pop ());979 alreadyLoaded.push_back(CompPlugin::pop());
1004 plugin.list ().pop_back ();980 plugin.list().pop_back();
1005 }981 }
1006 }982 }
1007983
1008 for (; i < pList.size (); i++)984 // Now work forward through requested plugins
985 for (; desireIndex < desiredPlugins.size(); desireIndex++)
1009 {986 {
1010 CompPlugin *p = NULL;987 CompPlugin *p = NULL;
1011 bool failedPush = false;988 bool failedPush = false;
1012989
1013 foreach (CompPlugin *pp, pop)990 // If already loaded, just try to push it...
991 foreach(CompPlugin * pp, alreadyLoaded)
1014 {992 {
1015 if (pList[i]. s () == pp->vTable->name ())993 if (desiredPlugins[desireIndex].s() == pp->vTable->name())
1016 {994 {
1017 if (CompPlugin::push (pp))995 if (CompPlugin::push (pp))
1018 {996 {
1019 p = pp;997 p = pp;
1020 pop.erase (std::find (pop.begin (), pop.end (), pp));998 alreadyLoaded.erase(
999 std::find(alreadyLoaded.begin(),
1000 alreadyLoaded.end(), pp));
1021 break;1001 break;
1022 }1002 }
1023 else1003 else
1024 {1004 {
1025 pop.erase (std::find (pop.begin (), pop.end (), pp));1005 alreadyLoaded.erase(
1026 CompPlugin::unload (pp);1006 std::find(alreadyLoaded.begin(),
1007 alreadyLoaded.end(), pp));
1008 blacklist.insert (desiredPlugins[desireIndex].s ());
1009 CompPlugin::unload(pp);
1027 p = NULL;1010 p = NULL;
1028 failedPush = true;1011 failedPush = true;
1029 break;1012 break;
@@ -1031,25 +1014,32 @@
1031 }1014 }
1032 }1015 }
10331016
1017 // ...otherwise, try to load and push
1034 if (p == 0 && !failedPush)1018 if (p == 0 && !failedPush)
1035 {1019 {
1036 p = CompPlugin::load (pList[i].s ().c_str ());1020 p = CompPlugin::load(desiredPlugins[desireIndex].s ().c_str ());
1037 1021
1038 if (p)1022 if (p)
1039 {1023 {
1040 if (!CompPlugin::push (p))1024 if (!CompPlugin::push(p))
1041 {1025 {
1042 CompPlugin::unload (p);1026 blacklist.insert (desiredPlugins[desireIndex].s ());
1027 CompPlugin::unload(p);
1043 p = 0;1028 p = 0;
1044 }1029 }
1045 }1030 }
1031 else
1032 {
1033 blacklist.insert (desiredPlugins[desireIndex].s ());
1034 }
1046 }1035 }
10471036
1048 if (p)1037 if (p)
1049 plugin.list ().push_back (p->vTable->name ());1038 plugin.list().push_back(p->vTable->name());
1050 }1039 }
10511040
1052 foreach (CompPlugin *pp, pop)1041 // Any plugins that are loaded, but were not re-initialized can be unloaded.
1042 foreach(CompPlugin * pp, alreadyLoaded)
1053 CompPlugin::unload (pp);1043 CompPlugin::unload (pp);
10541044
1055 if (!dirtyPluginList)1045 if (!dirtyPluginList)
@@ -4626,9 +4616,6 @@
46264616
4627 optionSetToggleWindowShadedKeyInitiate (CompScreenImpl::shadeWin);4617 optionSetToggleWindowShadedKeyInitiate (CompScreenImpl::shadeWin);
46284618
4629 if (isDirtyPluginList ())
4630 updatePlugins ();
4631
4632 return true;4619 return true;
4633}4620}
46344621

Subscribers

People subscribed via source and target branches