Indicator Date and Time

include/datetime/dbus-shared.h (+42/-0)
include/datetime/timezone-timedated.h (+4/-4)
include/datetime/timezones-live.h (+2/-3)
src/main.cpp (+12/-2)
src/timezone-timedated.cpp (+139/-135)
src/timezones-live.cpp (+7/-4)
tests/test-timezone-timedated.cpp (+156/-118)

To merge this branch:

bzr merge lp://qastaging/~charlesk/indicator-datetime/always-get-initial-tzid-from-timedate1

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Renato Araujo Oliveira Filho (community)		2016-04-08	Approve on 2016-04-13
Antti Kaijanmäki (community)			Approve on 2016-04-13
PS Jenkins bot (community)	continuous-integration		Approve on 2016-04-12
Review via email: mp+291421@code.qastaging.launchpad.net

Commit message

Fix possible startup timing issues when watching for a timezone from timedate1

Description of the change

Possible fix to bug #1567940, fixes a couple of bugs in the code that got tzid changes from org.freedesktop.timedate1:

1. Watch for timedate1 to show up on the bus, and query it when it does, so that there's no possible timing issue between indicator-datetime's startup and timedate1's startup. (This is the possible fix)

2. Use G_BUS_NAME_WATCHER_FLAGS_AUTO_START to ensure timedate1 shows up on the bus for us

3. Get the initial value by querying timedate1's properties instead of reading from /etc/timezone

4. If 'Timezone' shows up in timedate1's invalidated properties signal, again query for the value instead of reading /etc/timezone

lp://qastaging/~charlesk/indicator-datetime/always-get-initial-tzid-from-timedate1 updated on 2016-04-08

444. By Charles Kerr on 2016-04-08: in tests/test-timezone-timedated, fix copy-paste error in comments
445. By Charles Kerr on 2016-04-08: in timezone-timedated, check for error!=nullptr before passing it to g_error_matches()

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-04-08:

PASSED: Continuous integration, rev:443
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/370/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-amd64-ci/60
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-armhf-ci/60
        deb: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-armhf-ci/60/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-04-08:

PASSED: Continuous integration, rev:445
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/371/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-amd64-ci/61
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-armhf-ci/61
        deb: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-armhf-ci/61/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Revision history for this message

Renato Araujo Oliveira Filho (renatofilho) wrote on 2016-04-12:

code looks good, some small inline code comments.

review: Approve

Revision history for this message

Renato Araujo Oliveira Filho (renatofilho) on 2016-04-12:

review: Needs Information

Revision history for this message

Antti Kaijanmäki (kaijanmaki) wrote on 2016-04-12:

code looks good, but the juggling with m_connection looks a bit brittle.

How about initializing m_connection in the constructor with g_bus_get_sync(), unreffing it in the destructor and using g_bus_watch_name_on_connection() for name watching?

review: Needs Information

lp://qastaging/~charlesk/indicator-datetime/always-get-initial-tzid-from-timedate1 updated on 2016-04-12

446. By Charles Kerr on 2016-04-12: in TimedatedTimezone, take a GDBusConnection argument in the ctor to simplify state management
447. By Charles Kerr on 2016-04-12: in LiveTimezones, pass the primary timezone to it on construction. We used to create it implicitly but can't do that anymore now that TimedatedTimezone takes its own ctor argument.
448. By Charles Kerr on 2016-04-12: sync tests to new ctor arguments for TimedatedTimezone and LiveTimezones

Revision history for this message

Charles Kerr (charlesk) wrote on 2016-04-12:

Thanks for the feedback. Since both of you commented on the clumsiness of m_connection and m_subscription's state management, I've cleaned it up in r446..448 by passing the GDBusConnection* in the ctor.

This way we can unconditionally watch & subscribe in the ctor, then unconditionally unsubscribe and unwatch in the dtor, no edge cases or having to check to see if the pointers exist yet. :)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-04-12:

PASSED: Continuous integration, rev:448
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/373/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-amd64-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-armhf-ci/63
        deb: http://jenkins.qa.ubuntu.com/job/indicator-datetime-wily-armhf-ci/63/artifact/work/output/*zip*/output.zip