Merge lp:~charlesk/indicator-datetime/lp-917236 into lp:indicator-datetime/12.10

[Charles Kerr]

When clock skew is detected, rebuild the date/time labels.

Add a timer to monitor for clock skew, and when it's detected, rebuild the datetime menuitems that need it.

Larsu, I'd also like a second opinion on whether or not the SystemIdleHintChanged callback is still needed, since the clock skew code should have the same results...

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:187
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./label=quantal/6/console

review: Approve (continuous-integration)

[lp:~charlesk/indicator-datetime/lp-917236]

188. By Charles Kerr on 2012-09-20

clocks don't skew very often, so let's reduce the frequency that we test for this.

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:188
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/7/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./label=quantal/7/console

review: Approve (continuous-integration)

[Lars Karlitski (larsu)]

Why are SKEW_CHECK_INTERVAL_SEC and SKEW_DIFF_THRESHOLD_SEC enums instead of #defines or const ints? I prefer having those kinds of definitions at the top of the file.

If we remove the SystemIdleHintChanged callback, the clock in the menu will be updated after a maximum of SKEW_CHECK_INTERVAL_SEC seconds, right? I don't like that, even if it is only the clock in the menu.

Why are the timers running when the menu is not open?

review: Needs Information

[Charles Kerr (charlesk)]

> I prefer having those kinds of definitions at the top of the file.

Agreed, r189.

> Why are SKEW_CHECK_INTERVAL_SEC and SKEW_DIFF_THRESHOLD_SEC enums instead of #defines or const ints?

const ints aren't legal when you define threshold in terms of interval. enums in general safer than preprocessor tokens. But, in terms of our codebase, you're right #defines are way more common. r190.

> Why are the timers running when the menu is not open?

To ensure that the indicator's label gets updated via the UpdateTime signal.

[lp:~charlesk/indicator-datetime/lp-917236]

189. By Charles Kerr on 2012-09-20

move declaration of SKEW_CHECK_INTERVAL_SEC and SKEW_DIFF_THRESHOLD_SEC to the top of the file.

190. By Charles Kerr on 2012-09-20

for SKEW_DIFF_THRESHOLD_SEC and SKEW_CHECK_INTERVAL_SEC, use #define instead of enum

[Charles Kerr (charlesk)]

(btw, could you also take a look at https://code.launchpad.net/~charlesk/ido/lp-964005/+merge/125409 and https://code.launchpad.net/~charlesk/dbusmenu/fix-json-tests/+merge/125378 so that I can do ido and dbusmenu releases?)

[lp:~charlesk/indicator-datetime/lp-917236]

191. By Charles Kerr on 2012-09-20

Fix r190 typo. Yet another reminder to drink my morning coffee /before/ pushing.

[Charles Kerr (charlesk)]

Moreover, without the label we wouldn't need the timer at all -- we could connect to the menu's about-to-show event.

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:190
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/8/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./label=quantal/8/console

review: Approve (continuous-integration)

[lp:~charlesk/indicator-datetime/lp-917236]

192. By Charles Kerr on 2012-09-20

copyediting: typo tweak

[Lars Karlitski (larsu)]

> > Why are the timers running when the menu is not open?
>
> To ensure that the indicator's label gets updated via the UpdateTime signal.

Oh, I thought that was handled in the plugin and we're only talking about the label on the top of the menu here. Sorry.

Thanks for the other fixes.

review: Approve

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:191
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/9/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./label=quantal/9/console

review: Approve (continuous-integration)

=== modified file 'src/datetime-service.c'
--- src/datetime-service.c	2012-09-12 21:59:02 +0000
+++ src/datetime-service.c	2012-09-20 11:51:18 +0000
@@ -52,6 +52,11 @@
 #include "settings-shared.h"
 #include "utils.h"
 
+/* how often to check for clock skew */
+#define SKEW_CHECK_INTERVAL_SEC 10
+
+#define SKEW_DIFF_THRESHOLD_SEC (SKEW_CHECK_INTERVAL_SEC + 5)
+
 #ifdef HAVE_CCPANEL
  #define SETTINGS_APP_INVOCATION "gnome-control-center indicator-datetime"
 #else
@@ -61,7 +66,7 @@
 static void geo_create_client (GeoclueMaster * master, GeoclueMasterClient * client, gchar * path, GError * error, gpointer user_data);
 static gboolean update_appointment_menu_items (gpointer user_data);
 static void update_location_menu_items (void);
-static void setup_timer (void);
+static void day_timer_reset (void);
 static void geo_client_invalid (GeoclueMasterClient * client, gpointer user_data);
 static gboolean get_greeter_mode (void);
 
@@ -355,7 +360,7 @@
 	g_date_time_unref (datetime);
 	g_free(utf8);
 
-	return FALSE;
+	return G_SOURCE_REMOVE;
 }
 
 /* Run a particular program based on an activation */
@@ -1106,14 +1111,26 @@
 	return;
 }
 
