KDE/kdebase/workspace/libs/taskmanager

Aaron J. Seigo aseigo at kde.org
Sun Jun 21 21:04:40 CEST 2009


SVN commit 984938 by aseigo:

don't delete the grouping strategy immediatley when changing it. this is due to the following possible chain of events:

* window comes or goes
* the grouping strategy is asked to update the groups based on this event, and tells the group manager about a change in events
* the group manager notifies the outside world about it
* the tasks widget's size changes becuase of this and that affects the optimal number of entries to show, which it relays to the group manager
* the group manager realizes it now has enough room for all the tasks, and switches the grouping strategy ... BY DELETING THE GROUPING STRATEGY!
* execution then eventually returns to wherever we were at step 2 ... BOOM (with all sorts of oddness in the backtraces :)

this also explains why it was intermitent (change of grouping collection which caused a size change in the tasks widget which altered the optimal number of buttons) and only for some people ("only group when full" and with a variable size tasks widget, e.g. on an expanding panel)

this is a pretty big set of changes, and i've gone over them carefully and tested them as thoroughly as i can, but additional feedback from people using SVN would be great as this is a big set of changes this close to release

CCMAIL:plasma-devel at kde.org
BUGS:193042,188378


 M  +37 -11    abstractgroupingstrategy.cpp  
 M  +7 -0      abstractgroupingstrategy.h  
 M  +23 -20    groupmanager.cpp  
 M  +6 -4      strategies/kustodiangroupingstrategy.cpp  
 M  +40 -31    strategies/manualgroupingstrategy.cpp  
 M  +14 -9     strategies/programgroupingstrategy.cpp  


--- trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupingstrategy.cpp #984937:984938
@@ -55,6 +55,17 @@
 
 AbstractGroupingStrategy::~AbstractGroupingStrategy()
 {
+    destroy();
+    qDeleteAll(d->createdGroups);
+    delete d;
+}
+
+void AbstractGroupingStrategy::destroy()
+{
+    if (!d->groupManager) {
+        return;
+    }
+
     foreach (TaskGroup *group, d->createdGroups) { //cleanup all created groups
         disconnect(group, 0, this, 0);
 
@@ -73,8 +84,8 @@
         emit groupRemoved(group);
     }
 
-    qDeleteAll(d->createdGroups);
-    delete d;
+    d->groupManager = 0;
+    deleteLater();
 }
 
 GroupManager::TaskGroupingStrategy AbstractGroupingStrategy::type() const
@@ -99,13 +110,22 @@
     return QList<QAction*>();
 }
 
+GroupPtr AbstractGroupingStrategy::rootGroup() const
+{
+    if (d->groupManager) {
+        return d->groupManager->rootGroup();
+    }
+
+    return 0;
+}
+
 TaskGroup* AbstractGroupingStrategy::createGroup(ItemList items)
 {
     GroupPtr oldGroup;
     if (!items.isEmpty() && items.first()->isGrouped()) {
         oldGroup = items.first()->parentGroup();
     } else {
-        oldGroup = d->groupManager->rootGroup();
+        oldGroup = rootGroup();
     }
 
     TaskGroup *newGroup = new TaskGroup(d->groupManager);
@@ -115,7 +135,10 @@
         newGroup->add(item);
     }
 
-    oldGroup->add(newGroup);
+    if (oldGroup) {
+        oldGroup->add(newGroup);
+    }
+
     return newGroup;
 }
 
@@ -130,17 +153,20 @@
 
     TaskGroup *parentGroup = group->parentGroup();
     if (!parentGroup) {
-        parentGroup = d->groupManager->rootGroup();
+        parentGroup = rootGroup();
     }
 
