Merge lp:~aacid/unity-2d/fix_memory_leaks_from_qtbamf_lists into lp:unity-2d

[Albert Astals Cid]

Use QScopedPointers to properly delete the lists returned from qtbamf

[Albert Astals Cid (aacid)]

There is one fix missing, it is part of https://code.launchpad.net/~aacid/unity-2d/doNotCallWindowsMoreThanNeeded_854634/+merge/87256

[Lohith D Shivamurthy (dyams)]

Albert,
Very nice. Good catch. I can approve the branch already.

I wanted to get clarified for one thing more. Well, In a way, It mayn't be related to your current branch. But just in case if you are also as curious as me, go through the following paragraph.

I just observed one corner case not being handled properly in
  <code> WindowInfo::getBamfWindowForApplication </code> line #46, in windowinfo.cpp
Currently whats happening is, even when there is no match found for given 'xid' in the list of applicatio->windows(),
The function still returns a valid BamfWindow pointer, which is the last item in the list.
 <code> window = windows->at(i); </code> line #56, windowinfo.cpp
Should we add
  <code> window = NULL </code>
line below:
  <code>
    if (window->xid() == xid) { line # 57, 58 & 59, in windowinfo.cpp
      break;
    }
  </code>
?

Correct me if am wrong. please.

review: Needs Information

[Albert Astals Cid (aacid)]

Right, seems if that the xid is not found we are returning the last window, probably not the desired behaviour of the function.

That was already the old behaviour before my patch, i can fix that if you want, but for me it makes more sense to do so in a different merge request to keep stuff more contained. What to you think?

[Albert Astals Cid (aacid)]

As discussed over IRC, fixing the other issue in a separate merge request https://code.launchpad.net/~aacid/unity-2d/fixGetBamfWindowForApplication/+merge/87321

[Lohith D Shivamurthy (dyams)]

Nice.
Thank you.

=== modified file 'libunity-2d-private/src/launcherapplication.cpp'
--- libunity-2d-private/src/launcherapplication.cpp	2011-11-24 08:45:18 +0000
+++ libunity-2d-private/src/launcherapplication.cpp	2012-01-02 15:37:24 +0000
@@ -763,7 +763,7 @@
 
     if (compiz.isValid()) {
         Qt::HANDLE root = QX11Info::appRootWindow();
-        BamfUintList* xids = m_application->xids();
+        QScopedPointer<BamfUintList> xids(m_application->xids());
         QStringList fragments;
         for (int i = 0; i < xids->size(); i++) {
             uint xid = xids->at(i);

=== modified file 'libunity-2d-private/src/windowinfo.cpp'
--- libunity-2d-private/src/windowinfo.cpp	2011-08-01 13:04:32 +0000
+++ libunity-2d-private/src/windowinfo.cpp	2012-01-02 15:37:24 +0000
@@ -51,7 +51,7 @@
     }
 
     BamfWindow *window = NULL;
-    BamfWindowList *windows = application->windows();
+    QScopedPointer<BamfWindowList> windows(application->windows());
     for (int i = 0; i < windows->size(); i++) {
         window = windows->at(i);
         if (window->xid() == xid) {

=== modified file 'libunity-2d-private/src/windowslist.cpp'
--- libunity-2d-private/src/windowslist.cpp	2011-07-29 13:49:34 +0000
+++ libunity-2d-private/src/windowslist.cpp	2012-01-02 15:37:24 +0000
@@ -85,7 +85,7 @@
     QList<BamfApplication*> applications;
 
     /* List the windows of all the applications */
-    BamfApplicationList *allapplications = matcher.applications();
+    QScopedPointer<BamfApplicationList> allapplications(matcher.applications());
     for (int i = 0; i < allapplications->size(); i++) {
          applications.append(allapplications->at(i));
     }
@@ -97,7 +97,7 @@
             continue;
         }
 
-        BamfWindowList *bamfWindows = application->windows();
+        QScopedPointer<BamfWindowList> bamfWindows(application->windows());
         for (int i = 0; i < bamfWindows->size(); i++) {
             BamfWindow* window = bamfWindows->at(i);
             if (!window->user_visible()) {