Merge lp:~ted/indicator-sound/lp1305902 into lp:indicator-sound/14.04

[Ted Gould]

Protect against icon serialization errors

The trace isn't entirely clear, but this seems like the most likely case of a null getting into build.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:442
http://jenkins.qa.ubuntu.com/job/indicator-sound-ci/129/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-trusty-amd64-ci/69
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-trusty-armhf-ci/68
        deb: http://jenkins.qa.ubuntu.com/job/indicator-sound-trusty-armhf-ci/68/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-sound-ci/129/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

Okay, it took me a little bit but I agree this probably is the cause; the icon is the only place where g_variant_new_variant() is called. All the other calls to g_builder_add_variant() in the generated code look safe.

This is kind of a convoluted way to fix this, the lines immediately above have steps to create the same icon to handle icon == null. What about something like

> GLib.Variant serialized = null;
> if (this._player.icon != null)
> serialized = this._player.icon.serialize();
> if (serialized == null) {
> var icon = new ThemedIcon.with_default_fallbacks ("application-default-icon");
> serialized = icon.serialize();
> }
> this.proxy.player_icon = serialized;

[lp:~ted/indicator-sound/lp1305902]

443. By Ted Gould on 2014-04-22

Unroll one more level of if statement

[Ted Gould (ted)]

Cyclomatic complexity reduced!

[Charles Kerr (charlesk)]

\o/

review: Approve

Unmerged revisions

443. By Ted Gould on 2014-04-22

Unroll one more level of if statement

442. By Ted Gould on 2014-04-14

Protect against icon serialization errors

=== modified file 'src/accounts-service-user.vala'
--- src/accounts-service-user.vala	2014-03-21 21:35:59 +0000
+++ src/accounts-service-user.vala	2014-04-22 15:17:30 +0000
@@ -58,13 +58,19 @@
 			} else {
 				this.proxy.timestamp = GLib.get_monotonic_time();
 				this.proxy.player_name = this._player.name;
-				if (this._player.icon == null) {
+
+				/* Serialize the icon if it exits, if it doesn't or errors then
+				   we need to use the application default icon */
+				GLib.Variant? icon_serialization = null;
+				if (this._player.icon != null)
+					icon_serialization = this._player.icon.serialize();
+				if (icon_serialization == null) {
 					var icon = new ThemedIcon.with_default_fallbacks ("application-default-icon");
-					this.proxy.player_icon = icon.serialize();
-				} else {
-					this.proxy.player_icon = this._player.icon.serialize();
+					icon_serialization = icon.serialize();
 				}
+				this.proxy.player_icon = icon_serialization;
 
+				/* Set state of the player */
 				this.proxy.running = this._player.is_running;
 				this.proxy.state = this._player.state;