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:[email protected]
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);
     }
 }
 
_______________________________________________
Plasma-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to