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

Status: RESOLVED FIXED

(SeaMonkey :: Testing Infrastructure, defect)

Description

[Serge Gautherie (:sgautherie)]

While working on bug 726505, I noticed bug 658738 comment 102 changeset...

Comment 1

[Serge Gautherie (:sgautherie)]

... 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.

Comment 2

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Port bug 658738 comment 17 (Add missing removeEventListener() calls), Fix a mismatch in browser_394759.js [Checked in: Comment 4]

Comment 3

[Serge Gautherie (:sgautherie)]

... and none of bug 658738 blocking bugs fixed in so far affects SeaMonkey.

Comment 4

[Serge Gautherie (:sgautherie)]

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

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures| [Checked in: Comment 10]

A step to more sync'.

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) browser_394759.js: Port FF changeset cd888c5d77f0 and changeset a6fe7bb368a9, Add browserWindowsCount() calls, Do 3 s/gBrowser/getBrowser()/ [Checked in: Comment 19]

*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.

Comment 7

[Serge Gautherie (:sgautherie)]

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.

Comment 8

[[e-mail]]

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 9

[Serge Gautherie (:sgautherie)]

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 [e-mail] 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

Comment 10

[Serge Gautherie (:sgautherie)]

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 11

[Justin Wood (: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]

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.

Comment 12

[Serge Gautherie (:sgautherie)]

Jens, context has changed on -central, please apply it manually to -aurora. Thanks.

Comment 13

[Jens Hatlak (:InvisibleSmiley)]

(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.

Comment 14

[Serge Gautherie (:sgautherie)]

Attachment: (BBv1-29) Port |Bug 595292 - browser_394759.js seems to hit network with dns failures| [Checkin: Comment 16]

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.

Comment 15

[Serge Gautherie (:sgautherie)]

(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 16

[Jens Hatlak (:InvisibleSmiley)]

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

Comment 17

[Jens Hatlak (:InvisibleSmiley)]

[No idea whether this is supposed to "fix" 2.9, so leaving flags untouched.]

Comment 18

[[e-mail]]

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.

Comment 19

[Serge Gautherie (:sgautherie)]

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

Comment 20

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) Port bug 658738 comment 102 (split browser_394759.js into three tests) [Checked in: Comments 22 and 25]

Retained differences with Firefox:
Services.prefs + getBrowser() + listener names + nits.

Comment 21

[Serge Gautherie (:sgautherie)]

(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.

Comment 22

[Serge Gautherie (:sgautherie)]

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.

Comment 23

[Serge Gautherie (:sgautherie)]

Let's file blocking bugs if more work is needed.

Comment 24

[Justin Wood (:Callek)]

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.

Comment 25

[Jens Hatlak (:InvisibleSmiley)]

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

Comment 26

[Serge Gautherie (:sgautherie)]

(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.