+static void
+on_clock_skew (void)
+{
+	/* tell the indicators to refresh */
+	if (IS_DATETIME_INTERFACE (dbus))
+		datetime_interface_update (DATETIME_INTERFACE(dbus));
+
+	/* update our day label */
+	update_datetime (NULL);
+	day_timer_reset();
+
+	return;
+}
+
 /* Run when the timezone file changes */
 static void
 timezone_changed (GFileMonitor * monitor, GFile * file, GFile * otherfile, GFileMonitorEvent event, gpointer user_data)
 {
 	update_current_timezone();
-	datetime_interface_update(DATETIME_INTERFACE(user_data));
-	update_datetime(NULL);
-	setup_timer();
+	on_clock_skew();
 	return;
 }
 
@@ -1133,42 +1150,58 @@
 }
 
 /* Source ID for the timer */
-static guint timer = 0;
+static guint day_timer = 0;
 
 /* Execute at a given time, update and setup a new
    timer to go again.  */
 static gboolean
-timer_func (gpointer user_data)
+day_timer_func (gpointer user_data)
 {
-	timer = 0;
+	day_timer = 0;
 	/* Reset up each time to reduce error */
-	setup_timer();
+	day_timer_reset();
 	update_datetime(NULL);
-	return FALSE;
+	return G_SOURCE_REMOVE;
 }
 
 /* Sets up the time to launch the timer to update the
    date in the datetime entry */
 static void
-setup_timer (void)
+day_timer_reset (void)
 {
-	if (timer != 0) {
-		g_source_remove(timer);
-		timer = 0;
+	if (day_timer != 0) {
+		g_source_remove(day_timer);
+		day_timer = 0;
 	}
 
 	time_t t;
 	t = time(NULL);
 	struct tm * ltime = localtime(&t);
 
-	timer = g_timeout_add_seconds(((23 - ltime->tm_hour) * 60 * 60) +
-	                              ((59 - ltime->tm_min) * 60) +
-	                              ((60 - ltime->tm_sec)) + 60 /* one minute past */,
-	                              timer_func, NULL);
+	day_timer = g_timeout_add_seconds(((23 - ltime->tm_hour) * 60 * 60) +
+	                                  ((59 - ltime->tm_min) * 60) +
+	                                  ((60 - ltime->tm_sec)) + 60 /* one minute past */,
+	                                  day_timer_func, NULL);
 
 	return;
 }
 
+static gboolean
+skew_check_timer_func (gpointer unused G_GNUC_UNUSED)
+{
+	static time_t prev_time = 0;
+	const time_t cur_time = time (NULL);
+	const double diff_sec = fabs (difftime (cur_time, prev_time));
+
+	if (prev_time && (diff_sec > SKEW_DIFF_THRESHOLD_SEC)) {
+		g_debug (G_STRLOC" clock skew detected (%.0f seconds)", diff_sec);
+		on_clock_skew ();
+	}
+
+	prev_time = cur_time;
+	return G_SOURCE_CONTINUE;
+}
+
 static void
 session_active_change_cb (GDBusProxy * proxy, gchar * sender_name, gchar * signal_name,
                           GVariant * parameters, gpointer user_data)
@@ -1178,9 +1211,7 @@
 		gboolean idle = FALSE;
 		g_variant_get(parameters, "(b)", &idle);
 		if (!idle) {
-			datetime_interface_update(DATETIME_INTERFACE(user_data));
-			update_datetime(NULL);
-			setup_timer();
+			on_clock_skew ();
 		}
 	}
 	return;
@@ -1427,8 +1458,13 @@
 	/* Setup timezone watch */
 	build_timezone(dbus);
 
-	/* Setup the timer */
-	setup_timer();
+	/* Set up the day timer */
+	day_timer_reset();
+
+	/* Set up the skew-check timer */
+	g_timeout_add_seconds (SKEW_CHECK_INTERVAL_SEC,
+	                       skew_check_timer_func,
+	                       NULL);
 
 	/* And watch for system resumes */
 	g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM,

=== modified file 'src/indicator-datetime.c'
--- src/indicator-datetime.c	2012-05-07 23:48:29 +0000
+++ src/indicator-datetime.c	2012-09-20 11:51:18 +0000
@@ -154,6 +154,7 @@
 
 static void indicator_datetime_class_init (IndicatorDatetimeClass *klass);
 static void indicator_datetime_init       (IndicatorDatetime *self);
+static void timezone_update_all_labels    (IndicatorDatetime *self);
 static void set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec);
 static void get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec);
 static void indicator_datetime_dispose    (GObject *object);
@@ -280,6 +281,10 @@
   	// Set the calendar to todays date
 	ido_calendar_menu_item_set_date (self->priv->ido_calendar, y, m-1, d);
 
+	/* Update in case date was changed outside of indicator-datetime */
+	update_label(self, NULL);
+	timezone_update_all_labels(self);
+
 	// Make sure the day-selected signal is sent so the menu updates - may duplicate
 	/*GVariant *variant = g_variant_new_uint32((guint)curtime);
 	guint timestamp = (guint)time(NULL);