Merge lp:~charlesk/hud/qtgactiongroup-cleanup into lp:hud/14.04

[Charles Kerr]

Simplify the implementation of QtGActionGroup.

This started off with the intent of removing the overhead of g_action_group_list_actions() in QtGActionGroup::Action(), but then I found QtGActionGroup::Action() was only called in the object's constructor and destructor, so it made more sense to remove the function altogether.

Summary of changes:

  * Plugged GVariant leak in QtGActionGroup::TriggerAction()

  * Plugged char* leak in QtGMenuUtils::makeStringListQVariant

  * ActionGroup ctor calls g_action_group_list_actions() once instead of n+1 times

  * ActionGroup dtor calls g_action_group_list_actions() once instead of n times

  * Removed unused public method Size()

  * Removed newly-unused public method Action()

  * Removed newly-unused field m_size

* Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
  * Yes
 * Did you build your software in a clean sbuild/pbuilder chroot or ppa?
  * Yes
 * Did you build your software in a clean sbuild/pbuilder armhf chroot or ppa?
  * Yes
 * Has your component "TestPlan” been executed successfully on emulator, N4?
  * Yes
 * Has a 5 minute exploratory testing run been executed on N4?
  * Yes
 * If you changed the packaging (debian), did you subscribe a core-dev to this MP?
  * N/A
 * If you changed the UI, did you subscribe the design-reviewers to this MP?
  * No change
 * What components might get impacted by your changes?
  * Unity7
  * Unity8
 * Have you requested review by the teams of these owning components?
  * Yes

Check List:
https://wiki.ubuntu.com/Process/Merges/Checklists/hud

Test Plan:
https://wiki.ubuntu.com/Process/Merges/TestPlan/hud

Silo:
<waiting for silo>

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:382
http://jenkins.qa.ubuntu.com/job/hud-ci/259/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/110
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/109
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/109

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/259/rebuild

review: Approve (continuous-integration)

[Marcus Tomlinson (marcustomlinson)]

LGTM

review: Approve

=== modified file 'libqtgmenu/internal/QtGActionGroup.cpp'
--- libqtgmenu/internal/QtGActionGroup.cpp	2014-03-11 05:41:28 +0000
+++ libqtgmenu/internal/QtGActionGroup.cpp	2014-03-17 18:40:44 +0000
@@ -27,22 +27,16 @@
 {
   ConnectCallbacks();
 
-  gchar** actions_list = g_action_group_list_actions( m_action_group );
+  auto actions_list = g_action_group_list_actions( m_action_group );
   if( actions_list )
   {
-    while( actions_list[m_size] != nullptr )
+    for( int i = 0; actions_list[i]; ++i )
     {
-      ++m_size;
+      emit ActionAdded( actions_list[i] );
     }
 
     g_strfreev( actions_list );
   }
-
-  // add initial items
-  for( int i = 0; i < m_size; ++i )
-  {
-    emit ActionAdded( Action( i ) );
-  }
 }
 
 QtGActionGroup::~QtGActionGroup()
@@ -52,9 +46,15 @@
     return;
   }
 
-  for( int i = 0; i < m_size; ++i )
+  auto actions_list = g_action_group_list_actions( m_action_group );
+  if( actions_list )
   {
-    emit ActionRemoved( Action( i ) );
+    for( int i = 0; actions_list[i]; ++i )
+    {
+      emit ActionRemoved( actions_list[i] );
+    }
+
+    g_strfreev( actions_list );
   }
 
   DisconnectCallbacks();
@@ -70,28 +70,6 @@
   return m_action_group;
 }
 
-int QtGActionGroup::Size() const
-{
-  return m_size;
-}
-
-QString QtGActionGroup::Action( int index )
-{
-  if( index >= m_size )
-  {
-    return QString();
-  }
-
-  QString action_name;
-  gchar** actions_list = g_action_group_list_actions( m_action_group );
-
-  action_name = QString( actions_list[index] );
-
-  g_strfreev( actions_list );
-
-  return action_name;
-}
-
 void QtGActionGroup::TriggerAction( QString action_name, bool checked )
 {
   QString prefix = action_name.left( action_name.indexOf( '.' ) );
@@ -117,15 +95,16 @@
     {
       GVariant* param = g_variant_new_string( action.c_str() );
       g_action_group_activate_action( m_action_group, action.c_str(), param );
+      g_variant_unref( param );
     }
   }
 }
 
 void QtGActionGroup::EmitStates()
 {
-  gchar** actions_list = g_action_group_list_actions( m_action_group );
+  auto actions_list = g_action_group_list_actions( m_action_group );
 
-  for( int i = 0; i < m_size; ++i )
+  for( int i = 0; actions_list && actions_list[i]; ++i )
   {
     gchar* action_name = actions_list[i];
 
@@ -158,8 +137,6 @@
       action_name );
   if( type != nullptr )
     emit self->ActionParameterized( self->m_action_prefix + "." + action_name, type != nullptr );
-
-  ++self->m_size;
 }
 
 void QtGActionGroup::ActionRemovedCallback( GActionGroup* action_group, gchar* action_name,
@@ -167,8 +144,6 @@
 {
   QtGActionGroup* self = reinterpret_cast< QtGActionGroup* >( user_data );
   emit self->ActionRemoved( action_name );
-
-  --self->m_size;
 }
 
 void QtGActionGroup::ActionEnabledCallback( GActionGroup* action_group, gchar* action_name,

=== modified file 'libqtgmenu/internal/QtGActionGroup.h'
--- libqtgmenu/internal/QtGActionGroup.h	2014-03-11 05:31:53 +0000
+++ libqtgmenu/internal/QtGActionGroup.h	2014-03-17 18:40:44 +0000
@@ -37,9 +37,6 @@
   virtual ~QtGActionGroup();
 
   GActionGroup* ActionGroup() const;
-  int Size() const;
-
-  QString Action( int index );
 
 Q_SIGNALS:
   void ActionAdded( QString action_name );
@@ -66,7 +63,6 @@
   void DisconnectCallbacks();
 
 private:
-  int m_size = 0;
 
   QString m_action_prefix;
   GActionGroup* m_action_group = nullptr;

=== modified file 'libqtgmenu/internal/QtGMenuUtils.cpp'
--- libqtgmenu/internal/QtGMenuUtils.cpp	2013-11-20 04:28:46 +0000
+++ libqtgmenu/internal/QtGMenuUtils.cpp	2014-03-17 18:40:44 +0000
@@ -92,6 +92,8 @@
   while( stringArray[i] != NULL )
     value << QString::fromUtf8( stringArray[i++] );
 
+  g_free (const_cast<gchar**>(stringArray));
+
   return QVariant::fromValue( value );
 }