Closed Bug 357079 Opened 14 years ago Closed 13 years ago

Agenda tab doesn't update Today's date after hibernating Windows

Categories

(Calendar :: Lightning Only, defect)

Lightning 0.3
x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hrr, Assigned: martinschroeder)

References

Details

Attachments

(1 file, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7

When resuming Windows after it was hibernated the previous day with Firefox remaining opened, the date of Today in the Agenda tab isn't updated.

After quitting Firefox and restarting it the date is alright.

Reproducible: Didn't try

*** This bug has been marked as a duplicate of 329775 ***
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
I reopen Bug 357079 because this is a slightly different problem than Bug 329775.
Bug 329775 is about update problem if Sunbird/Thunderbird runs overnight.
Bug 357079 is about update problem after System resumes from Hibernate state but was not running overnight.
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
> Bug 357079 is about update problem after System resumes from Hibernate state
> but was not running overnight.
> 

The bug occurs not only by hibernating but by stand-by too:
- Open Ligthning.
- Put the computer in stand-by state.
- Wait the next day.
- Resume the computer.
- The agenda tree is not updated.

Regards, Jacques-D.
More observations:
Sometimes the date does update correctly.  But it appears to only be when there are no events listed in the agenda.  The agenda date has been happily updating most of last week, with no events coming up.  Yesterday (Monday) an event came into view under soon (next Sunday).  Last night the agenda did not update.
*** Bug 358010 has been marked as a duplicate of this bug. ***
Status: UNCONFIRMED → NEW
Component: Calendar Views → Lightning Only
Ever confirmed: true
QA Contact: views → lightning
Version: unspecified → Lightning 0.3
When looking for similar bugs I found Bug 179056 that leads to Bug 282013. 
If I read Bug 282013 correct all timers will be restart after System resumes from Hibernate state. 

So if Thunderbird with Lightning is started at e.g. 23:00 the midnight update timer introduced with Bug 329775 is scheduled to fire after one hour. Now the system is shutdown in hibernate mode while Thunderbird is still running. After resuming from hibernate mode the midnight update timer will be restarted. That means that the Agenda should update about one hour after resuming from hibernate mode.

This behavior would also explain the alarm problems reported in Bug 357706.

Could someone test and verify this behavior please?
Ok, I will test it.

Is this test procedure ok:

1. Start Thunderbird at day time (eg this morning).
2. By leaving the office, put the machine in standby mode.
3. The day after, resume the machine.
4. Wait at least 1 hour.
5. Check the status of agenda tree

BR, Jacques-D.
So, what we need to do:
1.) Register a nsIObserver to listen to the topics defined in https://bugzilla.mozilla.org/attachment.cgi?id=219440&action=diff (specifically 'wake-notification')
2.) When our observer gets called, we should call refreshUIBits();  This will give us a new set of timers, and also reset the views/agenda.
I have test it this night and the agenda tree does not update.

- Got to standby at about 22:30
- Resume at about 07:30
- Wait till 09:30, still wrong date for "Today"

BR, Jacques-D.
Duplicate of this bug: 360766
Taking this bug.
Status: NEW → ASSIGNED
Attached patch Patch v1 for Lightning (obsolete) — Splinter Review
Idea from comment#8.
Attachment #253022 - Flags: second-review?(dmose)
Attachment #253022 - Flags: first-review?(jminta)
Attached patch Patch v1 for Sunbird (obsolete) — Splinter Review
Idea from comment#8.
Attachment #253023 - Flags: second-review?(mvl)
Attachment #253023 - Flags: first-review?(jminta)
Assignee: nobody → mschroeder
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Comment on attachment 253023 [details] [diff] [review]
Patch v1 for Sunbird

r2=mvl
Attachment #253023 - Flags: second-review?(mvl) → second-review+
I tested the Lightning patch on a Windows PC and found a few problems.

First of all the wake_notification is sent twice after awaking from
standby. As refreshUIBits() is not rentrant, the agenda is updated twice
and all agenda items are added twice. refreshUIBits() is not rentrant due
to the fact, that it calls refreshPeriodDates(), which calls
refreshCalendarQuery(). In refreshCalendarQuery() first all agenda items
are deleted and then getItems() is called. If refreshCalendarQuery() is
called again before the getItems() listener is called, the result of both
getItems() calls is added to the agenda tree.

The next problem is, that refreshUIBits() calls scheduleMidnightUpdate()
and therefore sets up a new timer. As a result, we now have two timers
running, which lead at midnight to the same reentrance problem.

