Unity

Merge lp://qastaging/~3v1n0/unity/quicklist-menu-items-leak-fix into lp://qastaging/unity

  1. quicklist-menu-items-leak-fix
  2. Merge into trunk
Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: no longer in the source branch.
Merged at revision: 2565
Proposed branch: lp://qastaging/~3v1n0/unity/quicklist-menu-items-leak-fix
Merge into: lp://qastaging/unity
Diff against target: 759 lines (+106/-124)
26 files modified
launcher/AbstractLauncherIcon.h (+2/-1)
launcher/BFBLauncherIcon.cpp (+16/-31)
launcher/BFBLauncherIcon.h (+7/-5)
launcher/BamfLauncherIcon.cpp (+31/-25)
launcher/BamfLauncherIcon.h (+1/-1)
launcher/DeviceLauncherIcon.cpp (+3/-3)
launcher/DeviceLauncherIcon.h (+1/-1)
launcher/HudLauncherIcon.cpp (+0/-6)
launcher/HudLauncherIcon.h (+0/-1)
launcher/LauncherIcon.cpp (+8/-7)
launcher/LauncherIcon.h (+2/-2)
launcher/MockLauncherIcon.h (+2/-2)
launcher/QuicklistMenuItem.cpp (+2/-2)
launcher/QuicklistMenuItem.h (+1/-1)
launcher/QuicklistMenuItemCheckmark.cpp (+1/-1)
launcher/QuicklistMenuItemCheckmark.h (+1/-1)
launcher/QuicklistMenuItemLabel.cpp (+1/-1)
launcher/QuicklistMenuItemLabel.h (+1/-1)
launcher/QuicklistMenuItemRadio.cpp (+1/-1)
launcher/QuicklistMenuItemRadio.h (+1/-1)
launcher/QuicklistMenuItemSeparator.cpp (+1/-1)
launcher/QuicklistMenuItemSeparator.h (+1/-1)
launcher/TrashLauncherIcon.cpp (+7/-14)
launcher/TrashLauncherIcon.h (+2/-2)
tests/test_bfb_launcher_icon.cpp (+1/-1)
tests/test_quicklist_view.cpp (+12/-11)
To merge this branch: bzr merge lp://qastaging/~3v1n0/unity/quicklist-menu-items-leak-fix
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Tim Penhey Pending
Andrea Azzarone Pending
Review via email: mp+119606@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2012-08-13.

Commit message

AbstractLauncherIcon: Use a vector of glib::Object<DbusmenuMenuitem> to get the Menus

Description of the change

We have some leak around in the quicklist menu items, for many icons the allocated DbusmenuMenuitem are never unreferenced.
Fixing this by moving the whole stack to the usage of the glib::Object<DbusmenuMenuitem> smart pointers, so that we can be sure that even the temporary objects are reffed/unreffed when needed.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote : Posted in a previous version of this proposal

Why did you make a bunch of properties public in QuicklistMenuItem?

review: Needs Information
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

Ooops... Because I wanted to test one thing and I forgot to put the things back.
Thanks for noticing it, fixed!

Revision history for this message
Andrea Azzarone (azzar1) wrote : Posted in a previous version of this proposal

typedef std::vector<glib::Object<DbusmenuMenuitem>> MenuItemsList;

/me doesn't like this typedef beucase it adds disinformation ;)

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

Yes, I don't like either, but the type here was really too complex and long to keep it in the original form...

Revision history for this message
Andrea Azzarone (azzar1) wrote : Posted in a previous version of this proposal

> Yes, I don't like either, but the type here was really too complex and long to
> keep it in the original form...

What about MenuItemsVector?

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

If you really prefer this one... Done!

Revision history for this message
Andrea Azzarone (azzar1) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote : Posted in a previous version of this proposal

No proposals found for merge of lp:~3v1n0/unity/bfb-quicklist-does-not-close-dash into lp:unity.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Approving as it was already approved by andyrock.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.