-    int index = parentGroup->members().indexOf(group);
-    foreach (const AbstractItemPtr& item, group->members()) {
-        parentGroup->add(item);
-        //move item to the location where its group was
-        d->groupManager->manualSortingRequest(item, index); //move items to position of group
+    if (parentGroup && d->groupManager) {
+        int index = parentGroup->members().indexOf(group);
+        foreach (const AbstractItemPtr& item, group->members()) {
+            parentGroup->add(item);
+            //move item to the location where its group was
+            d->groupManager->manualSortingRequest(item, index); //move items to position of group
+        }
+
+        parentGroup->remove(group);
     }
 
-    parentGroup->remove(group);
     emit groupRemoved(group);
     group->deleteLater();
 }
--- trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupingstrategy.h #984937:984938
@@ -47,6 +47,8 @@
     AbstractGroupingStrategy(GroupManager *groupManager);
     virtual ~AbstractGroupingStrategy();
 
+    void destroy();
+
      /** Handles a new item */
     virtual void handleItem(AbstractItemPtr) = 0;
 
@@ -61,6 +63,11 @@
     */
     virtual QList<QAction*> strategyActions(QObject *parent, AbstractGroupableItem *item);
 
+    /**
+     * Returns the root group to use in grouping
+     */
+    GroupPtr rootGroup() const;
+
     enum EditableGroupProperties
     {
         None = 0,
--- trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.cpp #984937:984938
@@ -61,7 +61,7 @@
           showOnlyCurrentScreen(false),
           showOnlyMinimized(false),
           onlyGroupWhenFull(false),
-          changingGroupingStragegy(false)
+          changingGroupingStrategy(false)
     {
     }
 
@@ -101,7 +101,7 @@
     bool showOnlyCurrentScreen : 1;
     bool showOnlyMinimized : 1;
     bool onlyGroupWhenFull : 1;
-    bool changingGroupingStragegy : 1;
+    bool changingGroupingStrategy : 1;
     QUuid configToken;
 };
 
@@ -502,22 +502,22 @@
     return d->onlyGroupWhenFull;
 }
 