In addition, the original timer set up in scheduleMidnightUpdate() will fire
after standby, but only later. If I got it right, then @mozilla.org/timer
is implemented that way, that it fires after the number of milliseconds
which were defined in the delay parameter. If a PC is awaked from standby,
the timer continues and still counts down the number of milliseconds,
but in real time the timer fires after delay + time in standby.
Therefore the original timer fires sometimes on that day and sets up a
new timer for midnight. As a result we now have three timers firing at
next midnight. Even worse, if we set the PC n times a day to standby,
at midnight we have 1+2*n timers firing.

As a conclusion, this bug can be fixed by
a) firing wake_notification only once and
b) cancel the original timer in the wake_notification observer.
In addition the refreshUIBits() function should be reentrant, even if
it's not necessary with those fixes.

Finally I just learned, that one should always keep a reference to the
timer, otherwise the timer doesn't work reliable.
(In reply to comment #15)
> If a PC is awaked from standby, the timer continues and still
> counts down the number of milliseconds,

Even worse, the time is restarted (and not continued) after waking up from sleep mode (Bug 282013). We probably also need a similar solution for the alarm reminders (Bug 357706).
Comment on attachment 253022 [details] [diff] [review]
Patch v1 for Lightning

Martin, can you update this patch in light of comment #15?
Attachment #253022 - Flags: second-review?(dmose)
Attachment #253022 - Flags: first-review?(jminta)
Attachment #253023 - Flags: first-review?(jminta)
(In reply to comment #15)
> As a conclusion, this bug can be fixed by
> a) firing wake_notification only once and

I don't know how to accomplish that.

> b) cancel the original timer in the wake_notification observer.

I have already the code for this problem.

> In addition the refreshUIBits() function should be reentrant, even if
> it's not necessary with those fixes.

I also don't know how to accomplish that.

