Review Request: Workaround Taskbar ghost entries bug

[Aaron J. Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102779/#review7107
-----------------------------------------------------------


the check makes sense; still wish we knew *where* these odd tasks were coming from (my guess is that they are created and then deleted almost immediately somewhere, but not before the tasks widget is told about them ..)

in any case, it looks like there is a potential problem with the new'ing of the WindowTaskItem when the groupItem is invalid. if you could clear that up and then commit, that'd be awesome. thanks for tracking this down! i know how much time and effort you poured into this ... many will be thankful.


plasma/desktop/applets/tasks/taskgroupitem.cpp
<http://git.reviewboard.kde.org/r/102779/#comment6235>

    looks like a possible memory leak (well, not quite; it's still parented properly) due to the WindowTaskItem being new'd but not actually used.
    
    it could be written sth like this instead:
    
    } else {
        TaskManager::TaskItem * taskItem = static_cast<TaskManager::TaskItem*>(groupableItem);
    
        if (taskItem->startup() || taskItem->task()) {
            WindowTaskItem *windowItem = new WindowTaskItem(this, m_applet);
            windowItem->setTask(taskItem);
            item = windowItem;
        }
    }
    
    if (!item) {
        return 0;
    }


- Aaron J. Seigo


On Oct. 5, 2011, 9:26 a.m., Alex Fiestas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102779/
> -----------------------------------------------------------
> 
> (Updated Oct. 5, 2011, 9:26 a.m.)
> 
> 
> Review request for Plasma and Aaron J. Seigo.
> 
> 
> Description
> -------
> 
> Well, as some of you may know I have been tracking down the ghost entries bug... this time I have NOT been able to fix the source of the problem but at least after some GDB at BDS I think I have a workaround.
> 
> A Ghost entry happens when a TaskItem is not either a startup or a task, in that case the code that "paints" the task doesn't know what to do but it reserve the space anyway.
> Currently the code path is like this:
> 1-TaskGruopItem::itemAdded is called
> 2-It calles TaskGruopItem::createAbstractItem
> 3-createAbstractItem takes a GroupableItem and cast it either to a GroupItem, or LauncherItem or TaskItem depending on the ::itemTtype()
> 
> createAbstractItem is prepared to return 0 when an AbstractTaskItem can't be created, and even further WindowTaskItem::setTask is checking if the taskItem is not a startup or a task, the problem is that setTask returns void so we can't really know if the "setting" was succeed or not. In that case, an "empty" WindowTaskItem is returned, being it a ghost entry.
> 
> This workaround what does is check if the taskItem is valid, if it is not 0 is returned.
> 
> This is a workaround since, in theory a TaskItem should ALWAYS be either a startup or a task so we should find the root of the problem instead of adding yet another check. Is precisely this kind of checks that make all *tasks* stack a mess.
> 
> 
> Diffs
> -----
> 
>   plasma/desktop/applets/tasks/taskgroupitem.cpp eec27c2 
> 
> Diff: http://git.reviewboard.kde.org/r/102779/diff/diff
> 
> 
> Testing
> -------
> 
> I have been using the patch for 3 days playing around with activities and virtual desktops, so far so good.
> 
> 
> Thanks,
> 
> Alex Fiestas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20111005/dcbd31a2/attachment-0001.html>