The default bug view has changed. See this FAQ.

shutting down calendar doesn't wait for in-progress writes to finish

RESOLVED FIXED

Status

Calendar
Internal Components
--
major
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: dmose, Assigned: Joey Minta)

Tracking

({dataloss})

Trunk
dataloss

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.95 KB, patch
Michiel van Leeuwen (email: mvl+moz@)
: first-review+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
This was spun off of bug 218983.
(Reporter)

Updated

11 years ago
Whiteboard: [cal relnote]
(Assignee)

Comment 1

11 years ago
Created attachment 225896 [details] [diff] [review]
proposal

This is how I see this bug possibly being fixed.  It's really tough to test and I'm not sure I understand all the nuances.  Another option is to piggy-back off the tryToClose window function in the main UI.  This would make it a bit easier to prompt to tell the user what's going on.  Feedback?
(In reply to comment #1)
> Another option is to piggy-back off the tryToClose window function in the
> main UI.

Note that closing the main window does NOT quit the app on Mac, so that approach may not work across the board
(Assignee)

Comment 3

11 years ago
(In reply to comment #2)
> Note that closing the main window does NOT quit the app on Mac, so that
> approach may not work across the board
> 
There's still hiddenWindow.xul that we could use.

dmose and I wanted to get mconnor's opinion on this, as he knows this code better.
(Assignee)

Updated

11 years ago
Severity: normal → major
Flags: blocking0.3+
(Assignee)

Comment 4

11 years ago
mconnor points to: http://lxr.mozilla.org/mozilla/source/xpfe/components/startup/public/nsIAppStartup.idl#88

mail seems to use this quite extensively: http://lxr.mozilla.org/mozilla/source/mail/components/nsMailDefaultHandler.js#82
(Assignee)

Comment 5

11 years ago
Created attachment 229313 [details] [diff] [review]
i will survive

Patch successfully keeps our app around until we finish the write.  Tested by delaying writing with a really big for-loop.
Assignee: base → jminta
Attachment #225896 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #229313 - Flags: first-review?(mvl)
(Assignee)

Updated

11 years ago
Whiteboard: [cal relnote] → [cal relnote][patch in hand]
Comment on attachment 229313 [details] [diff] [review]
i will survive

>                 try  {
>+                    appStartup.enterLastWindowClosingSurvivalArea();
>                 } catch (ex) {
>+                    appStartup.exitLastWindowClosingSurvivalArea();
>                 }

Looks like you want a finally block there, instead of putting the other call to exitLastWindowClosingSurvivalArea in a completly different place in the code.
(Assignee)

Comment 7

11 years ago
(In reply to comment #6)
> Looks like you want a finally block there, instead of putting the other call to
> exitLastWindowClosingSurvivalArea in a completly different place in the code.
> 
The other call is made after an observer is notified following an async call.  (after the write actually finished) |finally| has no way, that I know of, to handle that.
Comment on attachment 229313 [details] [diff] [review]
i will survive

whoops, you are right. async code still manages to confuse me...
r=mvl
Attachment #229313 - Flags: first-review?(mvl) → first-review+
(Assignee)

Comment 9

11 years ago
patch checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: [cal relnote][patch in hand] → [patch in hand]
Whiteboard: [patch in hand]
You need to log in before you can comment on or make changes to this bug.