-void GroupManager::setOnlyGroupWhenFull(bool state)
+void GroupManager::setOnlyGroupWhenFull(bool onlyGroupWhenFull)
 {
-    //kDebug() << state;
-    if (d->onlyGroupWhenFull == state) {
+    //kDebug() << onlyGroupWhenFull;
+    if (d->onlyGroupWhenFull == onlyGroupWhenFull) {
         return;
     }
 
-    d->onlyGroupWhenFull = state;
+    d->onlyGroupWhenFull = onlyGroupWhenFull;
 
-    if (state) {
+    disconnect(d->rootGroup, SIGNAL(itemAdded(AbstractItemPtr)), this, SLOT(checkIfFull()));
+    disconnect(d->rootGroup, SIGNAL(itemRemoved(AbstractItemPtr)), this, SLOT(checkIfFull()));
+
+    if (onlyGroupWhenFull) {
         connect(d->rootGroup, SIGNAL(itemAdded(AbstractItemPtr)), this, SLOT(checkIfFull()));
         connect(d->rootGroup, SIGNAL(itemRemoved(AbstractItemPtr)), this, SLOT(checkIfFull()));
         d->checkIfFull();
-    } else {
-        disconnect(d->rootGroup, SIGNAL(itemAdded(AbstractItemPtr)), this, SLOT(checkIfFull()));
-        disconnect(d->rootGroup, SIGNAL(itemRemoved(AbstractItemPtr)), this, SLOT(checkIfFull()));
     }
 }
 
@@ -536,13 +536,14 @@
     if (!onlyGroupWhenFull || groupingStrategy != GroupManager::ProgramGrouping) {
         return;
     }
+
     if (itemList.size() >= groupIsFullLimit) {
         if (!abstractGroupingStrategy) {
             geometryTasks.clear();
             q->setGroupingStrategy(GroupManager::ProgramGrouping);
         }
     } else if (abstractGroupingStrategy) {
-         geometryTasks.clear();
+        geometryTasks.clear();
         q->setGroupingStrategy(GroupManager::NoGrouping);
         //let the visualization thing we still use the programGrouping
         groupingStrategy = GroupManager::ProgramGrouping;
@@ -643,12 +644,12 @@
 
 void GroupManager::setGroupingStrategy(TaskGroupingStrategy strategy)
 {
-    if (d->changingGroupingStragegy ||
+    if (d->changingGroupingStrategy ||
         (d->abstractGroupingStrategy && d->abstractGroupingStrategy->type() == strategy)) {
         return;
     }
 
-    d->changingGroupingStragegy = true;
+    d->changingGroupingStrategy = true;
 
     //kDebug() << strategy << kBacktrace();
     if (d->onlyGroupWhenFull) {
@@ -656,13 +657,13 @@
         disconnect(d->rootGroup, SIGNAL(itemRemoved(AbstractItemPtr)), this, SLOT(checkIfFull()));
     }
 
-    delete d->abstractGroupingStrategy;
-    d->abstractGroupingStrategy = 0;
+    if (d->abstractGroupingStrategy) {
+        disconnect(d->abstractGroupingStrategy, 0, this, 0);
+        d->abstractGroupingStrategy->destroy();
+        d->abstractGroupingStrategy = 0;
+    }
 
     switch (strategy) {
-        case NoGrouping:
-            d->abstractGroupingStrategy = 0;
-            break;
         case ManualGrouping:
             d->abstractGroupingStrategy = new ManualGroupingStrategy(this);
             break;
@@ -675,9 +676,11 @@
             d->abstractGroupingStrategy = new KustodianGroupingStrategy(this);
             break;
 
+        case NoGrouping:
+            break;
+
         default:
             kDebug() << "Strategy not implemented";
-            d->abstractGroupingStrategy = 0;
     }
 
     d->groupingStrategy = strategy;
@@ -694,7 +697,7 @@
         connect(d->rootGroup, SIGNAL(itemRemoved(AbstractItemPtr)), this, SLOT(checkIfFull()));
     }
 
-    d->changingGroupingStragegy = false;
+    d->changingGroupingStrategy = false;
 }
 
 } // TaskManager namespace
--- trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/kustodiangroupingstrategy.cpp #984937:984938
@@ -44,7 +44,6 @@
         :editableGroupProperties(AbstractGroupingStrategy::None)
     {
     }
-    GroupManager *groupManager;
     AbstractGroupingStrategy::EditableGroupProperties editableGroupProperties;
 };
 
@@ -53,7 +52,6 @@
     :AbstractGroupingStrategy(groupManager),
      d(new Private)
 {
-    d->groupManager = groupManager;
     setType(GroupManager::KustodianGrouping);
 
     QStringList defaultApps;
@@ -86,13 +84,17 @@
 
 void KustodianGroupingStrategy::handleItem(AbstractItemPtr item)
 {
+    if (!rootGroup()) {
+        return;
+    }
+
     if (item->isGroupItem()) {
-        d->groupManager->rootGroup()->add(item);
+        rootGroup()->add(item);
         return;
     }
 
     TaskItem *task = dynamic_cast<TaskItem*>(item);
-    if (task && !programGrouping(task, d->groupManager->rootGroup())) {
+    if (task && !programGrouping(task, rootGroup())) {
         QString name = desktopNameFromClassName(task->task()->classClass());
         //kDebug() << "create new subgroup in root as this classname doesn't have a group " << name;
 
--- trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualgroupingstrategy.cpp #984937:984938
@@ -24,6 +24,7 @@
 #include "manualgroupingstrategy.h"
 
 #include <QAction>
+#include <QPointer>
 
 #include <KDebug>
 #include <KLocale>
@@ -48,13 +49,12 @@
     {
     }
 
-    GroupManager *groupManager;
     QHash<int, TaskGroupTemplate*> templateTrees;
     TaskGroupTemplate* currentTemplate;
     QList<TaskGroup*> protectedGroups;
     AbstractGroupingStrategy::EditableGroupProperties editableGroupProperties;
     AbstractGroupableItem *tempItem;
-    TaskGroup *tempGroup;
+    QPointer<TaskGroup> tempGroup;
     int oldDesktop;
 };
 
@@ -64,7 +64,6 @@
     :AbstractGroupingStrategy(groupManager),
      d(new Private)
 {
-    d->groupManager = groupManager;
     setType(GroupManager::ManualGrouping);
 }
 
@@ -110,13 +109,17 @@
 
 void ManualGroupingStrategy::removeGroup()
 {
-    Q_ASSERT(d->tempGroup);
+    if (!d->tempGroup) {
+        return;
+    }
+
     if (d->tempGroup->parentGroup()) {
         foreach (AbstractGroupableItem *item, d->tempGroup->members()) {
             d->tempGroup->parentGroup()->add(item);
         }
         //Group gets automatically closed on empty signal
     }
+
     d->tempGroup = 0;
 }
 
@@ -138,6 +141,10 @@
 //Check if the item was previously manually grouped
 void ManualGroupingStrategy::handleItem(AbstractItemPtr item)
 {
+    if (!rootGroup()) {
+        return;
+    }
+
     //kDebug();
     if (d->currentTemplate) { //TODO this won't work over sessions because the task is identified by the pointer (maybe the name without the current status would work), one way would be to store the items per name if the session is closed and load them per name on startup but use the pointer otherwise because of changing names of browsers etc
         TaskGroupTemplate *templateGroup = d->currentTemplate;
@@ -157,7 +164,7 @@
                     oldTemplateGroup->group()->add(templateGroup->group()); //add group to parent Group
                 } else {
                     //kDebug();
-                    d->groupManager->rootGroup()->add(item);
+                    rootGroup()->add(item);
                     return;
                 }
             }
@@ -167,10 +174,10 @@
             templateGroup->remove(item);
         } else {
             //kDebug() << "Item not in templates";
-            d->groupManager->rootGroup()->add(item);
+            rootGroup()->add(item);
         }
     } else {
-        d->groupManager->rootGroup()->add(item);
+        rootGroup()->add(item);
     }
 }
 
@@ -184,7 +191,7 @@
 void ManualGroupingStrategy::desktopChanged(int newDesktop)
 {
     //kDebug() << "old: " << d->oldDesktop << "new: " << newDesktop;
-    if (d->oldDesktop == newDesktop) {
+    if (d->oldDesktop == newDesktop || !rootGroup()) {
         return;
     }
 
@@ -193,8 +200,8 @@
         d->currentTemplate->clear();
     }
     //kDebug();
-    TaskGroupTemplate *group = createDuplication(d->groupManager->rootGroup());
-    d->templateTrees.insert(d->oldDesktop, group); 
+    TaskGroupTemplate *group = createDuplication(rootGroup());
+    d->templateTrees.insert(d->oldDesktop, group);
     if (d->templateTrees.contains(newDesktop)) {
         //kDebug() << "Template found";
         d->currentTemplate = d->templateTrees.value(newDesktop);
@@ -218,46 +225,48 @@
 {
     //kDebug();
     TaskGroup *group = qobject_cast<TaskGroup*>(sender());
-    if (!group) {
+    if (!group || !rootGroup()) {
         return;
     }
+
     if (newDesktop && (newDesktop != d->oldDesktop)) {
         if (group->parentGroup()) {
             group->parentGroup()->remove(group);
         }
     }
+
     TaskGroupTemplate *templateGroup;
-if (newDesktop) {
-    if (d->templateTrees.contains(newDesktop)) {
-        //kDebug() << "Template found";
-        templateGroup = d->templateTrees.value(newDesktop);
-    } else {
-        //kDebug() << "No Template found";
-        templateGroup = new TaskGroupTemplate(this, 0);
-        templateGroup->setGroup(d->groupManager->rootGroup());
-        d->templateTrees.insert(newDesktop, templateGroup);
-    }
-    //Add group to all existing desktops
-} else {
-    for (int i = 1; i <= TaskManager::self()->numberOfDesktops(); i++) {
+    if (newDesktop) {
         if (d->templateTrees.contains(newDesktop)) {
             //kDebug() << "Template found";
             templateGroup = d->templateTrees.value(newDesktop);
-            if (templateGroup->hasMember(group)) {
-                continue;
-            }
         } else {
             //kDebug() << "No Template found";
             templateGroup = new TaskGroupTemplate(this, 0);
-            templateGroup->setGroup(d->groupManager->rootGroup());
+            templateGroup->setGroup(rootGroup());
             d->templateTrees.insert(newDesktop, templateGroup);
         }
-        templateGroup->add(createDuplication(group));
+        //Add group to all existing desktops
+    } else {
+        for (int i = 1; i <= TaskManager::self()->numberOfDesktops(); i++) {
+            if (d->templateTrees.contains(newDesktop)) {
+                //kDebug() << "Template found";
+                templateGroup = d->templateTrees.value(newDesktop);
+                if (templateGroup->hasMember(group)) {
+                    continue;
+                }
+            } else {
+                //kDebug() << "No Template found";
+                templateGroup = new TaskGroupTemplate(this, 0);
+                templateGroup->setGroup(rootGroup());
+                d->templateTrees.insert(newDesktop, templateGroup);
+            }
+
+            templateGroup->add(createDuplication(group));
+        }
     }
 }
 
-}
-
 bool ManualGroupingStrategy::groupItems(ItemList items)
 {
     //kDebug();
--- trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/programgroupingstrategy.cpp #984937:984938
@@ -44,7 +44,7 @@
         :editableGroupProperties(AbstractGroupingStrategy::None)
     {
     }
-    GroupManager *groupManager;
+
     AbstractGroupingStrategy::EditableGroupProperties editableGroupProperties;
     QPointer<AbstractGroupableItem> tempItem;
     QStringList blackList; //Programs in this list should not be grouped
@@ -55,7 +55,6 @@
     :AbstractGroupingStrategy(groupManager),
      d(new Private)
 {
-    d->groupManager = groupManager;
     setType(GroupManager::ProgramGrouping);
 
     KConfig groupBlacklist( "taskbargroupblacklistrc", KConfig::NoGlobals );
@@ -69,7 +68,7 @@
     KConfigGroup blackGroup( &groupBlacklist, "Blacklist" );
     blackGroup.writeEntry( "Applications", d->blackList );
     blackGroup.config()->sync();
-    
+
     delete d;
 }
 
@@ -123,8 +122,8 @@
         d->blackList.append(name);
         if (d->tempItem->isGroupItem()) {
             closeGroup(qobject_cast<TaskGroup*>(d->tempItem));
-        } else {
-            d->groupManager->rootGroup()->add(d->tempItem);
+        } else if (rootGroup()) {
+            rootGroup()->add(d->tempItem);
         }
     }
 
@@ -133,20 +132,26 @@
 
 void ProgramGroupingStrategy::handleItem(AbstractItemPtr item)
 {
+    GroupPtr root = rootGroup();
+
+    if (!root) {
+        return;
+    }
+
     if (item->isGroupItem()) {
         //kDebug() << "item is groupitem";
-        d->groupManager->rootGroup()->add(item);
+        root->add(item);
         return;
     } else if (d->blackList.contains((qobject_cast<TaskItem*>(item))->task()->classClass())) {
         //kDebug() << "item is in blacklist";
-        d->groupManager->rootGroup()->add(item);
+        root->add(item);
         return;
     }
 
     TaskItem *task = dynamic_cast<TaskItem*>(item);
-    if (task && !programGrouping(task, d->groupManager->rootGroup())) {
+    if (task && !programGrouping(task, root)) {
         //kDebug() << "joined rootGroup ";
-        d->groupManager->rootGroup()->add(item);
+        root->add(item);
     }
 }
 


More information about the Plasma-devel mailing list