From 543251c2c65663aff367d4ddba719e89de7e03ca Mon Sep 17 00:00:00 2001 From: Jens Granseuer Date: Wed, 11 Nov 2009 19:37:04 +0100 Subject: [PATCH] [typing-break] Reset timer after suspend This patch does 2 things: 1) Defines a DrwTimer that we use instead of GTimer. This is just a thin wrapper around g_get_current_time, and it means we can accurately track typing/idle periods based on real-world wall-clock time, which GTimer is apparently not intended to do. 2) The typing monitor has some complicated state handling where it transitions between an IDLE state and a TYPING state. This transition is based on running a callback once per second, and checking whether any keystrokes have been recorded since the last time the callback was called. The actual idle *time* is tracked separately, independently of these two states, but only when we are in the IDLE *state* was the idle *time* checked to see if we should reset the break timer. This leads to a race condition -- if we suspend while in the TYPING state, then eventually we will wake up, notice that no key press has happened in the last second, *reset the idle timer*, transition to the IDLE state, and then check the amount of time on the idle timer. We will thus never notice the amount of time that the computer was suspended. I considered making the IDLE/TYPING transition code smarter, but it turns out the desired behavior for the two states is entirely identical anyway, so rather than adding more complexity to this pointless code, I just diked it out and replaced them both by a single state called RUNNING. Closes bug #430797. --- typing-break/Makefile.am | 4 +- typing-break/drw-break-window.c | 9 +-- typing-break/drw-timer.c | 52 ++++++++++++++++ typing-break/drw-timer.h | 42 +++++++++++++ typing-break/drwright.c | 101 ++++++++++---------------------- 5 files changed, 134 insertions(+), 74 deletions(-) create mode 100644 typing-break/drw-timer.c create mode 100644 typing-break/drw-timer.h diff --git a/typing-break/Makefile.am b/typing-break/Makefile.am index 31b090782..f1b89bf65 100644 --- a/typing-break/Makefile.am +++ b/typing-break/Makefile.am @@ -11,7 +11,9 @@ gnome_typing_monitor_SOURCES = \ drw-utils.c \ drw-utils.h \ drw-selection.c \ - drw-selection.h + drw-selection.h \ + drw-timer.c \ + drw-timer.h gnome_typing_monitor_CPPFLAGS = \ -DGNOMELOCALEDIR="\"$(datadir)/locale\"" \ diff --git a/typing-break/drw-break-window.c b/typing-break/drw-break-window.c index 813c5b941..8edf8e04a 100644 --- a/typing-break/drw-break-window.c +++ b/typing-break/drw-break-window.c @@ -35,6 +35,7 @@ #include "drwright.h" #include "drw-utils.h" #include "drw-break-window.h" +#include "drw-timer.h" struct _DrwBreakWindowPrivate { GtkWidget *clock_label; @@ -44,7 +45,7 @@ struct _DrwBreakWindowPrivate { GtkWidget *postpone_entry; GtkWidget *postpone_button; - GTimer *timer; + DrwTimer *timer; gint break_time; gchar *break_text; @@ -270,7 +271,7 @@ drw_break_window_init (DrwBreakWindow *window) gtk_window_stick (GTK_WINDOW (window)); - priv->timer = g_timer_new (); + priv->timer = drw_timer_new (); /* Make sure we have a valid time label from the start. */ clock_timeout_cb (window); @@ -319,7 +320,7 @@ drw_break_window_dispose (GObject *object) priv = window->priv; if (priv->timer) { - g_timer_destroy (priv->timer); + drw_timer_destroy (priv->timer); priv->timer = NULL; } @@ -382,7 +383,7 @@ clock_timeout_cb (DrwBreakWindow *window) priv = window->priv; - seconds = 1 + priv->break_time - g_timer_elapsed (priv->timer, NULL); + seconds = 1 + priv->break_time - drw_timer_elapsed (priv->timer); seconds = MAX (0, seconds); if (seconds == 0) { diff --git a/typing-break/drw-timer.c b/typing-break/drw-timer.c new file mode 100644 index 000000000..d108034c6 --- /dev/null +++ b/typing-break/drw-timer.c @@ -0,0 +1,52 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */ +/* + * Copyright (C) 2009 Nathaniel Smith + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include +#include "drw-timer.h" + +struct _DrwTimer +{ + GTimeVal start_time; +}; + +DrwTimer * drw_timer_new () +{ + DrwTimer * timer = g_new0 (DrwTimer, 1); + drw_timer_start (timer); + return timer; +} + +void drw_timer_start (DrwTimer *timer) +{ + g_get_current_time (&timer->start_time); +} + +double drw_timer_elapsed (DrwTimer *timer) +{ + GTimeVal now; + g_get_current_time (&now); + return now.tv_sec - timer->start_time.tv_sec; +} + +void drw_timer_destroy (DrwTimer *timer) +{ + g_free (timer); +} + diff --git a/typing-break/drw-timer.h b/typing-break/drw-timer.h new file mode 100644 index 000000000..1a259c5c7 --- /dev/null +++ b/typing-break/drw-timer.h @@ -0,0 +1,42 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */ +/* + * Copyright (C) 2009 Nathaniel Smith + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#ifndef __DRW_TIMER_H__ +#define __DRW_TIMER_H__ + +/* + * This file defines a timer interface similar to GTimer, but defined in real + * wall-clock time. A GTimer may stop counting while the computer is suspended + * or the process is stopped: + * https://bugzilla.gnome.org/show_bug.cgi?id=552994 + * but a DrwTimer keeps counting regardless. + * + * Currently this only provides second resolution as compared to GTimer's + * microsecond resolution, but a typing break program doesn't really need + * microsecond resolution anyway. + */ + +typedef struct _DrwTimer DrwTimer; +DrwTimer * drw_timer_new (); +void drw_timer_start (DrwTimer *timer); +double drw_timer_elapsed (DrwTimer *timer); +void drw_timer_destroy (DrwTimer *timer); + +#endif /* __DRW_TIMER_H__ */ diff --git a/typing-break/drwright.c b/typing-break/drwright.c index 3775dbdba..9c7c2a0d3 100644 --- a/typing-break/drwright.c +++ b/typing-break/drwright.c @@ -21,7 +21,6 @@ */ #include -#include #include #include #include @@ -34,6 +33,7 @@ #include "drw-break-window.h" #include "drw-monitor.h" #include "drw-utils.h" +#include "drw-timer.h" #define BLINK_TIMEOUT 200 #define BLINK_TIMEOUT_MIN 120 @@ -44,10 +44,8 @@ typedef enum { STATE_START, - STATE_IDLE, - STATE_TYPE, - STATE_WARN_TYPE, - STATE_WARN_IDLE, + STATE_RUNNING, + STATE_WARN, STATE_BREAK_SETUP, STATE_BREAK, STATE_BREAK_DONE_SETUP, @@ -64,14 +62,12 @@ struct _DrWright { GtkUIManager *ui_manager; DrwState state; - GTimer *timer; - GTimer *idle_timer; + DrwTimer *timer; + DrwTimer *idle_timer; gint last_elapsed_time; gint save_last_time; - gboolean is_active; - /* Time settings. */ gint type_time; gint break_time; @@ -166,7 +162,7 @@ update_icon (DrWright *dr) break; default: - r = (float) (g_timer_elapsed (dr->timer, NULL) + dr->save_last_time) / + r = (float) (drw_timer_elapsed (dr->timer) + dr->save_last_time) / (float) dr->type_time; break; } @@ -174,8 +170,7 @@ update_icon (DrWright *dr) offset = CLAMP ((height - 0) * (1.0 - r), 1, height - 0); switch (dr->state) { - case STATE_WARN_TYPE: - case STATE_WARN_IDLE: + case STATE_WARN: pixbuf = dr->red_bar; set_pixbuf = FALSE; break; @@ -220,7 +215,7 @@ blink_timeout_cb (DrWright *dr) gfloat r; gint timeout; - r = (dr->type_time - g_timer_elapsed (dr->timer, NULL) - dr->save_last_time) / dr->warn_time; + r = (dr->type_time - drw_timer_elapsed (dr->timer) - dr->save_last_time) / dr->warn_time; timeout = BLINK_TIMEOUT + BLINK_TIMEOUT_FACTOR * r; if (timeout < BLINK_TIMEOUT_MIN) { @@ -301,11 +296,11 @@ maybe_change_state (DrWright *dr) gint elapsed_idle_time; if (debug) { - g_timer_reset (dr->idle_timer); + drw_timer_start (dr->idle_timer); } - elapsed_time = g_timer_elapsed (dr->timer, NULL) + dr->save_last_time; - elapsed_idle_time = g_timer_elapsed (dr->idle_timer, NULL); + elapsed_time = drw_timer_elapsed (dr->timer) + dr->save_last_time; + elapsed_idle_time = drw_timer_elapsed (dr->idle_timer); if (elapsed_time > dr->last_elapsed_time + dr->warn_time) { /* If the timeout is delayed by the amount of warning time, then @@ -327,56 +322,28 @@ maybe_change_state (DrWright *dr) dr->save_last_time = 0; - g_timer_start (dr->timer); - g_timer_start (dr->idle_timer); + drw_timer_start (dr->timer); + drw_timer_start (dr->idle_timer); if (dr->enabled) { - dr->state = STATE_IDLE; + dr->state = STATE_RUNNING; } update_tooltip (dr); stop_blinking (dr); break; - case STATE_IDLE: + case STATE_RUNNING: + case STATE_WARN: if (elapsed_idle_time >= dr->break_time) { dr->state = STATE_BREAK_DONE_SETUP; - } else if (dr->is_active) { - dr->state = STATE_TYPE; - } - break; - - case STATE_TYPE: - if (elapsed_time >= dr->type_time - dr->warn_time) { - dr->state = STATE_WARN_TYPE; - - start_blinking (dr); } else if (elapsed_time >= dr->type_time) { dr->state = STATE_BREAK_SETUP; + } else if (dr->state != STATE_WARN + && elapsed_time >= dr->type_time - dr->warn_time) { + dr->state = STATE_WARN; + start_blinking (dr); } - else if (!dr->is_active) { - dr->state = STATE_IDLE; - g_timer_start (dr->idle_timer); - } - break; - - case STATE_WARN_TYPE: - if (elapsed_time >= dr->type_time) { - dr->state = STATE_BREAK_SETUP; - } - else if (!dr->is_active) { - dr->state = STATE_WARN_IDLE; - } - break; - - case STATE_WARN_IDLE: - if (elapsed_idle_time >= dr->break_time) { - dr->state = STATE_BREAK_DONE_SETUP; - } - else if (dr->is_active) { - dr->state = STATE_WARN_TYPE; - } - break; case STATE_BREAK_SETUP: @@ -392,7 +359,7 @@ maybe_change_state (DrWright *dr) gtk_status_icon_set_from_pixbuf (dr->icon, dr->red_bar); - g_timer_start (dr->timer); + drw_timer_start (dr->timer); dr->break_window = drw_break_window_new (); @@ -438,17 +405,14 @@ maybe_change_state (DrWright *dr) break; case STATE_BREAK_DONE: - if (dr->is_active) { - dr->state = STATE_START; - if (dr->break_window) { - gtk_widget_destroy (dr->break_window); - dr->break_window = NULL; - } + dr->state = STATE_START; + if (dr->break_window) { + gtk_widget_destroy (dr->break_window); + dr->break_window = NULL; } break; } - dr->is_active = FALSE; dr->last_elapsed_time = elapsed_time; update_icon (dr); @@ -468,7 +432,7 @@ update_tooltip (DrWright *dr) return TRUE; } - elapsed_time = g_timer_elapsed (dr->timer, NULL); + elapsed_time = drw_timer_elapsed (dr->timer); min = floor (0.5 + (dr->type_time - elapsed_time - dr->save_last_time) / 60.0); @@ -492,8 +456,7 @@ static void activity_detected_cb (DrwMonitor *monitor, DrWright *dr) { - dr->is_active = TRUE; - g_timer_start (dr->idle_timer); + drw_timer_start (dr->idle_timer); } static void @@ -634,10 +597,10 @@ break_window_postpone_cb (GtkWidget *window, gtk_widget_destroy (dr->break_window); - dr->state = STATE_TYPE; + dr->state = STATE_RUNNING; dr->break_window = NULL; - elapsed_time = g_timer_elapsed (dr->timer, NULL); + elapsed_time = drw_timer_elapsed (dr->timer); if (elapsed_time + dr->save_last_time >= dr->type_time) { /* Typing time has expired, but break was postponed. @@ -648,7 +611,7 @@ break_window_postpone_cb (GtkWidget *window, dr->save_last_time = dr->type_time - MAX (dr->warn_time, (gint) postpone_time); } - g_timer_start (dr->timer); + drw_timer_start (dr->timer); maybe_change_state (dr); update_icon (dr); update_tooltip (dr); @@ -783,8 +746,8 @@ drwright_new (void) item = gtk_ui_manager_get_widget (dr->ui_manager, "/Pop/TakeABreak"); gtk_widget_set_sensitive (item, dr->enabled); - dr->timer = g_timer_new (); - dr->idle_timer = g_timer_new (); + dr->timer = drw_timer_new (); + dr->idle_timer = drw_timer_new (); dr->state = STATE_START;