Code review comment for lp:~3v1n0/unity/dbus-indicators-proxy

Overall I'm liking this a lot - mostly because of "(+234/-516)" :) I have some concerns though:

269 + RequestSyncAll();
270 +
271 + reconnect_timeout_id_ = g_timeout_add_seconds(1, [](gpointer data) -> gboolean {
272 + auto self = static_cast<DBusIndicators::Impl*>(data);
273 +
274 + if (!self->gproxy_.IsConnected())
275 + {
276 + self->RequestSyncAll();
277 + return TRUE;
278 + }
279 +
280 + self->reconnect_timeout_id_ = 0;
281 + return FALSE;
282 + }, this);

Are you sure you want to call RequestSyncAll also outside of the lambda expression? Plus you should be checking if reconnect_timeout_id == 0, before calling g_timeout_add...

146 + struct CallData
147 + {
148 + Impl* self;
149 + GVariant *parameters;
150 + };

I'm not really liking this, it's unclear who owns the variant (or rather why isn't it unreffed) - using our Variant wrapper should fix this. What's worse is that the CallData can be alive even after the Impl instance is destroyed which can lead to a crash.