So, I'm stuck. Maybe, somebody can give me hints.
(In reply to comment #18)
> (In reply to comment #15)
> > As a conclusion, this bug can be fixed by
> > a) firing wake_notification only once and
> 
> I don't know how to accomplish that.

In principal this must be fixed deep in the mozilla core, please grep
for wake_notification in the mozilla directory. But I fear, that this
is probably a long-term adventure. As a short-term hack you can set a
time limit for the two wake_notification events. You reject the second
event, if it's in the time limit.

> > In addition the refreshUIBits() function should be reentrant, even if
> > it's not necessary with those fixes.
> 
> I also don't know how to accomplish that.

The real problem is to make refreshCalendarQuery() reentrant.
This can be done by using a job queue. Please have a look
at the refresh method of the calendar-multiday-view binding.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → NEW
Status: NEW → ASSIGNED
Duplicate of this bug: 374070
Duplicate of this bug: 377171
I observe this behaviour if I don't hibernate or suspend the [Windows] host. Thunderbird 2.0.0.0 (20070326) and Lightning 0.3.1 (2007021403).
Any instructions on how to install the patch please? I am also observing this behavior when resuming from a hibernated state on WinXP.
Scott, the patch is not ready for applying. I'll prepare a new one, but that won't make 0.5. If you build Lightning yourself, you can apply the final patch before the next major release.
Duplicate of this bug: 386718
Until this bug is fixed, is it possible to add a button to the Today pane that allows one to 'reset' to today's date?

I can't imagine that being a difficult task and it would take away the requirement to restart TB every day.
(In reply to comment #26)
> Until this bug is fixed, is it possible to add a button to the Today pane that
> allows one to 'reset' to today's date?
This feature is already on its way, the relevant bug is #388414. For more details on this feature see [1]. The bug specifically deals with introducing the  date control at the top of the today pane.

[1] http://wiki.mozilla.org/Calendar:Improving_the_Calendar_Views#Today_Pane_Switched_On
Martin, are you going to take care of this issue in the 0.7 timeframe?
(In reply to comment #28)
> Martin, are you going to take care of this issue in the 0.7 timeframe?

I'll try to finish it next week.

Attached patch Patch v2 (obsolete) — Splinter Review
(In reply to comment #19)
> (In reply to comment #18)
> > (In reply to comment #15)
> > > As a conclusion, this bug can be fixed by
> > > a) firing wake_notification only once and
> > 
> > I don't know how to accomplish that.
> 
> In principal this must be fixed deep in the mozilla core, please grep
> for wake_notification in the mozilla directory. But I fear, that this
> is probably a long-term adventure. As a short-term hack you can set a
> time limit for the two wake_notification events. You reject the second
> event, if it's in the time limit.
> 

This is no bug! PBT_APMRESUMEAUTOMATIC notifies applications that the computer has woken up automatically to handle an event. An application will not generally respond unless it is handling the event, because the user is not present. If the system detects any user activity after broadcasting PBT_APMRESUMEAUTOMATIC, it will broadcast a PBT_APMRESUMESUSPEND event to let applications know they can resume full interaction with the user. (see http://msdn2.microsoft.com/en-us/library/aa372718.aspx and http://mxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp#4385)

> > > In addition the refreshUIBits() function should be reentrant, even if
> > > it's not necessary with those fixes.
> > 
> > I also don't know how to accomplish that.
> 
> The real problem is to make refreshCalendarQuery() reentrant.
> This can be done by using a job queue. Please have a look
> at the refresh method of the calendar-multiday-view binding.
> 

I implemented it the same way.
Attachment #253022 - Attachment is obsolete: true
Attachment #253023 - Attachment is obsolete: true
Attachment #277382 - Flags: review?
Attachment #277382 - Flags: review? → review?(Berend.Cornelius)
Subscribing to bug
Temporary workaround is to open a message (read or reply) as a new window, close the main window, and then re-open the main window (rerun TB while subwindow is open).

Slightly faster than complete close and reopen, especially if using password manager or if using passworded RSS feeds.
Berend, can you review my patch until the end of the week, or should I switch to another reviewer? The bug should be resolved for 0.7 IMHO.
Attachment #277382 - Flags: review?(Berend.Cornelius) → review?(michael.buettner)
Comment on attachment 277382 [details] [diff] [review]
Patch v2

Basically, this patch adds an observer in order to know when the machine gets notified after hibernation. Although, this is all shiny and good the necessary bits and pieces have been scattered throughout the code instead of honouring locality of dependency. Reduction in locality hurts modularity which introduces a maintenance nightmare. In other words, the observer in addition to the necessary boilerplate code can all be held locally inside of scheduleMidnightUpdate(). Furthermore, this allows to share this code between Sunbird and Lightning, since we would keep it in a shared file.

>     var udCallback = {
>-        notify: function(timer) {
>+        notify: function(gMidnightTimer) {
>             aRefreshCallback();
'timer' is an argument, it hasn't any to do with the global variable 'gMidnightTimer'. No need to rename this.

>+   // Add observer for wake after sleep/hibernate/standby to create new timers and refresh UI
>+   var observerService = Components.classes["@mozilla.org/observer-service;1"]
>+                                   .getService(Components.interfaces.nsIObserverService);
>+   observerService.addObserver(wakeObserver, "wake_notification", false);
You could move this inside scheduleMidnightUpdate().

>+// Observer for wake after sleep/hibernate/standby to create new timers and refresh UI
>+var wakeObserver =
>+{
>+   observe: function (aSubject, aTopic, aData)
>+   {
>+       if (aTopic == "wake_notification") {
>+           refreshUIBits();
>+       }
>+   }
>+};
This would fit perfectly as a local object inside of scheduleMidnightUpdate(). Furthermore, this would allow to call aRefreshCallback() instead of directly calling refreshUIBits(). This would use the callback that is passed to scheduleMidnightUpdate(), instead of reducing generality by directly calling refreshUIBits().

>+   // Remove observer for wake after sleep/hibernate/standby
>+   var observerService = Components.classes["@mozilla.org/observer-service;1"]
>+                                   .getService(Components.interfaces.nsIObserverService);
>+   observerService.removeObserver(wakeObserver, "wake_notification");
This is the last piece that would make sense to be moved into scheduleMidnightUpdate().

>+agendaTreeView.refreshCalendarQuery =
>+function refreshCalendarQuery()
>+{
>+    var refreshJob = {};
>+    this.refreshQueue.push(refreshJob);
>+    this.popRefreshQueue();
>+};
This, as well as the related pieces, look just fine.
Attachment #277382 - Flags: review?(michael.buettner) → review-
Attached patch Patch v3Splinter Review
Attachment #277382 - Attachment is obsolete: true
Attachment #281178 - Flags: review?(michael.buettner)
Comment on attachment 281178 [details] [diff] [review]
Patch v3

Well done, thanks for the patch, Martin. r=mickey.
Attachment #281178 - Flags: review?(michael.buettner) → review+
patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 14 years ago13 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.7
Duplicate of this bug: 403548
You need to log in before you can comment on or make changes to this bug.