The default bug view has changed. See this FAQ.

Compose windows are no longer recycled

VERIFIED FIXED in Thunderbird 23.0

Status

Thunderbird
Message Compose Window
--
major
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

(Blocks: 1 bug, {regression})

17 Branch
Thunderbird 23.0
x86
All
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird20 wontfix, thunderbird21 wontfix, thunderbird22+ fixed, thunderbird-esr1722+ fixed)

Details

(Whiteboard: [regression:tb17][regression started from Tb 17 by 777063] )

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Bug 777063 has a logic error in its code to obtain an nsIXULWindow from an nsIDOMWindow so it fails to recycle any compose windows.
(Assignee)

Comment 1

4 years ago
Created attachment 742490 [details] [diff] [review]
Proposed patch

Transferred from bug 814651 as requested.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #742490 - Flags: review?(mbanner)
Attachment #742490 - Flags: feedback?(m-wada)
Blocks: 814651
Blocks: 818607
Whiteboard: [regression started from Tb 17 by 777063]
Version: Trunk → 17
Whiteboard: [regression started from Tb 17 by 777063] → [regression:tb17][regression started from Tb 17 by 777063]
Comment on attachment 742490 [details] [diff] [review]
Proposed patch

Review of attachment 742490 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for new crisp bug for this regression.

(In reply to neil@parkwaycc.co.uk from comment #0)
> so it fails to recycle any compose windows.

Was the failure "don't recycle" only?
  Is the failure simply "worked as if mail.compose.max_recycled_windows=0 always" only?
  Is possible resource leak issue involved in the "fails to recycle"?
  (resource leak issue of bug 818607 even after fix of Bug 765074)
Attachment #742490 - Flags: feedback?(m-wada)
(Assignee)

Comment 3

4 years ago
(In reply to WADA from comment #2)
> (In reply to comment #0)
> > so it fails to recycle any compose windows.
> 
> Was the failure "don't recycle" only?
>   Is the failure simply "worked as if mail.compose.max_recycled_windows=0
>   always" only?
That appears to be the case, yes.

>   Is possible resource leak issue involved in the "fails to recycle"?
>   (resource leak issue of bug 818607 even after fix of Bug 765074)
Fixing this bug may conceal the leak because of the recycling effect.
(Presumably setting max_recycled_windows=0 would then expose the leak again.)
Comment on attachment 742490 [details] [diff] [review]
Proposed patch

Yep, looks good. Tested the compose window is now cached.
Attachment #742490 - Flags: review?(mbanner) → review+
tracking-thunderbird22: --- → +
tracking-thunderbird-esr17: --- → 22+
(Assignee)

Comment 5

4 years ago
Pushed comm-central changeset c448a8a204be.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-thunderbird23: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 23.0
Ccked with following trunk nightly build.
> Mozilla/5.0 (Windows NT 5.1; rv:23.0) Gecko/20100101 Thunderbird/23.0a1
> Application Build ID 	: 20130504031036

(0) mail.compose.max_recycled_windows=5
(1) Open compose window, get DOM and XUL window via Window Enumerator.
    Call window objects "DOM-win-X" and "XUL-win-X".
    DOM-win-X.isCachedWin = false
(2) Keep reference to the DOM-win-X, XUL-win-X
(3) Close composition window, with keeping reference to window objects.
    i.e. "destory XUL window" event notification is ignored.
    => Window object is not returned by DOM and XUL Window Enumerator
    => DOM-win-X.isCachedWin = true
(4) Open composition window again
    => Same window object is returned by DOM and XUL Window Enumerator
    => DOM-win-X.isCachedWin = false
Composition window is cached and is re-used as designed => VERIFIED
Status: RESOLVED → VERIFIED
This bug causes severe memory leak problem of bug 818607(memory leak by Bug 765074), which was exposed to Tb 17 users due to this bug, even though many Tb users use mail.compose.max_recycled_windows>0(default is 1 or 5).
I believe patch of this bug is better landed on Tb 17 ESR.
(Assignee)

Comment 8

4 years ago
Comment on attachment 742490 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Regression caused by (bug #): 777063
User impact if declined: Performance impacted
  (compose windows slow to open and close and leak memory)
Testing completed (on c-c, etc.): Verified on c-c
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None
Attachment #742490 - Flags: approval-comm-esr17?
Attachment #742490 - Flags: approval-comm-beta?
Attachment #742490 - Flags: approval-comm-aurora?
Comment on attachment 742490 [details] [diff] [review]
Proposed patch

a=me for Aurora.

For esr17, we'll wait until the next cycle, so this has some testing (and we're going to cut it in the next 24 hours ish).

It is up to SeaMonkey if they want to land this on beta for the next release (no more TB betas for this cycle), but there's an implicit a=me if they want it.
Attachment #742490 - Flags: approval-comm-aurora? → approval-comm-aurora+
https://hg.mozilla.org/releases/comm-aurora/rev/7c915bfda901
status-thunderbird22: affected → fixed
status-thunderbird23: fixed → ---
Attachment #742490 - Flags: approval-comm-beta?
Attachment #742490 - Flags: approval-comm-esr17? → approval-comm-esr17+
https://hg.mozilla.org/releases/comm-esr17/rev/cec6ac1b39f6
status-thunderbird20: affected → wontfix
status-thunderbird21: affected → wontfix
status-thunderbird-esr17: affected → fixed
You need to log in before you can comment on or make changes to this bug.