Closed Bug 726521 Opened 8 years ago Closed 8 years ago

Port |Bug 658738 - [meta] We seem to be leaking hundreds of windows until shutdown during browser-chrome tests| to SeaMonkey

Categories

(SeaMonkey :: Testing Infrastructure, defect)

defect
Not set

Tracking

(seamonkey2.8 wontfix, seamonkey2.9 fixed, seamonkey2.10 fixed)

RESOLVED FIXED
seamonkey2.11
Tracking Status
seamonkey2.8 --- wontfix
seamonkey2.9 --- fixed
seamonkey2.10 --- fixed

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(Blocks 1 open bug)

Details

(Keywords: meta, Whiteboard: [fixed in SM 2.9: BBv1-29; SM 2.10: Av1, Bv1, Cv1, Dv1])

Attachments

(5 files)

While working on bug 726505, I noticed bug 658738 comment 102 changeset...
... and the only other changeset to port (so far) is bug 658738 comment 17 changeset.

*****

In the end,
http://mxr.mozilla.org/comm-central/search?string=addEventListener&case=1&find=%2Fsuite%2F.*%2Ftest.*%2Fbrowser
"Found 151 matching lines in 54 files"
and
http://mxr.mozilla.org/comm-central/search?string=removeEventListener&case=1&find=%2Fsuite%2F.*%2Ftest.*%2Fbrowser
"Found 148 matching lines in 50 files"
should match, in total and also wrt each test.
... and none of bug 658738 blocking bugs fixed in so far affects SeaMonkey.
Blocks: 724446
Attachment #597284 - Flags: review?(neil) → review+
Comment on attachment 597284 [details] [diff] [review]
(Av1) Port bug 658738 comment 17 (Add missing removeEventListener() calls), Fix a mismatch in browser_394759.js
[Checked in: Comment 4]

http://hg.mozilla.org/comm-central/rev/074ad890780f
Attachment #597284 - Attachment description: (Av1) Port bug 658738 comment 17 (Add missing removeEventListener() calls), Fix a mismatch in browser_394759.js → (Av1) Port bug 658738 comment 17 (Add missing removeEventListener() calls), Fix a mismatch in browser_394759.js [Checked in: Comment 4]
Flags: in-testsuite+
Target Milestone: --- → seamonkey2.10
Blocks: 724441
Blocks: 734153
Depends on: 595292
A step to more sync'.
Attachment #604432 - Flags: review?(bugspam.Callek)
Attachment #604432 - Flags: approval-comm-beta?
Attachment #604432 - Flags: approval-comm-aurora?
Depends on: 633711
Blocks: 510890, 633711
No longer depends on: 633711
*FF changeset cd888c5d77f0 and changeset a6fe7bb368a9: usual Dao's "no bug no review".
*browserWindowsCount() calls: missed in SM bug 633711.
*s/gBrowser/getBrowser()/: mistakenly undone in SM bug 510890.
Attachment #604464 - Flags: review?(neil)
Comment on attachment 604432 [details] [diff] [review]
(Bv1) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures|
[Checked in: Comment 10]

Whoever reviews this first.
Attachment #604432 - Flags: review?(neil)
Attachment #604432 - Flags: review?(neil) → review+
Comment on attachment 604464 [details] [diff] [review]
(Cv1) browser_394759.js: Port FF changeset cd888c5d77f0 and changeset a6fe7bb368a9, Add browserWindowsCount() calls, Do 3 s/gBrowser/getBrowser()/
[Checked in: Comment 19]

r=me on the window/win rename; where was the browserWindowsCount() added?
Comment on attachment 604464 [details] [diff] [review]
(Cv1) browser_394759.js: Port FF changeset cd888c5d77f0 and changeset a6fe7bb368a9, Add browserWindowsCount() calls, Do 3 s/gBrowser/getBrowser()/
[Checked in: Comment 19]

