Merge lp://qastaging/~charlesk/indicator-datetime/lp-1074999 into lp://qastaging/indicator-datetime/13.04

PASSED: Continuous integration, rev:200
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./build=pbuilder,distribution=quantal,flavor=amd64/22/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./build=pbuilder,distribution=quantal,flavor=i386/22/console

Nice cleanup!

`geo_master` doesn't need to be a global variable: it's only ever used in geo_start.

Why are you calling geo_stop unconditionally in on_use_geoclue_changed? I'd put it in the `else` below, in case the callback is called even though the value hasn't changed (which may happen according to the docs).

> `geo_master` doesn't need to be a global variable: it's only ever used in geo_start.

The issue is that geoclue_master_create_client_async() populates its private GeoclueMasterAsyncData struct with a pointer to that master without reffing it. That's why datetime-service keeps its own ref.

> Why are you calling geo_stop unconditionally in on_use_geoclue_changed? I'd put it in the `else` below, in case the callback is called even though the value hasn't changed (which may happen according to the docs).

It's to ensure that even if the callback triggered multiple calls to geo_start(), we would have a call to geo_stop() in between to guarantee that geo_master, geo_client, and geo_address were cleaned up.

> > `geo_master` doesn't need to be a global variable: it's only ever used in
> geo_start.
>
> The issue is that geoclue_master_create_client_async() populates its private
> GeoclueMasterAsyncData struct with a pointer to that master without reffing
> it. That's why datetime-service keeps its own ref.

That should be fixed in geoclue.

> > Why are you calling geo_stop unconditionally in on_use_geoclue_changed? I'd
> put it in the `else` below, in case the callback is called even though the
> value hasn't changed (which may happen according to the docs).
>
> It's to ensure that even if the callback triggered multiple calls to
> geo_start(), we would have a call to geo_stop() in between to guarantee that
> geo_master, geo_client, and geo_address were cleaned up.

Sounds reasonable.

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/indicator-datetime-autolanding/12/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-datetime-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/12/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-datetime-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/12/console

Misconfiguration foiled autolanding; re-approving to send through again.

Thanks Allan!