Merge lp:~unity-api-team/hud/lp-1298656 into lp:hud/14.04

Proposed by Pete Woods
Status: Merged
Approved by: Pete Woods
Approved revision: 423
Merged at revision: 391
Proposed branch: lp:~unity-api-team/hud/lp-1298656
Merge into: lp:hud/14.04
Diff against target: 1441 lines (+508/-209)
14 files modified
data/hud.conf.in (+0/-12)
debian/changelog (+27/-0)
libqtgmenu/QtGMenuImporter.cpp (+2/-2)
libqtgmenu/QtGMenuImporter.h (+2/-2)
libqtgmenu/internal/QtGActionGroup.cpp (+31/-31)
libqtgmenu/internal/QtGActionGroup.h (+7/-3)
libqtgmenu/internal/QtGMenuImporterPrivate.cpp (+21/-13)
libqtgmenu/internal/QtGMenuImporterPrivate.h (+4/-3)
libqtgmenu/internal/QtGMenuModel.cpp (+331/-99)
libqtgmenu/internal/QtGMenuModel.h (+20/-17)
service/DBusMenuCollector.cpp (+9/-17)
service/HudServiceImpl.cpp (+45/-5)
service/HudServiceImpl.h (+5/-1)
tests/unit/qtgmenu/TestQtGMenu.cpp (+4/-4)
To merge this branch: bzr merge lp:~unity-api-team/hud/lp-1298656
Reviewer Review Type Date Requested Status
Antti Kaijanmäki (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+215459@code.launchpad.net

Commit message

Harden HUD against misbehaving applications

Report the offending applications using Apport's recoverable problem tool.
Switch to using shared pointers where possible for managing memory.

Description of the change

* 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:
https://launchpad.net/~ci-train-ppa-service/+archive/landing-009

To post a comment you must log in.
lp:~unity-api-team/hud/lp-1298656 updated
398. By Pete Woods

More smart pointers

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
399. By Marcus Tomlinson

Set QAction parent to the QMenu it is added to.

400. By Marcus Tomlinson

Emit MenuItemsChanged signal upon inserting a child model.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
401. By Marcus Tomlinson

1. Publicly expose the static QtGMenuModel::CreateChild method (this method is safe enough for public use and simplifies testing).
2. Reference count the QActions in the m_actions map as we could get duplicates in the menu hierarchy.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
402. By Marcus Tomlinson

Abort when on an invalid item index from "items-changed", and added simple CreateChild test.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
403. By Marcus Tomlinson

Added sender's process command line as a local string to AbortWithLocals

404. By Marcus Tomlinson

Reduce number of calls to g_menu_model_get_n_items

405. By Marcus Tomlinson

Removed now invalid CreateChild test

406. By Marcus Tomlinson

Some cleaning up

407. By Marcus Tomlinson

Ah, nobody cares if there's a ", " at the end of a list

408. By Marcus Tomlinson

Missing break statements

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
409. By Pete Woods

Report a recoverable error for the application that is sending the strange signals

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
410. By Pete Woods

Switch to simpler QDBus call for process owner

411. By Pete Woods

Close the write channel for the recoverable report

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
412. By Pete Woods

Separate with semicolons, only set parent properties if there's a parent

413. By Pete Woods

We can't start detached if we want to write to stdin

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
414. By Pete Woods

Whoops!

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
415. By Pete Woods

Clean up the error reporting

416. By Pete Woods

Change bucket signature

417. By Pete Woods

More shared pointers, and fix string replacement of underscores

418. By Pete Woods

Disconnect from GMenus as soon as they send us any invalid information

419. By Pete Woods

Fix the error reporting - write nulls instead of new lines

420. By Pete Woods

I don't think we should have deleted this line

421. By Pete Woods

Reduce noise

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
422. By Pete Woods

Yet more shared pointers

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~unity-api-team/hud/lp-1298656 updated
423. By Pete Woods

Check removed items range

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

OK. looks good.

Quite a lot of noise from the raw to smart pointer transition, but that's just what it is.

I am a bit concerned about the mixing of std::shared_ptr's and QSharedPointers, but that is not the topic of this review. Just something you guys might want to consider in the future.

Approved.

review: Approve
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

* Are any changes against your component pending/needed to land the MP under review in a functional state and are those called out explicitly by the submitter?

No. Yes.

* Did you do exploratory testing related to the component you own with the MP changeset included?

I don't own anything.

* Has the submitter requested review by all the relevant teams/reviewres?

Yes.

* If you are the reviewer owning the component the MP is against, have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?

Yes.

lp:~unity-api-team/hud/lp-1298656 updated
424. By Marcus Tomlinson

Store all QAction pointers of actions with the same name, rather than just the first occurrence, as that pointer could become invalid.

425. By Pete Woods

Remove duplication from upstart config

426. By Pete Woods

Add warning about missing actions on update status

427. By Marcus Tomlinson

Emit ActionEnabled and ActionParameterized regardless of value in EmitStates() (these methods are far quicker now so no need for the crude optimisation)

428. By Marcus Tomlinson

Removed unused local variable

429. By Marcus Tomlinson

Moved m_parent->UpdateExtQMenu() into UpdateExtQMenu()

430. By Marcus Tomlinson

When updating m_children upon addition of new items, start from the end and work backwards as not to overlap items as we shift them up.

431. By Marcus Tomlinson

Not finding an action is acceptable as we need the action group to emit all of its action states when menus change. Hence, we often receive states for items not yet added.

432. By Pete Woods

Rearrange the safety valve check to try and be sure it can't be skipped somehow

433. By Pete Woods

Legacy queries now time out after being "closed" by the user interface for a certain time

434. By Pete Woods

Update changelog ready for release

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/hud.conf.in'
2--- data/hud.conf.in 2013-11-27 14:28:43 +0000
3+++ data/hud.conf.in 2014-06-04 13:54:33 +0000
4@@ -1,22 +1,10 @@
5 description "Unity HUD"
6
7-pre-start script
8- if [ -z $DBUS_SESSION_BUS_ADDRESS ]; then
9- echo "Working around missing DBUS_SESSION_BUS_ADDRESS variable"
10- printf "DuplicateSignature\0DBusSessionAddressNotSet" | /usr/share/apport/recoverable_problem -p `pidof -s -o 1 init`
11- . "${HOME}/.cache/upstart/dbus-session"
12- initctl set-env "DBUS_SESSION_BUS_ADDRESS=$DBUS_SESSION_BUS_ADDRESS"
13- fi
14-end script
15-
16 # Currently started by the dbus service file
17 # start on dbus-activation com.canonical.hud
18 stop on desktop-end
19 start on started dbus and ((xsession SESSION=ubuntu-touch) or (xsession SESSION=ubuntu-touch-surfaceflinger) or (xsession SESSION=ubuntu))
20
21-env HUD_SERVICE_TIMEOUT=0
22-export HUD_SERVICE_TIMEOUT
23-
24 pre-start script
25 if [ -z $DBUS_SESSION_BUS_ADDRESS ]; then
26 echo "Working around missing DBUS_SESSION_BUS_ADDRESS variable"
27
28=== modified file 'debian/changelog'
29--- debian/changelog 2014-04-02 10:38:40 +0000
30+++ debian/changelog 2014-06-04 13:54:33 +0000
31@@ -1,3 +1,30 @@
32+hud (14.04-0ubuntu1) UNRELEASED; urgency=medium
33+
34+ [ Pete Woods ]
35+ * Resolve crasher in previous attempted SRU. (LP: #1298656)
36+ - Fix order of menu traversal.
37+ - Add timeout to legacy HUD queries.
38+ - Improve legacy menu safety valve trigger.
39+ - Remove duplicate entries in upstart job.
40+
41+ -- Pete Woods <pete.woods@canonical.com> Wed, 28 May 2014 10:16:06 +0200
42+
43+hud (13.10.1+14.04.20140425-0ubuntu1) trusty; urgency=low
44+
45+ [ Pete Woods ]
46+ * Harden HUD against misbehaving applications Report the offending
47+ applications using Apport's recoverable problem tool. Switch to
48+ using shared pointers where possible for managing memory. (LP:
49+ #1298656)
50+
51+ [ Marcus Tomlinson ]
52+ * Harden HUD against misbehaving applications Report the offending
53+ applications using Apport's recoverable problem tool. Switch to
54+ using shared pointers where possible for managing memory. (LP:
55+ #1298656)
56+
57+ -- Ubuntu daily release <ps-jenkins@lists.canonical.com> Fri, 25 Apr 2014 08:48:46 +0000
58+
59 hud (13.10.1+14.04.20140402-0ubuntu1) trusty; urgency=low
60
61 [ Pete Woods ]
62
63=== modified file 'libqtgmenu/QtGMenuImporter.cpp'
64--- libqtgmenu/QtGMenuImporter.cpp 2014-03-11 05:04:08 +0000
65+++ libqtgmenu/QtGMenuImporter.cpp 2014-06-04 13:54:33 +0000
66@@ -43,12 +43,12 @@
67 {
68 }
69
70-GMenuModel* QtGMenuImporter::GetGMenuModel() const
71+QSharedPointer<GMenuModel> QtGMenuImporter::GetGMenuModel() const
72 {
73 return d->GetGMenuModel();
74 }
75
76-GActionGroup* QtGMenuImporter::GetGActionGroup( int index ) const
77+QSharedPointer<GActionGroup> QtGMenuImporter::GetGActionGroup( int index ) const
78 {
79 return d->GetGActionGroup( index );
80 }
81
82=== modified file 'libqtgmenu/QtGMenuImporter.h'
83--- libqtgmenu/QtGMenuImporter.h 2014-03-12 09:04:14 +0000
84+++ libqtgmenu/QtGMenuImporter.h 2014-06-04 13:54:33 +0000
85@@ -47,8 +47,8 @@
86 const QMap<QString, QDBusObjectPath>& action_paths, QObject* parent = 0 );
87 virtual ~QtGMenuImporter();
88
89- GMenuModel* GetGMenuModel() const;
90- GActionGroup* GetGActionGroup( int index = 0 ) const;
91+ QSharedPointer<GMenuModel> GetGMenuModel() const;
92+ QSharedPointer<GActionGroup> GetGActionGroup( int index = 0 ) const;
93
94 std::shared_ptr< QMenu > GetQMenu() const;
95
96
97=== modified file 'libqtgmenu/internal/QtGActionGroup.cpp'
98--- libqtgmenu/internal/QtGActionGroup.cpp 2014-03-19 23:40:09 +0000
99+++ libqtgmenu/internal/QtGActionGroup.cpp 2014-06-04 13:54:33 +0000
100@@ -21,13 +21,20 @@
101
102 using namespace qtgmenu;
103
104-QtGActionGroup::QtGActionGroup( const QString& action_prefix, GActionGroup* action_group )
105+QtGActionGroup::QtGActionGroup( QSharedPointer<GDBusConnection> connection,
106+ const QString& action_prefix,
107+ const QString& service,
108+ const QString& path )
109 : m_action_prefix( action_prefix ),
110- m_action_group( action_group )
111+ m_action_group(
112+ G_ACTION_GROUP(
113+ g_dbus_action_group_get(connection.data(),
114+ service.toUtf8().constData(),
115+ path.toUtf8().constData())), &g_object_unref)
116 {
117 ConnectCallbacks();
118
119- auto actions_list = g_action_group_list_actions( m_action_group );
120+ auto actions_list = g_action_group_list_actions( m_action_group.data() );
121 if( actions_list )
122 {
123 for( int i = 0; actions_list[i]; ++i )
124@@ -46,7 +53,7 @@
125 return;
126 }
127
128- auto actions_list = g_action_group_list_actions( m_action_group );
129+ auto actions_list = g_action_group_list_actions( m_action_group.data() );
130 if( actions_list )
131 {
132 for( int i = 0; actions_list[i]; ++i )
133@@ -58,14 +65,9 @@
134 }
135
136 DisconnectCallbacks();
137-
138- g_object_unref( m_action_group );
139- m_action_group = nullptr;
140-
141- return;
142 }
143
144-GActionGroup* QtGActionGroup::ActionGroup() const
145+QSharedPointer<GActionGroup> QtGActionGroup::ActionGroup() const
146 {
147 return m_action_group;
148 }
149@@ -83,12 +85,12 @@
150 const QString& action(split.second);
151 QByteArray action_utf = action.toUtf8();
152
153- const GVariantType* type = g_action_group_get_action_parameter_type( m_action_group,
154+ const GVariantType* type = g_action_group_get_action_parameter_type( m_action_group.data(),
155 action_utf.constData() );
156
157 if( type == nullptr )
158 {
159- g_action_group_activate_action( m_action_group, action_utf.constData(), nullptr );
160+ g_action_group_activate_action( m_action_group.data(), action_utf.constData(), nullptr );
161 }
162 else
163 {
164@@ -96,7 +98,7 @@
165 if( g_variant_type_equal( type, G_VARIANT_TYPE_STRING ) )
166 {
167 GVariant* param = g_variant_new_string( action_utf.constData() );
168- g_action_group_activate_action( m_action_group, action_utf.constData(), param );
169+ g_action_group_activate_action( m_action_group.data(), action_utf.constData(), param );
170 g_variant_unref( param );
171 }
172 }
173@@ -116,21 +118,19 @@
174
175 void QtGActionGroup::EmitStates()
176 {
177- auto actions_list = g_action_group_list_actions( m_action_group );
178+ auto actions_list = g_action_group_list_actions( m_action_group.data() );
179
180 for( int i = 0; actions_list && actions_list[i]; ++i )
181 {
182 gchar* action_name = actions_list[i];
183
184- bool enabled = G_ACTION_GROUP_GET_IFACE( m_action_group ) ->get_action_enabled( m_action_group,
185+ bool enabled = G_ACTION_GROUP_GET_IFACE( m_action_group.data() ) ->get_action_enabled( m_action_group.data(),
186 action_name );
187- if( !enabled )
188- emit ActionEnabled( FullName(m_action_prefix, action_name), enabled );
189+ emit ActionEnabled( FullName(m_action_prefix, action_name), enabled );
190
191- const GVariantType* type = g_action_group_get_action_parameter_type( m_action_group,
192+ const GVariantType* type = g_action_group_get_action_parameter_type( m_action_group.data(),
193 action_name );
194- if( type != nullptr )
195- emit ActionParameterized( FullName(m_action_prefix, action_name), type != nullptr );
196+ emit ActionParameterized( FullName(m_action_prefix, action_name), type != nullptr );
197 }
198
199 g_strfreev( actions_list );
200@@ -142,12 +142,12 @@
201 QtGActionGroup* self = reinterpret_cast< QtGActionGroup* >( user_data );
202 emit self->ActionAdded( action_name );
203
204- bool enabled = G_ACTION_GROUP_GET_IFACE( self->m_action_group ) ->get_action_enabled(
205- self->m_action_group, action_name );
206+ bool enabled = G_ACTION_GROUP_GET_IFACE( self->m_action_group.data() ) ->get_action_enabled(
207+ self->m_action_group.data(), action_name );
208 if( !enabled )
209 emit self->ActionEnabled( FullName(self->m_action_prefix, action_name), enabled );
210
211- const GVariantType* type = g_action_group_get_action_parameter_type( self->m_action_group,
212+ const GVariantType* type = g_action_group_get_action_parameter_type( self->m_action_group.data(),
213 action_name );
214 if( type != nullptr )
215 emit self->ActionParameterized( FullName(self->m_action_prefix, action_name), type != nullptr );
216@@ -178,22 +178,22 @@
217 {
218 if( m_action_group && m_action_added_handler == 0 )
219 {
220- m_action_added_handler = g_signal_connect( m_action_group, "action-added",
221+ m_action_added_handler = g_signal_connect( m_action_group.data(), "action-added",
222 G_CALLBACK( ActionAddedCallback ), this );
223 }
224 if( m_action_group && m_action_removed_handler == 0 )
225 {
226- m_action_removed_handler = g_signal_connect( m_action_group, "action-removed",
227+ m_action_removed_handler = g_signal_connect( m_action_group.data(), "action-removed",
228 G_CALLBACK( ActionRemovedCallback ), this );
229 }
230 if( m_action_group && m_action_enabled_handler == 0 )
231 {
232- m_action_enabled_handler = g_signal_connect( m_action_group, "action-enabled-changed",
233+ m_action_enabled_handler = g_signal_connect( m_action_group.data(), "action-enabled-changed",
234 G_CALLBACK( ActionEnabledCallback ), this );
235 }
236 if( m_action_group && m_action_state_changed_handler == 0 )
237 {
238- m_action_state_changed_handler = g_signal_connect( m_action_group, "action-state-changed",
239+ m_action_state_changed_handler = g_signal_connect( m_action_group.data(), "action-state-changed",
240 G_CALLBACK( ActionStateChangedCallback ), this );
241 }
242 }
243@@ -202,19 +202,19 @@
244 {
245 if( m_action_group && m_action_added_handler != 0 )
246 {
247- g_signal_handler_disconnect( m_action_group, m_action_added_handler );
248+ g_signal_handler_disconnect( m_action_group.data(), m_action_added_handler );
249 }
250 if( m_action_group && m_action_removed_handler != 0 )
251 {
252- g_signal_handler_disconnect( m_action_group, m_action_removed_handler );
253+ g_signal_handler_disconnect( m_action_group.data(), m_action_removed_handler );
254 }
255 if( m_action_group && m_action_enabled_handler != 0 )
256 {
257- g_signal_handler_disconnect( m_action_group, m_action_enabled_handler );
258+ g_signal_handler_disconnect( m_action_group.data(), m_action_enabled_handler );
259 }
260 if( m_action_group && m_action_state_changed_handler != 0 )
261 {
262- g_signal_handler_disconnect( m_action_group, m_action_state_changed_handler );
263+ g_signal_handler_disconnect( m_action_group.data(), m_action_state_changed_handler );
264 }
265
266 m_action_added_handler = 0;
267
268=== modified file 'libqtgmenu/internal/QtGActionGroup.h'
269--- libqtgmenu/internal/QtGActionGroup.h 2014-03-19 23:40:09 +0000
270+++ libqtgmenu/internal/QtGActionGroup.h 2014-06-04 13:54:33 +0000
271@@ -20,6 +20,7 @@
272 #define QTGACTIONGROUP_H
273
274 #include <QObject>
275+#include <QSharedPointer>
276 #include <QVariant>
277
278 #undef signals
279@@ -33,10 +34,13 @@
280 Q_OBJECT
281
282 public:
283- QtGActionGroup( const QString& action_prefix, GActionGroup* action_group );
284+ QtGActionGroup(QSharedPointer<GDBusConnection> connection,
285+ const QString& action_prefix,
286+ const QString& service,
287+ const QString& path);
288 virtual ~QtGActionGroup();
289
290- GActionGroup* ActionGroup() const;
291+ QSharedPointer<GActionGroup> ActionGroup() const;
292
293 Q_SIGNALS:
294 void ActionAdded( QString action_name );
295@@ -66,7 +70,7 @@
296 private:
297
298 QString m_action_prefix;
299- GActionGroup* m_action_group = nullptr;
300+ QSharedPointer<GActionGroup> m_action_group;
301
302 gulong m_action_added_handler = 0;
303 gulong m_action_removed_handler = 0;
304
305=== modified file 'libqtgmenu/internal/QtGMenuImporterPrivate.cpp'
306--- libqtgmenu/internal/QtGMenuImporterPrivate.cpp 2014-03-19 23:26:48 +0000
307+++ libqtgmenu/internal/QtGMenuImporterPrivate.cpp 2014-06-04 13:54:33 +0000
308@@ -30,7 +30,7 @@
309 m_service_watcher( service, QDBusConnection::sessionBus(),
310 QDBusServiceWatcher::WatchForOwnerChange ),
311 m_parent( parent ),
312- m_connection( g_bus_get_sync( G_BUS_TYPE_SESSION, NULL, NULL ) ),
313+ m_connection( g_bus_get_sync( G_BUS_TYPE_SESSION, NULL, NULL ), &g_object_unref ),
314 m_service( service ),
315 m_menu_path( menu_path ),
316 m_action_paths( action_paths )
317@@ -48,26 +48,24 @@
318 {
319 ClearMenuModel();
320 ClearActionGroups();
321-
322- g_object_unref( m_connection );
323 }
324
325-GMenuModel* QtGMenuImporterPrivate::GetGMenuModel()
326+QSharedPointer<GMenuModel> QtGMenuImporterPrivate::GetGMenuModel()
327 {
328 if( m_menu_model == nullptr )
329 {
330- return nullptr;
331+ return QSharedPointer<GMenuModel>();
332 }
333
334 return m_menu_model->Model();
335 }
336
337-GActionGroup* QtGMenuImporterPrivate::GetGActionGroup( int index )
338+QSharedPointer<GActionGroup> QtGMenuImporterPrivate::GetGActionGroup( int index )
339 {
340 if( index >= m_action_groups.size() ||
341 m_action_groups[index] == nullptr )
342 {
343- return nullptr;
344+ return QSharedPointer<GActionGroup>();
345 }
346
347 return m_action_groups[index]->ActionGroup();
348@@ -160,13 +158,12 @@
349 ClearMenuModel();
350
351 QString menu_path = m_menu_path.path();
352- m_menu_model =
353- std::make_shared< QtGMenuModel > (
354- G_MENU_MODEL( g_dbus_menu_model_get( m_connection, m_service.toUtf8().constData(), menu_path.toUtf8().constData() ) ),
355- m_service, menu_path, m_action_paths );
356+ m_menu_model = std::make_shared< QtGMenuModel > ( m_connection, m_service, menu_path, m_action_paths );
357
358 connect( m_menu_model.get(), SIGNAL( MenuItemsChanged( QtGMenuModel*, int, int,
359 int ) ), &m_parent, SIGNAL( MenuItemsChanged()) );
360+
361+ connect( m_menu_model.get(), SIGNAL( MenuInvalid() ), this, SLOT( MenuInvalid() ) );
362 }
363
364 void QtGMenuImporterPrivate::RefreshGActionGroup()
365@@ -181,8 +178,8 @@
366
367 QString action_path = action_path_it.value().path();
368 m_action_groups.push_back(
369- std::make_shared< QtGActionGroup > ( action_path_it.key(),
370- G_ACTION_GROUP( g_dbus_action_group_get( m_connection, m_service.toUtf8().constData(), action_path.toUtf8().constData() ) ) ) );
371+ std::make_shared<QtGActionGroup>(m_connection,
372+ action_path_it.key(), m_service, action_path));
373
374 auto action_group = m_action_groups.back();
375
376@@ -198,3 +195,14 @@
377
378 LinkMenuActions();
379 }
380+
381+void QtGMenuImporterPrivate::MenuInvalid()
382+{
383+ disconnect( &m_service_watcher, SIGNAL( serviceRegistered( const QString& ) ), this,
384+ SLOT( ServiceRegistered() ) );
385+
386+ disconnect( &m_service_watcher, SIGNAL( serviceUnregistered( const QString& ) ), this,
387+ SLOT( ServiceUnregistered() ) );
388+
389+ ServiceUnregistered();
390+}
391
392=== modified file 'libqtgmenu/internal/QtGMenuImporterPrivate.h'
393--- libqtgmenu/internal/QtGMenuImporterPrivate.h 2014-03-19 23:26:48 +0000
394+++ libqtgmenu/internal/QtGMenuImporterPrivate.h 2014-06-04 13:54:33 +0000
395@@ -43,8 +43,8 @@
396 const QMap<QString, QDBusObjectPath>& action_paths, QtGMenuImporter& parent );
397 virtual ~QtGMenuImporterPrivate();
398
399- GMenuModel* GetGMenuModel();
400- GActionGroup* GetGActionGroup( int index = 0);
401+ QSharedPointer<GMenuModel> GetGMenuModel();
402+ QSharedPointer<GActionGroup> GetGActionGroup( int index = 0);
403
404 std::shared_ptr< QMenu > GetQMenu();
405
406@@ -62,13 +62,14 @@
407
408 void RefreshGMenuModel();
409 void RefreshGActionGroup();
410+ void MenuInvalid();
411
412 private:
413 QDBusServiceWatcher m_service_watcher;
414
415 QtGMenuImporter& m_parent;
416
417- GDBusConnection* m_connection;
418+ QSharedPointer<GDBusConnection> m_connection;
419 QString m_service;
420 QDBusObjectPath m_menu_path;
421 QMap<QString, QDBusObjectPath> m_action_paths;
422
423=== modified file 'libqtgmenu/internal/QtGMenuModel.cpp'
424--- libqtgmenu/internal/QtGMenuModel.cpp 2014-03-19 23:26:48 +0000
425+++ libqtgmenu/internal/QtGMenuModel.cpp 2014-06-04 13:54:33 +0000
426@@ -18,44 +18,52 @@
427
428 #include <QtGMenuModel.h>
429 #include <QtGMenuUtils.h>
430+#include <QCoreApplication>
431+#include <QDBusConnection>
432+#include <QDBusConnectionInterface>
433 #include <QDebug>
434+#include <QProcess>
435+#include <QRegularExpression>
436
437 using namespace qtgmenu;
438
439-QtGMenuModel::QtGMenuModel( GMenuModel* model )
440- : QtGMenuModel( model, LinkType::Root, nullptr, 0 )
441-{
442-}
443+static const QRegularExpression SINGLE_UNDERSCORE("(?<![_])[_](?![_])");
444
445-QtGMenuModel::QtGMenuModel( GMenuModel* model, const QString& bus_name, const QString& menu_path, const QMap<QString, QDBusObjectPath>& action_paths )
446- : QtGMenuModel( model, LinkType::Root, nullptr, 0 )
447+QtGMenuModel::QtGMenuModel( QSharedPointer<GDBusConnection> connection, const QString& bus_name,
448+ const QString& menu_path, const QMap<QString, QDBusObjectPath>& action_paths )
449+ : QtGMenuModel( QSharedPointer<GMenuModel>(G_MENU_MODEL( g_dbus_menu_model_get( connection.data(),
450+ bus_name.toUtf8().constData(),
451+ menu_path.toUtf8().constData() ) ), &g_object_unref ),
452+ LinkType::Root, nullptr, 0 )
453 {
454+ m_connection = connection;
455 m_bus_name = bus_name;
456 m_menu_path = menu_path;
457 m_action_paths = action_paths;
458 }
459
460-QtGMenuModel::QtGMenuModel( GMenuModel* model, LinkType link_type, QtGMenuModel* parent, int index )
461+QtGMenuModel::QtGMenuModel( QSharedPointer<GMenuModel> model, LinkType link_type, QtGMenuModel* parent, int index )
462 : m_parent( parent ),
463 m_model( model ),
464- m_link_type( link_type )
465+ m_link_type( link_type ),
466+ m_menu( new QMenu() ),
467+ m_ext_menu( new QMenu() )
468 {
469 ConnectCallback();
470
471 if( m_parent )
472 {
473- m_parent->InsertChild( this, index );
474-
475+ m_connection = m_parent->m_connection;
476 m_bus_name = m_parent->m_bus_name;
477 m_menu_path = m_parent->m_menu_path;
478 m_action_paths = m_parent->m_action_paths;
479
480 gchar* label = NULL;
481- if( g_menu_model_get_item_attribute( m_parent->m_model, index,
482+ if( g_menu_model_get_item_attribute( m_parent->m_model.data(), index,
483 G_MENU_ATTRIBUTE_LABEL, "s", &label ) )
484 {
485 QString qlabel = QString::fromUtf8( label );
486- qlabel.replace( '_', '&' );
487+ qlabel.replace( SINGLE_UNDERSCORE, "&" );
488 g_free( label );
489
490 m_ext_menu->setTitle( qlabel );
491@@ -63,7 +71,7 @@
492
493 gchar* action_name = NULL;
494 QString qaction_name;
495- if( g_menu_model_get_item_attribute( m_parent->m_model, index,
496+ if( g_menu_model_get_item_attribute( m_parent->m_model.data(), index,
497 G_MENU_ATTRIBUTE_ACTION, "s", &action_name ) )
498 {
499 qaction_name = QString::fromUtf8( action_name );
500@@ -74,7 +82,7 @@
501
502 // if this model has a "commitLabel" property, it is a libhud parameterized action
503 gchar* commit_label = NULL;
504- if( g_menu_model_get_item_attribute( m_parent->m_model, index,
505+ if( g_menu_model_get_item_attribute( m_parent->m_model.data(), index,
506 "commitLabel", "s", &commit_label ) )
507 {
508 g_free( commit_label );
509@@ -100,7 +108,7 @@
510
511 if( m_model )
512 {
513- m_size = g_menu_model_get_n_items( m_model );
514+ m_size = g_menu_model_get_n_items( m_model.data() );
515 }
516
517 ChangeMenuItems( 0, m_size, 0 );
518@@ -112,23 +120,15 @@
519 {
520 if( m_size > 0 )
521 {
522- MenuItemsChangedCallback( m_model, 0, m_size, 0, this );
523+ ChangeMenuItems( 0, 0, m_size );
524 }
525 DisconnectCallback();
526- g_object_unref( m_model );
527 }
528
529- for( auto child : m_children )
530- {
531- delete child;
532- }
533 m_children.clear();
534-
535- delete m_menu;
536- delete m_ext_menu;
537 }
538
539-GMenuModel* QtGMenuModel::Model() const
540+QSharedPointer<GMenuModel> QtGMenuModel::Model() const
541 {
542 return m_model;
543 }
544@@ -138,24 +138,19 @@
545 return m_link_type;
546 }
547
548-int QtGMenuModel::Size() const
549-{
550- return m_size;
551-}
552-
553 QtGMenuModel* QtGMenuModel::Parent() const
554 {
555 return m_parent;
556 }
557
558-QtGMenuModel* QtGMenuModel::Child( int index ) const
559+QSharedPointer<QtGMenuModel> QtGMenuModel::Child( int index ) const
560 {
561 if( m_children.contains( index ) )
562 {
563 return m_children.value( index );
564 }
565
566- return nullptr;
567+ return QSharedPointer<QtGMenuModel>();
568 }
569
570 std::shared_ptr< QMenu > QtGMenuModel::GetQMenu()
571@@ -178,7 +173,10 @@
572 auto action_it = m_actions.find( action_name );
573 if( action_it != end( m_actions ) )
574 {
575- action_it->second->setEnabled( enabled );
576+ for( auto& action : action_it->second )
577+ {
578+ action->setEnabled( enabled );
579+ }
580 }
581 }
582
583@@ -187,78 +185,122 @@
584 auto action_it = m_actions.find( action_name );
585 if( action_it != end( m_actions ) )
586 {
587- action_it->second->setProperty( c_property_isParameterized, parameterized );
588+ for( auto& action : action_it->second )
589+ {
590+ action->setProperty( c_property_isParameterized, parameterized );
591+ }
592 }
593 }
594
595-QtGMenuModel* QtGMenuModel::CreateChild( QtGMenuModel* parent, GMenuModel* model, int index )
596+QSharedPointer<QtGMenuModel> QtGMenuModel::CreateChild( QtGMenuModel* parent_qtgmenu, QSharedPointer<GMenuModel> parent_gmenu, int child_index )
597 {
598- LinkType linkType( LinkType::SubMenu );
599- GMenuModel* link = g_menu_model_get_item_link( model, index, G_MENU_LINK_SUBMENU );
600-
601- if( !link )
602- {
603- linkType = LinkType::Section;
604- link = g_menu_model_get_item_link( model, index, G_MENU_LINK_SECTION );
605- }
606-
607- if( link )
608- {
609- return new QtGMenuModel( link, linkType, parent, index );
610- }
611-
612- return nullptr;
613+ QSharedPointer<QtGMenuModel> new_child;
614+
615+ GMenuLinkIter* link_it = g_menu_model_iterate_item_links( parent_gmenu.data(), child_index );
616+
617+ // get the first link, if it exists, create the child accordingly
618+ if( link_it && g_menu_link_iter_next( link_it ) )
619+ {
620+ // if link is a sub menu
621+ if( strcmp( g_menu_link_iter_get_name( link_it ), G_MENU_LINK_SUBMENU ) == 0 )
622+ {
623+ new_child.reset(
624+ new QtGMenuModel(
625+ QSharedPointer<GMenuModel>(
626+ g_menu_link_iter_get_value(link_it),
627+ &g_object_unref), LinkType::SubMenu,
628+ parent_qtgmenu, child_index));
629+ }
630+ // else if link is a section
631+ else if( strcmp( g_menu_link_iter_get_name( link_it ), G_MENU_LINK_SECTION ) == 0 )
632+ {
633+ new_child.reset(
634+ new QtGMenuModel(
635+ QSharedPointer<GMenuModel>(
636+ g_menu_link_iter_get_value(link_it),
637+ &g_object_unref), LinkType::Section,
638+ parent_qtgmenu, child_index));
639+ }
640+ }
641+
642+ g_object_unref( link_it );
643+ return new_child;
644 }
645
646 void QtGMenuModel::MenuItemsChangedCallback( GMenuModel* model, gint index, gint removed,
647 gint added, gpointer user_data )
648 {
649 QtGMenuModel* self = reinterpret_cast< QtGMenuModel* >( user_data );
650+
651+ if( self->m_model != model )
652+ {
653+ qWarning() << "\"items-changed\" signal received from an unrecognised menu model";
654+ return;
655+ }
656+
657 self->ChangeMenuItems( index, added, removed );
658 }
659
660-void QtGMenuModel::ChangeMenuItems( int index, int added, int removed )
661+void QtGMenuModel::ChangeMenuItems( const int index, const int added, const int removed )
662 {
663+ const int n_items = g_menu_model_get_n_items( m_model.data() );
664+
665+ if( index < 0 || added < 0 || removed < 0 || index + added > n_items || index + removed > m_size )
666+ {
667+ ReportRecoverableError(index, added, removed);
668+ return;
669+ }
670+
671+ // process removed items first (see "items-changed" on the GMenuModel man page)
672 if( removed > 0 )
673 {
674+ // remove QAction from 'index' of our QMenu, 'removed' times
675 for( int i = 0; i < removed; ++i )
676 {
677 if( index < m_menu->actions().size() )
678 {
679 QAction* at_action = m_menu->actions().at( index );
680- ActionRemoved( at_action->property( c_property_actionName ).toString() );
681+ ActionRemoved( at_action->property( c_property_actionName ).toString(), at_action );
682 m_menu->removeAction( at_action );
683 }
684 }
685
686+ // update m_children
687 for( int i = index; i < m_size; ++i )
688 {
689- if( i <= ( index + removed ) )
690+ // remove children from index until ( index + removed )
691+ if( i < ( index + removed ) )
692 {
693- delete m_children.take( i );
694+ m_children.take( i );
695 }
696+ // shift children from ( index + removed ) to m_size into the now empty positions
697 else if( m_children.contains( i ) )
698 {
699 m_children.insert( i - removed, m_children.take( i ) );
700 }
701 }
702
703+ // update m_size
704 m_size -= removed;
705 }
706
707+ // now process added items
708 if( added > 0 )
709 {
710- // shift items up
711- for( int i = ( m_size + added ) - 1; i >= index; --i )
712+ // update m_children (start from the end and work backwards as not to overlap items as we shift them up)
713+ for( int i = m_size - 1; i >= index; --i )
714 {
715+ // shift 'added' items up from their current index to ( index + added )
716 if( m_children.contains( i ) )
717 {
718 m_children.insert( i + added, m_children.take( i ) );
719 }
720 }
721
722+ // update m_size
723 m_size += added;
724
725+ // now add a new QAction to our QMenu for each new item
726 for( int i = index; i < ( index + added ); ++i )
727 {
728 QAction* at_action = nullptr;
729@@ -267,32 +309,37 @@
730 at_action = m_menu->actions().at( i );
731 }
732
733- QtGMenuModel* model = CreateChild( this, m_model, i );
734+ // try first to create a child model
735+ QSharedPointer< QtGMenuModel > model = CreateChild( this, m_model, i );
736
737+ // if this is a menu item and not a model
738 if( !model )
739 {
740 QAction* new_action = CreateAction( i );
741 ActionAdded( new_action->property( c_property_actionName ).toString(), new_action );
742 m_menu->insertAction( at_action, new_action );
743 }
744+ // else if this is a section model
745 else if( model->Type() == LinkType::Section )
746 {
747+ InsertChild( model, i );
748 m_menu->insertSeparator( at_action );
749 }
750+ // else if this is a sub menu model
751 else if( model->Type() == LinkType::SubMenu )
752 {
753- m_menu->insertMenu( at_action, model->m_ext_menu );
754+ InsertChild( model, i );
755+ ActionAdded( model->m_ext_menu->menuAction()->property( c_property_actionName ).toString(),
756+ model->m_ext_menu->menuAction() );
757+ m_menu->insertMenu( at_action, model->m_ext_menu.data() );
758 }
759 }
760 }
761
762 // update external menu
763 UpdateExtQMenu();
764- if( m_link_type == LinkType::Section && m_parent )
765- {
766- m_parent->UpdateExtQMenu();
767- }
768
769+ // now tell the outside world that items have changed
770 emit MenuItemsChanged( this, index, removed, added );
771 }
772
773@@ -300,7 +347,7 @@
774 {
775 if( m_model && m_items_changed_handler == 0 )
776 {
777- m_items_changed_handler = g_signal_connect( m_model, "items-changed",
778+ m_items_changed_handler = g_signal_connect( m_model.data(), "items-changed",
779 G_CALLBACK( MenuItemsChangedCallback ), this );
780 }
781 }
782@@ -309,13 +356,13 @@
783 {
784 if( m_model && m_items_changed_handler != 0 )
785 {
786- g_signal_handler_disconnect( m_model, m_items_changed_handler );
787+ g_signal_handler_disconnect( m_model.data(), m_items_changed_handler );
788 }
789
790 m_items_changed_handler = 0;
791 }
792
793-void QtGMenuModel::InsertChild( QtGMenuModel* child, int index )
794+void QtGMenuModel::InsertChild( QSharedPointer<QtGMenuModel> child, int index )
795 {
796 if( m_children.contains( index ) )
797 {
798@@ -325,35 +372,27 @@
799 child->m_parent = this;
800 m_children.insert( index, child );
801
802- connect( child, SIGNAL( MenuItemsChanged( QtGMenuModel*, int, int, int ) ), this,
803+ connect( child.data(), SIGNAL( MenuItemsChanged( QtGMenuModel*, int, int, int ) ), this,
804 SIGNAL( MenuItemsChanged( QtGMenuModel*, int, int, int ) ) );
805
806- connect( child, SIGNAL( ActionTriggered( QString, bool ) ), this,
807+ connect( child.data(), SIGNAL( ActionTriggered( QString, bool ) ), this,
808 SIGNAL( ActionTriggered( QString, bool ) ) );
809-}
810-
811-int QtGMenuModel::ChildIndex( QtGMenuModel* child )
812-{
813- for( int i = 0; i < m_children.size(); ++i )
814- {
815- if( child == m_children[i] )
816- {
817- return i;
818- }
819- }
820-
821- return -1;
822+
823+ connect( child.data(), SIGNAL( MenuInvalid() ), this, SIGNAL( MenuInvalid() ) );
824+
825+ // emit signal informing subscribers that this child has added all of its menu items
826+ emit MenuItemsChanged( child.data(), 0, 0, child->m_size );
827 }
828
829 QAction* QtGMenuModel::CreateAction( int index )
830 {
831+ QAction* action = new QAction( m_menu.data() );
832+
833 // action label
834- QAction* action = new QAction( this );
835-
836 gchar* label = NULL;
837- if( g_menu_model_get_item_attribute( m_model, index, G_MENU_ATTRIBUTE_LABEL, "s", &label ) ) {
838+ if( g_menu_model_get_item_attribute( m_model.data(), index, G_MENU_ATTRIBUTE_LABEL, "s", &label ) ) {
839 QString qlabel = QString::fromUtf8( label );
840- qlabel.replace( '_', '&' );
841+ qlabel.replace( SINGLE_UNDERSCORE, "&" );
842 g_free( label );
843
844 action->setText( qlabel );
845@@ -361,7 +400,7 @@
846
847 // action name
848 gchar* action_name = NULL;
849- if( g_menu_model_get_item_attribute( m_model, index,
850+ if( g_menu_model_get_item_attribute( m_model.data(), index,
851 G_MENU_ATTRIBUTE_ACTION, "s", &action_name ) )
852 {
853 QString qaction_name = QString::fromUtf8( action_name );
854@@ -378,7 +417,7 @@
855 action->setProperty( c_property_menuPath, m_menu_path );
856
857 // action icon
858- GVariant* icon = g_menu_model_get_item_attribute_value( m_model, index, G_MENU_ATTRIBUTE_ICON,
859+ GVariant* icon = g_menu_model_get_item_attribute_value( m_model.data(), index, G_MENU_ATTRIBUTE_ICON,
860 G_VARIANT_TYPE_VARIANT );
861
862 if( icon )
863@@ -388,7 +427,7 @@
864
865 // action shortcut
866 gchar* shortcut = NULL;
867- if( g_menu_model_get_item_attribute( m_model, index, "accel", "s", &shortcut ) )
868+ if( g_menu_model_get_item_attribute( m_model.data(), index, "accel", "s", &shortcut ) )
869 {
870 QString qshortcut = QString::fromUtf8( shortcut );
871 g_free( shortcut );
872@@ -398,7 +437,7 @@
873
874 // action shortcut
875 gchar* toolbar_item = NULL;
876- if( g_menu_model_get_item_attribute( m_model, index, c_property_hud_toolbar_item, "s", &toolbar_item ) )
877+ if( g_menu_model_get_item_attribute( m_model.data(), index, c_property_hud_toolbar_item, "s", &toolbar_item ) )
878 {
879 QString qtoolbar_item = QString::fromUtf8( toolbar_item );
880 g_free( toolbar_item );
881@@ -408,7 +447,7 @@
882
883 // action keywords
884 gchar* keywords = NULL;
885- if( g_menu_model_get_item_attribute( m_model, index, c_property_keywords, "s", &keywords ) )
886+ if( g_menu_model_get_item_attribute( m_model.data(), index, c_property_keywords, "s", &keywords ) )
887 {
888 QVariant qkeywords = QString::fromUtf8( keywords );
889 g_free( keywords );
890@@ -458,14 +497,13 @@
891
892 if( action->isSeparator() )
893 {
894- QtGMenuModel* child = Child( i );
895+ QSharedPointer<QtGMenuModel> child = Child( i );
896 if( !child || child->Type() != LinkType::Section )
897 {
898 continue;
899 }
900- QMenu* section = child->m_ext_menu;
901
902- for( QAction* sub_action : section->actions() )
903+ for( QAction* sub_action : child->m_ext_menu->actions() )
904 {
905 m_ext_menu->addAction( sub_action );
906 }
907@@ -485,6 +523,12 @@
908 m_ext_menu->removeAction( last_action );
909 }
910 }
911+
912+ // if this is a section within a parent menu, we need to update the parent menu as well
913+ if( m_link_type == LinkType::Section && m_parent )
914+ {
915+ m_parent->UpdateExtQMenu();
916+ }
917 }
918
919 void QtGMenuModel::ActionAdded( const QString& name, QAction* action )
920@@ -494,17 +538,205 @@
921 {
922 m_parent->ActionAdded( name, action );
923 }
924-
925- m_actions[name] = action;
926+ else
927+ {
928+ // check if the action name is already in our map
929+ if( m_actions.find( name ) != m_actions.end() )
930+ {
931+ // add the QAction pointer to the list of actions under this name
932+ m_actions[name].push_back( action );
933+ }
934+ else
935+ {
936+ // otherwise insert the new action into the map
937+ m_actions.insert( std::make_pair( name, std::vector< QAction* >{ action } ) );
938+ }
939+ }
940 }
941
942-void QtGMenuModel::ActionRemoved( const QString& name )
943+void QtGMenuModel::ActionRemoved( const QString& name, QAction* action )
944 {
945 // remove action from top menu's m_actions
946 if( m_parent )
947 {
948- m_parent->ActionRemoved( name );
949- }
950-
951- m_actions.erase( name );
952+ m_parent->ActionRemoved( name, action );
953+ }
954+ else
955+ {
956+ // check if this action is actually in our map
957+ if( m_actions.find( name ) != m_actions.end() )
958+ {
959+ // remove the QAction pointer from the list of actions under this name
960+ auto& actionList = m_actions[name];
961+ auto actionIt = std::find( actionList.begin(), actionList.end(), action );
962+
963+ if( actionIt != actionList.end())
964+ {
965+ actionList.erase( actionIt );
966+ }
967+
968+ // if there are no more references to this action, remove it from the map
969+ if( actionList.size() == 0 )
970+ {
971+ m_actions.erase( name );
972+ }
973+ }
974+ }
975+}
976+
977+static void write_pair(QIODevice& device, const QString& key, const QString& value, bool last = false)
978+{
979+ device.write(key.toUtf8());
980+ device.write("", 1);
981+ device.write(value.toUtf8());
982+ if( !last )
983+ {
984+ device.write("", 1);
985+ }
986+
987+ if( !value.isEmpty())
988+ {
989+ qWarning() << key << " =" << value;
990+ }
991+}
992+
993+void QtGMenuModel::ReportRecoverableError(const int index, const int added, const int removed)
994+{
995+ if( m_error_reported )
996+ {
997+ return;
998+ }
999+
1000+ // gmenumodel properties
1001+ int gmenu_item_count = 0;
1002+ QString gmenu_action_names;
1003+
1004+ gmenu_item_count = g_menu_model_get_n_items( m_model.data() );
1005+
1006+ qWarning() << "Illegal arguments when updating GMenuModel: position ="
1007+ << index << ", added =" << added << ", removed =" << removed
1008+ << ", size =" << gmenu_item_count;
1009+
1010+ for( int i = 0; i < gmenu_item_count; ++i )
1011+ {
1012+ gchar* action_name = NULL;
1013+ if( g_menu_model_get_item_attribute( m_model.data(), i,
1014+ G_MENU_ATTRIBUTE_ACTION, "s", &action_name ) )
1015+ {
1016+ gmenu_action_names += action_name;
1017+ gmenu_action_names += ";";
1018+ g_free( action_name );
1019+ }
1020+ }
1021+
1022+ // parent model properties
1023+ QString parent_menu_label;
1024+ QString parent_menu_name;
1025+ QString parent_action_names;
1026+ QString parent_link_type;
1027+
1028+ if( m_parent )
1029+ {
1030+ parent_menu_label = m_parent->m_menu->menuAction()->text();
1031+ parent_menu_name = m_parent->m_menu->menuAction()->property( c_property_actionName ).toString();
1032+
1033+ for( QAction* action : m_parent->m_menu->actions() )
1034+ {
1035+ parent_action_names += action->property( c_property_actionName ).toString() + ";";
1036+ }
1037+
1038+ switch( m_parent->m_link_type )
1039+ {
1040+ case LinkType::Root:
1041+ parent_link_type = "root";
1042+ break;
1043+ case LinkType::Section:
1044+ parent_link_type = "section";
1045+ break;
1046+ case LinkType::SubMenu:
1047+ parent_link_type = "sub menu";
1048+ break;
1049+ }
1050+ }
1051+
1052+ // local model properties
1053+ QString menu_label;
1054+ QString menu_name;
1055+ QString action_names;
1056+ QString link_type;
1057+ QString action_paths;
1058+
1059+ menu_label = m_menu->menuAction()->text();
1060+ menu_name = m_menu->menuAction()->property( c_property_actionName ).toString();
1061+ for( QAction* action : m_menu->actions() )
1062+ {
1063+ action_names += action->property( c_property_actionName ).toString() + ";";
1064+ }
1065+
1066+ switch( m_link_type )
1067+ {
1068+ case LinkType::Root:
1069+ link_type = "root";
1070+ break;
1071+ case LinkType::Section:
1072+ link_type = "section";
1073+ break;
1074+ case LinkType::SubMenu:
1075+ link_type = "sub menu";
1076+ break;
1077+ }
1078+
1079+ for( auto const& action : m_action_paths )
1080+ {
1081+ action_paths += action.path() + ";";
1082+ }
1083+
1084+ uint sender_pid = QDBusConnection::sessionBus().interface()->servicePid(
1085+ m_bus_name);
1086+ if( sender_pid == 0 ) {
1087+ qWarning() << "Failed to read PID, cannot report error";
1088+ return;
1089+ }
1090+
1091+ QProcess recoverable;
1092+ recoverable.setProcessChannelMode(QProcess::ForwardedChannels);
1093+ recoverable.start("/usr/share/apport/recoverable_problem",
1094+ QStringList() << "-p" << QString::number(sender_pid));
1095+ if (recoverable.waitForStarted())
1096+ {
1097+ write_pair(recoverable, "DuplicateSignature", "GMenuModelItemsChangedInvalidIndex");
1098+ write_pair(recoverable, "BusName", m_bus_name);
1099+ write_pair(recoverable, "Position", QString::number(index));
1100+ write_pair(recoverable, "Added", QString::number(added));
1101+ write_pair(recoverable, "Removed", QString::number(removed));
1102+ write_pair(recoverable, "ItemCount", QString::number(gmenu_item_count));
1103+ write_pair(recoverable, "ActionNames", gmenu_action_names);
1104+
1105+ if ( m_parent )
1106+ {
1107+ write_pair(recoverable, "ParentMenuLabel", parent_menu_label);
1108+ write_pair(recoverable, "ParentMenuName", parent_menu_name);
1109+ write_pair(recoverable, "ParentActionNames", parent_action_names);
1110+ write_pair(recoverable, "ParentLinkType", parent_link_type);
1111+ }
1112+
1113+ write_pair(recoverable, "MenuLabel", menu_label);
1114+ write_pair(recoverable, "MenuName", menu_name);
1115+ write_pair(recoverable, "ActionNames", action_names);
1116+ write_pair(recoverable, "LinkType", link_type);
1117+
1118+ write_pair(recoverable, "MenuPath", m_menu_path);
1119+ write_pair(recoverable, "ActionPaths", action_paths, true);
1120+
1121+ recoverable.closeWriteChannel();
1122+ recoverable.waitForFinished();
1123+
1124+ m_error_reported = true;
1125+ }
1126+ else
1127+ {
1128+ qWarning() << "Failed to report recoverable error";
1129+ }
1130+
1131+ emit MenuInvalid();
1132 }
1133
1134=== modified file 'libqtgmenu/internal/QtGMenuModel.h'
1135--- libqtgmenu/internal/QtGMenuModel.h 2014-03-19 23:26:48 +0000
1136+++ libqtgmenu/internal/QtGMenuModel.h 2014-06-04 13:54:33 +0000
1137@@ -43,17 +43,14 @@
1138 Root, Section, SubMenu
1139 };
1140
1141- explicit QtGMenuModel( GMenuModel* model );
1142- QtGMenuModel( GMenuModel* model, const QString& bus_name, const QString& menu_path, const QMap<QString, QDBusObjectPath>& action_paths );
1143+ QtGMenuModel( QSharedPointer<GDBusConnection> connection, const QString& bus_name, const QString& menu_path, const QMap<QString, QDBusObjectPath>& action_paths );
1144 virtual ~QtGMenuModel();
1145
1146- GMenuModel* Model() const;
1147+ QSharedPointer<GMenuModel> Model() const;
1148 LinkType Type() const;
1149
1150- int Size() const;
1151-
1152 QtGMenuModel* Parent() const;
1153- QtGMenuModel* Child( int index ) const;
1154+ QSharedPointer<QtGMenuModel> Child( int index ) const;
1155
1156 std::shared_ptr< QMenu > GetQMenu();
1157
1158@@ -68,6 +65,7 @@
1159 Q_SIGNALS:
1160 void MenuItemsChanged( QtGMenuModel* model, int index, int removed, int added );
1161 void ActionTriggered( QString action_name, bool checked );
1162+ void MenuInvalid();
1163
1164 public Q_SLOTS:
1165 void ActionEnabled( QString action_name, bool enabled );
1166@@ -77,20 +75,19 @@
1167 void ActionTriggered( bool );
1168
1169 private:
1170- QtGMenuModel( GMenuModel* model, LinkType link_type, QtGMenuModel* parent, int index );
1171+ QtGMenuModel( QSharedPointer<GMenuModel> model, LinkType link_type, QtGMenuModel* parent, int index );
1172
1173- static QtGMenuModel* CreateChild( QtGMenuModel* parent, GMenuModel* model, int index );
1174+ static QSharedPointer<QtGMenuModel> CreateChild( QtGMenuModel* parent_qtgmenu, QSharedPointer<GMenuModel> parent_gmenu, int child_index );
1175
1176 static void MenuItemsChangedCallback( GMenuModel* model, gint index, gint removed, gint added,
1177 gpointer user_data );
1178
1179- void ChangeMenuItems( int index, int added, int removed );
1180+ void ChangeMenuItems( const int index, const int added, const int removed );
1181
1182 void ConnectCallback();
1183 void DisconnectCallback();
1184
1185- void InsertChild( QtGMenuModel* child, int index );
1186- int ChildIndex( QtGMenuModel* child );
1187+ void InsertChild( QSharedPointer<QtGMenuModel> child, int index );
1188
1189 QAction* CreateAction( int index );
1190
1191@@ -98,26 +95,32 @@
1192 void UpdateExtQMenu();
1193
1194 void ActionAdded( const QString& name, QAction* action );
1195- void ActionRemoved( const QString& name );
1196+ void ActionRemoved( const QString& name, QAction* action );
1197+
1198+ void ReportRecoverableError(const int index, const int added, const int removed);
1199
1200 private:
1201 QtGMenuModel* m_parent = nullptr;
1202- QMap< int, QtGMenuModel* > m_children;
1203+ QMap< int, QSharedPointer<QtGMenuModel>> m_children;
1204
1205- GMenuModel* m_model = nullptr;
1206+ QSharedPointer<GMenuModel> m_model;
1207 gulong m_items_changed_handler = 0;
1208
1209 LinkType m_link_type;
1210 int m_size = 0;
1211
1212- QMenu* m_menu = new QMenu();
1213- QMenu* m_ext_menu = new QMenu();
1214+ QScopedPointer<QMenu> m_menu;
1215+ QScopedPointer<QMenu> m_ext_menu;
1216
1217+ QSharedPointer<GDBusConnection> m_connection;
1218 QString m_bus_name;
1219 QString m_menu_path;
1220 QMap<QString, QDBusObjectPath> m_action_paths;
1221
1222- std::map< QString, QAction* > m_actions;
1223+ // a map of QActions indexed by their name and stored with a reference count
1224+ std::map< QString, std::vector< QAction* > > m_actions;
1225+
1226+ bool m_error_reported = false;
1227 };
1228
1229 } // namespace qtgmenu
1230
1231=== modified file 'service/DBusMenuCollector.cpp'
1232--- service/DBusMenuCollector.cpp 2014-04-01 13:13:04 +0000
1233+++ service/DBusMenuCollector.cpp 2014-06-04 13:54:33 +0000
1234@@ -82,25 +82,18 @@
1235 return !m_menuImporter.isNull();
1236 }
1237
1238-inline uint qHash(const QStringList &key, uint seed) {
1239- uint hash(0);
1240- for (const QString &s : key) {
1241- hash ^= qHash(s, seed);
1242- }
1243- return hash;
1244-}
1245-
1246 void DBusMenuCollector::openMenu(QMenu *menu, unsigned int &limit) {
1247+ --limit;
1248+ if (limit == 0) {
1249+ QString error = "Hit DBusMenu safety valve opening menu at " + m_service
1250+ + " " + m_path.path();
1251+ throw std::logic_error(error.toStdString());
1252+ }
1253+
1254 if (!menu) {
1255 return;
1256 }
1257
1258- if (limit == 0) {
1259- QString error = "Hit DBusMenu safety valve for menu at " + m_service
1260- + " " + m_path.path();
1261- throw std::logic_error(error.toStdString());
1262- }
1263-
1264 menu->aboutToShow();
1265
1266 for (int i(0); m_menuImporter && i < menu->actions().size(); ++i) {
1267@@ -114,15 +107,15 @@
1268
1269 QMenu *child(action->menu());
1270 if (child) {
1271- --limit;
1272 openMenu(child, limit);
1273 }
1274 }
1275 }
1276
1277 void DBusMenuCollector::hideMenu(QMenu *menu, unsigned int &limit) {
1278+ --limit;
1279 if (limit == 0) {
1280- QString error = "Hit DBusMenu safety valve for menu at " + m_service
1281+ QString error = "Hit DBusMenu safety valve closing menu at " + m_service
1282 + " " + m_path.path();
1283 throw std::logic_error(error.toStdString());
1284 }
1285@@ -131,7 +124,6 @@
1286 QAction *action = menu->actions().at(i);
1287 QMenu *child(action->menu());
1288 if (child) {
1289- --limit;
1290 hideMenu(child, limit);
1291 }
1292 }
1293
1294=== modified file 'service/HudServiceImpl.cpp'
1295--- service/HudServiceImpl.cpp 2014-02-18 13:35:12 +0000
1296+++ service/HudServiceImpl.cpp 2014-06-04 13:54:33 +0000
1297@@ -104,13 +104,24 @@
1298 QList<Suggestion> &suggestions, QDBusVariant &querykey) {
1299 QString sender(messageSender());
1300
1301- Query::Ptr query(m_legacyQueries[sender]);
1302+ QPair<Query::Ptr, QSharedPointer<QTimer>> entry(m_legacyQueries[sender]);
1303+ Query::Ptr query(entry.first);
1304+ QSharedPointer<QTimer> legacyTimeout(entry.second);
1305 if (query.isNull()) {
1306 query = createQuery(queryString, sender,
1307 Query::EmptyBehaviour::NO_SUGGESTIONS);
1308- m_legacyQueries[sender] = query;
1309+
1310+ legacyTimeout.reset(new QTimer());
1311+ legacyTimeout->setInterval(2000);
1312+ legacyTimeout->setSingleShot(true);
1313+ connect(legacyTimeout.data(), SIGNAL(timeout()), this,
1314+ SLOT(legacyTimeout()));
1315+
1316+ m_legacyQueries[sender] = qMakePair(query, legacyTimeout);
1317 } else {
1318 query->UpdateQuery(queryString);
1319+ legacyTimeout->stop();
1320+ legacyTimeout->setProperty("sender", QVariant());
1321 }
1322
1323 // The legacy API only allows you to search the current application
1324@@ -142,8 +153,13 @@
1325 Q_UNUSED(timestamp);
1326 QString sender(messageSender());
1327
1328- Query::Ptr query(m_legacyQueries.take(sender));
1329+ QPair<Query::Ptr, QSharedPointer<QTimer>> entry(m_legacyQueries.take(sender));
1330+ Query::Ptr query(entry.first);
1331+ QSharedPointer<QTimer> legacyTimeout(entry.second);
1332 if (!query.isNull()) {
1333+ legacyTimeout->stop();
1334+ legacyTimeout->setProperty("sender", QVariant());
1335+
1336 query->ExecuteCommand(itemKey, timestamp);
1337 closeQuery(query->path());
1338 }
1339@@ -156,8 +172,32 @@
1340 // We don't actually close legacy queries, or we'd be constructing
1341 // and destructing them during the search, due to the way that
1342 // Unity7 uses the API.
1343- Query::Ptr query(m_legacyQueries[sender]);
1344+ QPair<Query::Ptr, QSharedPointer<QTimer>> entry(m_legacyQueries[sender]);
1345+ Query::Ptr query(entry.first);
1346+ QSharedPointer<QTimer> legacyTimeout(entry.second);
1347 if (!query.isNull()) {
1348 query->UpdateQuery(QString());
1349- }
1350+ legacyTimeout->start();
1351+ legacyTimeout->setProperty("sender", sender);
1352+ }
1353+}
1354+
1355+void HudServiceImpl::legacyTimeout() {
1356+ QObject *timer(sender());
1357+ if (!timer) {
1358+ return;
1359+ }
1360+
1361+ QVariant from(timer->property("sender"));
1362+ if (from.isNull()) {
1363+ return;
1364+ }
1365+
1366+ QString sender(from.toString());
1367+ QPair<Query::Ptr, QSharedPointer<QTimer>> entry(m_legacyQueries.take(sender));
1368+ Query::Ptr query(entry.first);
1369+ if (query) {
1370+ closeQuery(query->path());
1371+ }
1372+
1373 }
1374
1375=== modified file 'service/HudServiceImpl.h'
1376--- service/HudServiceImpl.h 2014-02-18 13:35:12 +0000
1377+++ service/HudServiceImpl.h 2014-06-04 13:54:33 +0000
1378@@ -28,6 +28,7 @@
1379 #include <QDBusVariant>
1380 #include <QMap>
1381 #include <QScopedPointer>
1382+#include <QTimer>
1383
1384 class HudAdaptor;
1385
1386@@ -73,6 +74,9 @@
1387
1388 void CloseQuery(const QDBusVariant &querykey);
1389
1390+protected Q_SLOTS:
1391+ void legacyTimeout();
1392+
1393 protected:
1394 Query::Ptr createQuery(const QString &query, const QString &service,
1395 Query::EmptyBehaviour emptyBehaviour);
1396@@ -85,7 +89,7 @@
1397
1398 QMap<QDBusObjectPath, Query::Ptr> m_queries;
1399
1400- QMap<QString, Query::Ptr> m_legacyQueries;
1401+ QMap<QString, QPair<Query::Ptr, QSharedPointer<QTimer>>> m_legacyQueries;
1402
1403 QSharedPointer<ApplicationList> m_applicationList;
1404
1405
1406=== modified file 'tests/unit/qtgmenu/TestQtGMenu.cpp'
1407--- tests/unit/qtgmenu/TestQtGMenu.cpp 2014-03-20 06:46:57 +0000
1408+++ tests/unit/qtgmenu/TestQtGMenu.cpp 2014-06-04 13:54:33 +0000
1409@@ -71,28 +71,28 @@
1410
1411 int GetGMenuSize()
1412 {
1413- GMenuModel* menu = m_importer.GetGMenuModel();
1414+ QSharedPointer<GMenuModel> menu = m_importer.GetGMenuModel();
1415
1416 if( !menu )
1417 {
1418 return 0;
1419 }
1420
1421- gint item_count = g_menu_model_get_n_items( G_MENU_MODEL( menu ) );
1422+ gint item_count = g_menu_model_get_n_items( G_MENU_MODEL( menu.data() ) );
1423
1424 return item_count;
1425 }
1426
1427 int GetGActionCount()
1428 {
1429- GActionGroup* actions = m_importer.GetGActionGroup();
1430+ QSharedPointer<GActionGroup> actions = m_importer.GetGActionGroup();
1431
1432 if( !actions )
1433 {
1434 return 0;
1435 }
1436
1437- gchar** actions_list = g_action_group_list_actions( actions );
1438+ gchar** actions_list = g_action_group_list_actions( actions.data() );
1439
1440 int action_count = 0;
1441 while( actions_list[action_count] != nullptr )

Subscribers

People subscribed via source and target branches