(In reply to neil@parkwaycc.co.uk from comment #8)
> where was the browserWindowsCount() added?

In Firefox?
http://hg.mozilla.org/mozilla-central/rev/37f7608bae72
http://hg.mozilla.org/mozilla-central/rev/13601aff9814
Attachment #604432 - Attachment description: (Bv1) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures| → (Bv1) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures| [Checked in: Comment 10]
Attachment #604432 - Flags: review?(bugspam.Callek)
Comment on attachment 604432 [details] [diff] [review]
(Bv1) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures|
[Checked in: Comment 10]

http://hg.mozilla.org/comm-central/rev/3eaa70af5151
Comment on attachment 604432 [details] [diff] [review]
(Bv1) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures|
[Checked in: Comment 10]

too late for beta, That is already in comm-release now, and I don't want to take this in comm-release, if we get this into aurora before tuesday then it will be in beta as of then.
Attachment #604432 - Flags: approval-comm-beta?
Attachment #604432 - Flags: approval-comm-beta-
Attachment #604432 - Flags: approval-comm-aurora?
Attachment #604432 - Flags: approval-comm-aurora+
Jens, context has changed on -central, please apply it manually to -aurora. Thanks.
Keywords: checkin-needed
Whiteboard: [c-n: 3eaa70af5151 to c-a]
(In reply to Serge Gautherie (:sgautherie) from comment #12)
> Jens, context has changed on -central, please apply it manually to -aurora.

Now this is an understatement. The file to patch isn't there, and has only been created as a copy of browser_bug510890.js through changeset cff76f26dd7b (landed Wed Feb 15). Clearing c-n request.
Keywords: checkin-needed
Whiteboard: [c-n: 3eaa70af5151 to c-a]
Same as Bv1, but for comm-aurora/2.9.


(In reply to Jens Hatlak (:InvisibleSmiley) from comment #13)
> copy of browser_bug510890.js

Sorry, I had forgotten it had been renamed.
Keywords: checkin-needed
Whiteboard: [c-n: BBv1-29 to c-a]
(In reply to Serge Gautherie (:sgautherie) from comment #10)
> http://hg.mozilla.org/comm-central/rev/3eaa70af5151

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331364554.1331369643.15790.gz&fulltext=1
OS X 10.5 comm-central-trunk debug test mochitest-other on 2012/03/09 23:29:14

still succeeds.
Comment on attachment 604643 [details] [diff] [review]
(BBv1-29) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures| [Checkin: Comment 16]

http://hg.mozilla.org/releases/comm-aurora/rev/c0666f5a239c
Attachment #604643 - Attachment description: (BBv1-29) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures|. r=neil a-comm-aurora=Callek → (BBv1-29) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures|. r=neil a-comm-aurora=Callek [Checkin: Comment 16]
[No idea whether this is supposed to "fix" 2.9, so leaving flags untouched.]
Keywords: checkin-needed
Whiteboard: [c-n: BBv1-29 to c-a]
Comment on attachment 604464 [details] [diff] [review]
(Cv1) browser_394759.js: Port FF changeset cd888c5d77f0 and changeset a6fe7bb368a9, Add browserWindowsCount() calls, Do 3 s/gBrowser/getBrowser()/
[Checked in: Comment 19]

Ah, I see now, misak added browserWindowsCount() but not the checks.
Attachment #604464 - Flags: review?(neil) → review+
Comment on attachment 604464 [details] [diff] [review]
(Cv1) browser_394759.js: Port FF changeset cd888c5d77f0 and changeset a6fe7bb368a9, Add browserWindowsCount() calls, Do 3 s/gBrowser/getBrowser()/
[Checked in: Comment 19]

http://hg.mozilla.org/comm-central/rev/5d0e20c97905
Attachment #604464 - Attachment description: (Cv1) browser_394759.js: Port FF changeset cd888c5d77f0 and changeset a6fe7bb368a9, Add browserWindowsCount() calls, Do 3 s/gBrowser/getBrowser()/ → (Cv1) browser_394759.js: Port FF changeset cd888c5d77f0 and changeset a6fe7bb368a9, Add browserWindowsCount() calls, Do 3 s/gBrowser/getBrowser()/ [Checked in: Comment 19]
Attachment #604643 - Attachment description: (BBv1-29) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures|. r=neil a-comm-aurora=Callek [Checkin: Comment 16] → (BBv1-29) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures| [Checkin: Comment 16]
Retained differences with Firefox:
Services.prefs + getBrowser() + listener names + nits.
Attachment #605052 - Flags: review?(neil)
(In reply to Serge Gautherie (:sgautherie) from comment #1)
> In the end,
> http://mxr.mozilla.org/comm-central/
> search?string=addEventListener&case=1&find=%2Fsuite%2F.*%2Ftest.*%2Fbrowser
> "Found 151 matching lines in 54 files"
> and
> http://mxr.mozilla.org/comm-central/
> search?string=removeEventListener&case=1&find=%2Fsuite%2F.*%2Ftest.
> *%2Fbrowser
> "Found 148 matching lines in 50 files"
> should match, in total and also wrt each test.

Add   : "Found 152 matching lines in 54 files"
Remove: "Found 151 matching lines in 53 files"

The differences in counts are:
*http://mxr.mozilla.org/comm-central/find?text=&string=browser_form_restore_events_sample.html
 has 2 document.addEventListener(), which should automatically go away with the document.
*http://mxr.mozilla.org/comm-central/find?string=browser_popupNotification.js
 has an extra (conditional) removeEventListener() call in its cleanup(), as a safeguard.

Assuming all the others match correctly, /suite should be fine wrt that.
Attachment #605052 - Flags: review?(neil) → review+
Comment on attachment 605052 [details] [diff] [review]
(Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests)
[Checked in: Comments 22 and 25]

http://hg.mozilla.org/comm-central/rev/5bdb6dfb3e00


[Approval Request Comment]
Just to go along the rest of this bug.
Attachment #605052 - Attachment description: (Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests) → (Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests) [Checked in: Comment 22]
Attachment #605052 - Flags: approval-comm-beta?
Attachment #605052 - Flags: approval-comm-aurora?
Let's file blocking bugs if more work is needed.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: meta
Resolution: --- → FIXED
Target Milestone: seamonkey2.10 → seamonkey2.11
Depends on: 741070
Comment on attachment 605052 [details] [diff] [review]
(Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests)
[Checked in: Comments 22 and 25]

Beta is already in release repo, and while I could still take final build with a beta-approved patch tonight, I'm not taking this.
Attachment #605052 - Flags: approval-comm-beta?
Attachment #605052 - Flags: approval-comm-beta-
Attachment #605052 - Flags: approval-comm-aurora?
Attachment #605052 - Flags: approval-comm-aurora+
Keywords: checkin-needed
Whiteboard: [c-n: 5bdb6dfb3e00 to c-a]
Comment on attachment 605052 [details] [diff] [review]
(Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests)
[Checked in: Comments 22 and 25]

http://hg.mozilla.org/releases/comm-aurora/rev/2e0666f06d6b
Attachment #605052 - Attachment description: (Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests) [Checked in: Comment 22] → (Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests) [Checked in: Comments 22 and 25]
Keywords: checkin-needed
Whiteboard: [c-n: 5bdb6dfb3e00 to c-a]
Whiteboard: [fixed in SM 2.9: BBv1-29; SM 2.10: Av1, Bv1, Cv1, Dv1]
(In reply to Justin Wood (:Callek) from comment #24)
> (Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests)
> 
> Beta is already in release repo, and while I could still take final build
> with a beta-approved patch tonight, I'm not taking this.

Agreed: that request had stalled for too long and bug 741070 isn't fixed yet.
You need to log in before you can comment on or make changes to this bug.