482598 - Requesting a tinderbox for Mac with --enable-accessibility

Status: VERIFIED FIXED

(Release Engineering :: General, defect, P2)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

We are going to be ramping up Mac a11y development in the Q2 time frame so having a tinderbox to help us catch problems would be great.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Which builds to you mean ? The "build" ones producing dmg's, and/or unit test ?
What's the current status of a11y on the builds you're interested in ? Should we just enable accessibility on the existing mac boxes ? Do you like 20 questions ? :-)

Comment 2

[Marco Zehe (:MarcoZ)]

(In reply to comment #1)
> Which builds to you mean ? The "build" ones producing dmg's, and/or unit test ?

For now, unit test will suffice.
Both the --enable-accessibility MOZCONFIG option must be set (currently disabled for Mac), and the a11y test suite enabled.

> What's the current status of a11y on the builds you're interested in ? Should
> we just enable accessibility on the existing mac boxes ?

If it holds, that would be the preferred option.

> Do you like 20 questions ? :-)

From you always, Nick! ;-)

Comment 3

[[e-mail] (:bhearsum)]

Marco tells me that these tests should pass...I'm going to try this in staging today.

Comment 4

[[e-mail] (:bhearsum)]

Attachment: enable a11y on staging mac unittests

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 367232 [details] [diff] [review]
enable a11y on staging mac unittests

changeset:   1007:981ffd63e076

Comment 6

[[e-mail] (:bhearsum)]

I finally got a run on one of our Mac machines. Looks like there's a leak, but the tests all pass:
     |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->|
                                              Per-Inst   Leaked    Total      Rem      Mean       StdDev     Total      Rem      Mean       StdDev
   0 TOTAL                                          24       68  2302178        2 ( 2530.22 +/-  2775.87)  5732473        2 ( 2522.47 +/-  4377.26)
 158 nsBaseAppShell                                 60       60        1        1 (    1.00 +/-     0.00)     7338        1 (    7.72 +/-     1.08)
 656 nsRunnable                                      8        8     3654        1 (   24.42 +/-    52.33)     7312        1 (   25.20 +/-    52.37)

nsTraceRefcntImpl::DumpStatistics: 887 entries
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 68 bytes during test execution
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 1 instance of nsBaseAppShell with size 60 bytes
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 1 instance of nsRunnable with size 8 bytes

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> and the a11y test suite enabled.

On production:
{
http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py

        if not self.platform == 'macosx':
          self.addStep(unittest_steps.MozillaMochitest, warnOnWarnings=True,
           test_name="mochitest-a11y",
           workdir="build/objdir"
          )
}

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #6)
> I finally got a run on one of our Mac machines. Looks like there's a leak

Ben can you attach the mozconfig used for the accessible mac build? (Just want to follow a wild hunch)

Comment 9

[[e-mail] (:bhearsum)]

ac_add_options --enable-application=browser
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/objdir
ac_add_options --disable-debug
ac_add_options --enable-tests
ac_add_options --enable-optimize
ac_add_options --enable-logrefcnt
ac_add_options --enable-accessibility

export CFLAGS="-gdwarf-2"
export CXXFLAGS="-gdwarf-2"

Comment 10

[[e-mail] (:bhearsum)]

If it helps, the leak happens in one of the test_actions_inputs.html tests.

Comment 11

[[e-mail] (:bhearsum)]

I've been unable to get a proper stack for this leak. I tried --enable-optimize and --enable-debug, neither worked.

I'm not sure where to go on this - did you have any luck, David?

Comment 12

[Marco Zehe (:MarcoZ)]

Tomcat, any good idea how to debug this?

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #11)
> I'm not sure where to go on this - did you have any luck, David?

I couldn't recreate the leak locally (using a debug, unoptimized build).

Ben mention in IRC that sometimes he gets through the run with no leaks.

Comment 14

[David Bolter [:davidb] (NeedInfo me for attention)]

I need to understand our leak tests better, but I wonder if gc could be saving us on the runs that pass?

Comment 15

[Carsten Book [:Tomcat]]

(In reply to comment #12)
> Tomcat, any good idea how to debug this?

Ok, i can try to debug this later this week, but would be great if someone can point me to the tests (like how do i run them)

Comment 16

[[e-mail] (:bhearsum)]

(In reply to comment #15)
> (In reply to comment #12)
> > Tomcat, any good idea how to debug this?
> 
> Ok, i can try to debug this later this week, but would be great if someone can
> point me to the tests (like how do i run them)

Just set XPCOM_MEM_LEAK_LOG=1 and run 'make mochitest-a11y' in your objdir. If it helps, the mozconfig we built with is in comment#9.

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

I've spun off bug 486146 for this confirmed leak, thanks Ben.

I recommend disabling the test_actions_inputs.html so that you can move forward with this bug. We can attack the leak in the spin off.

Comment 18

[Marco Zehe (:MarcoZ)]

Disabled the offending file:
http://hg.mozilla.org/mozilla-central/rev/9754abf5f791

Comment 19

[Serge Gautherie (:sgautherie)]

(In reply to comment #18)
> Disabled the offending file:
> http://hg.mozilla.org/mozilla-central/rev/9754abf5f791

Ftr (for bug 470857), this test is not in 1.9.1 (yet), so MacOSX SeaMonkey should be able to enable it too ... if there is no other issues.

Comment 20

[[e-mail] (:bhearsum)]

With that test disabled I'm re-running an entire unittest build with a11y flipped on in staging.

Comment 21

[[e-mail] (:bhearsum)]

We're still getting a leak. It's looks it's triggered by more than just test_actions_input :(

Comment 22

[Serge Gautherie (:sgautherie)]

Given bug 472773 (and all the "same" bugs),
I would suggest to set up a leak threshold of 68 bytes for MacOSX a11y suite...

Comment 23

[David Bolter [:davidb] (NeedInfo me for attention)]

Serge, thanks for pointing out 472773!

I agree with the threshold workaround for now. I guess we should add that, file a "Remove threshold for MacOSX a11y suite" with 472773 a dependency?

Ben, what do you think?

Comment 24

[Serge Gautherie (:sgautherie)]

This intermittent leak would seem to have gone +/- unnoticed on the other tests suites so far, thus to keep the threshold at 0 there is what we still want.

Now, the question is how often does the a11y suite triggers it ?
We do want the threshold at 68 if "too often", we're fine at 0 if "rather seldom".

Ben, what is the best available way to find this out ?
(Enable the suite in production and monitor what happens ?)

Comment 25

[[e-mail] (:bhearsum)]

I would not want to enable this in production without either the leak being fixed or setting the leak threshold. In my tests it leaked at least 10-20% of the time. I would greatly prefer the leak to be fixed.

Comment 26

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) Force a leak threshold for test suite [Checkin: Comment 34 & 35 & 81]

Not possible to use 0 anymore but who cares: use 1.


(In reply to comment #25)
> In my tests it leaked at least 10-20% of the time.

Want a threshold.

> I would greatly prefer the leak to be fixed.

Bug 472773...

Comment 27

[[e-mail] (:bhearsum)]

(In reply to comment #26)
> Created an attachment (id=370881) [details]
> (Bv1) Force a leak threshold for test suite
> 
> Not possible to use 0 anymore but who cares: use 1.
> 
> 
> (In reply to comment #25)
> > In my tests it leaked at least 10-20% of the time.
> 
> Want a threshold.
> 
> > I would greatly prefer the leak to be fixed.
> 
> Bug 472773...

I have no idea what this comment means.

Also, the leak threshold doesn't belong in automation.py.

Comment 28

[Serge Gautherie (:sgautherie)]

(In reply to comment #27)
> > > I would greatly prefer the leak to be fixed.
> > 
> > Bug 472773...
> 
> I have no idea what this comment means.

It means there is no progress there, yet that should not block here.

> Also, the leak threshold doesn't belong in automation.py.

At least, runtests.py is where such overrides used to be!
Where does this one belong ?
(Is this leak Firefox specific ?)

Comment 29

[[e-mail] (:bhearsum)]

(In reply to comment #28)
> (In reply to comment #27)
> > > > I would greatly prefer the leak to be fixed.
> > > 
> > > Bug 472773...
> > 
> > I have no idea what this comment means.
> 
> It means there is no progress there, yet that should not block here.
> 
> > Also, the leak threshold doesn't belong in automation.py.
> 
> At least, runtests.py is where such overrides used to be!
> Where does this one belong ?
> (Is this leak Firefox specific ?)

Okay, I could be wrong here. Putting known leak numbers into the harness feels dirty to me, but if the automation.py owners are fine with it, I won't object.

Comment 30

[David Bolter [:davidb] (NeedInfo me for attention)]

dbaron pushed a patch on bug 483500 that might have fixed the leak (on central, with approval for 1.9.1... should land soon there I expect). Can we still recreate it?

(note: this is still low priority)

Comment 31

[Serge Gautherie (:sgautherie)]

(In reply to comment #30)
> dbaron pushed a patch on bug 483500 that might have fixed the leak

No, bug 483500 is unrelated to bug 472773.

Comment 32

[Jeff Walden [:Waldo]]

Comment on attachment 370881 [details] [diff] [review]
(Bv1) Force a leak threshold for test suite
[Checkin: Comment 34 & 35 & 81]

The nice thing about the previous setup for overriding leaks was that an explicit leak-threshold overrode this override, but ripping that out simplified the code and was the right thing to do; it's too bad we need it again for this edge case.

I assume someone's working on driving that to 0, right?  68 bytes is probably easy-peasy, and it'd be nice not to have this in for long.

Comment 33

[Jeff Walden [:Waldo]]

Oh, bleh, it's *that* leak.  Wish I knew why it happened, just too bad it's not really possible to use the balance tree for nsBaseAppShell to try and reduce...

Comment 34

[Serge Gautherie (:sgautherie)]

Comment on attachment 370881 [details] [diff] [review]
(Bv1) Force a leak threshold for test suite
[Checkin: Comment 34 & 35 & 81]


http://hg.mozilla.org/mozilla-central/rev/343a04674077


(In reply to comment #32)

Available answers are in comment 26.

Comment 35

[Serge Gautherie (:sgautherie)]

Comment on attachment 370881 [details] [diff] [review]
(Bv1) Force a leak threshold for test suite
[Checkin: Comment 34 & 35 & 81]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bddca23a486a

Comment 36

[[e-mail] (:bhearsum)]

> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bddca23a486a
> http://hg.mozilla.org/mozilla-central/rev/343a04674077

Does this mean we can enable mac a11y?

Comment 37

[Serge Gautherie (:sgautherie)]

(In reply to comment #36)
> Does this mean we can enable mac a11y?

If the mlk was the last blocking issue then it should mean that, yes.

Comment 38

[[e-mail] (:bhearsum)]

(In reply to comment #37)
> (In reply to comment #36)
> > Does this mean we can enable mac a11y?
> 
> If the mlk was the last blocking issue then it should mean that, yes.

AFAIK it was. I'll do one last staging run tomorrow just to be sure, but if that goes well we're good to go!

Comment 39

[[e-mail] (:bhearsum)]

Attachment: enable mac a11y unittests

I had a 100% green test run in staging after the threshold was checked in. It's time to enable mac a11y. Kairo, this patch enables it in the CCUnitestFactory, too - let me know if that's ok or not. I'm not sure if you need to use a different threshold or not...

Comment 40

[David Bolter [:davidb] (NeedInfo me for attention)]

OK just to double check. Will this be an additional tinderbox?  I'm not sure I would want it to replace the non-a11y tinderbox... since Mac FF with a11y is not the default build currently.

Comment 41

[[e-mail] (:bhearsum)]

(In reply to comment #40)
> OK just to double check. Will this be an additional tinderbox?  I'm not sure I
> would want it to replace the non-a11y tinderbox... since Mac FF with a11y is
> not the default build currently.

Argh. This patch is invalid then. Sorry for my misunderstanding....

I don't think we have the build resources do another entire build right now though...

Comment 42

[David Bolter [:davidb] (NeedInfo me for attention)]

Well, I had hoped that we'd be closer to making a11y the default build configuration... I'm working on some issues related to that (with Apple) but it is and low priority right now.

Hopefully build resources grow.

Comment 43

[[e-mail] (:bhearsum)]

Guess enabling it on the existing machines isn't ready yet, bumping this down to a P3.

Comment 44

[Serge Gautherie (:sgautherie)]

(In reply to comment #39)
> Kairo, this patch enables it in the CCUnitestFactory,
> too - let me know if that's ok or not. I'm not sure if you need to use a

SeaMonkey will want to run a11y too,

> different threshold or not...

but you're right that we should first check whether SM needs a threshold for bug 470710 for a11y too or not.


(In reply to comment #40)
> Will this be an additional tinderbox?  I'm not sure I

It would be the existing unit test "box" (only), as Ben "already" did (in his first patch) for the 'staging' environment.

> would want it to replace the non-a11y tinderbox... since Mac FF with a11y is
> not the default build currently.

In comment 2, Marco wrote that enabling it (at least) on the test box was fine.
(And that's what we worked on so far.)
Just to triple check, do you actually mean this solution is not wanted anymore/yet?
(Enabling a11y by default for (all) the other builds would be up to you when it is ready.)


(In reply to comment #41)
> Argh. This patch is invalid then.

Maybe too soon (!?), does need the mozconfig option first, but not invalid...

Comment 45

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #44)
> > would want it to replace the non-a11y tinderbox... since Mac FF with a11y is
> > not the default build currently.
> 
> In comment 2, Marco wrote that enabling it (at least) on the test box was fine.
> (And that's what we worked on so far.)
> Just to triple check, do you actually mean this solution is not wanted
> anymore/yet?

I'll talk to Marco (and try to catch Alexander Surkov as well) tomorrow on this.

Comment 46

[David Bolter [:davidb] (NeedInfo me for attention)]

The main reason we are not ready is that comment #0 is no longer true. Mac a11y is no longer an official Q2 goal.

Comment 47

[Serge Gautherie (:sgautherie)]

(In reply to comment #46)
> Mac a11y is no longer an official Q2 goal.

If there would be (future) drawbacks to run tests (with) a11y, then we should not do it yet.
Otherwise, it seems that we are ready now to enable them, so we should do it, to remove the exception in the buildbot config and get non-regression tests from now on.

Comment 48

[David Bolter [:davidb] (NeedInfo me for attention)]

Ben, as per our IRC chat, I'll go with Marco on this and approve --enable-accessibility for Mac on mozilla central. The users are pacing and we expect to make this a Q3 goal.

Comment 49

[[e-mail] (:bhearsum)]

(In reply to comment #48)
> Ben, as per our IRC chat, I'll go with Marco on this and approve
> --enable-accessibility for Mac on mozilla central. The users are pacing and we
> expect to make this a Q3 goal.

To be absolutely crystal clear the plan is to turn on --enable-accessibility and a11y tests for *only* mac unittest builds on mozilla-central. Tracemonkey and mozilla-1.9.1 unittest builds will remain unchanged. (We should turn it on on Tracemonkey after 3.5 ships, though.)

I will be announcing this today, and plan to make the change next week.

Comment 50

[Robert Kaiser]

That means we need a param on the unittest classes for turning it on/off and have that set from the branches array, right?

Comment 51

[[e-mail] (:bhearsum)]

(In reply to comment #50)
> That means we need a param on the unittest classes for turning it on/off and
> have that set from the branches array, right?

Sortof...because we don't have "branches" for each platform specifically for unittests we need to do it a bit differently. I'll have a patch up shortly with one approach.

Comment 52

[[e-mail] (:bhearsum)]

Attachment: optionally enable mac a11y

Here's one approach to only enabling mac a11y on mozilla-central.

Comment 53

[Chris AtLee [:catlee]]

Comment on attachment 374286 [details] [diff] [review]
optionally enable mac a11y

>+            exec_reftest_suites=True, exec_mochi_suites=True, run_a11y=Tru,
>+            **kwargs):

Looks good except for the typo above.

Comment 54

[Robert Kaiser]

Comment on attachment 374286 [details] [diff] [review]
optionally enable mac a11y

Looks good for me with the typo fix, though I wonder if we should actually default this False in factory.py for now, as else we run a11y tests on all masters that don't have the appropriate changes yet (like production or seamonkey-unittest)

Comment 55

[Serge Gautherie (:sgautherie)]

(In reply to comment #49)
> Tracemonkey and mozilla-1.9.1 unittest builds will remain unchanged.

Out of curiosity, is there a known problem with TM and with 1.9.1?
Or is it only we want to see how 1.9.2 behaves first?

Comment 56

[[e-mail] (:bhearsum)]

(In reply to comment #55)
> (In reply to comment #49)
> > Tracemonkey and mozilla-1.9.1 unittest builds will remain unchanged.
> 
> Out of curiosity, is there a known problem with TM and with 1.9.1?
> Or is it only we want to see how 1.9.2 behaves first?

I don't want to do anything that affects 3.5 - we have enough complications there as it is. We do want to enable a11y on tm at some point, though.

Comment 57

[[e-mail] (:bhearsum)]

(In reply to comment #54)
> (From update of attachment 374286 [details] [diff] [review])
> Looks good for me with the typo fix, though I wonder if we should actually
> default this False in factory.py for now, as else we run a11y tests on all
> masters that don't have the appropriate changes yet (like production or
> seamonkey-unittest)

This is a good point - but I think we'll hit problems either way. If we default UnittestBuildFactory.run_a11y to True the production/seamonkey mac machines will start running it, unless we explicitly tell them not to. If we default it to False, everything in production/seamonkey will stop running a11y tests, unless we explicitly tell them too. Seems like I should patch the production, seamonkey-unittest, and maybe thunderbird-unittest configs to make sure we don't unexpectedly change that.

Comment 58

[[e-mail] (:bhearsum)]

Attachment: explicitly disable mac a11y in production and seamonkey unittests

Here's the same patch plus equivalent production and seamonkey changes.

Comment 59

[[e-mail] (:bhearsum)]

Attachment: one more time, enabling mac a11y in production m-c builds this time [Checkin: See comment 72]

Sorry, my previous patch was wrong, since it didn't enable a11y anywhere in production.

Comment 60

[Robert Kaiser]

Comment on attachment 374515 [details] [diff] [review]
one more time, enabling mac a11y in production m-c builds this time
[Checkin: See comment 72]

Thanks for thinking about this again and fixing that, we'll all sleep better with it :)

>diff -r 6735f61c2812 mozilla2/master.cfg
>--- a/mozilla2/master.cfg	Fri Apr 24 13:23:53 2009 +1200
>+++ b/mozilla2/master.cfg	Fri Apr 24 15:40:21 2009 -0400

Needs a "run_a11y=runA11y," addition in the call as well, right? :P

>diff -r 6735f61c2812 seamonkey-unittest/config.py
>--- a/seamonkey-unittest/config.py	Fri Apr 24 13:23:53 2009 +1200
>+++ b/seamonkey-unittest/config.py	Fri Apr 24 15:40:21 2009 -0400
>@@ -70,6 +70,7 @@
> BRANCHES['comm-central']['platforms']['macosx']['builds_before_reboot'] = None
> # Enable unit tests
> BRANCHES['comm-central']['enable_unittests'] = True
>+BRANCHES['comm-central']['enable_mac_a11y'] = False
> BRANCHES['comm-central']['unittest_build_space'] = 5
> BRANCHES['comm-central']['platforms']['linux']['mochitest_leak_threshold'] = 0
> BRANCHES['comm-central']['platforms']['win32']['mochitest_leak_threshold'] = 200

If you enable in production right away (I thought we were waiting with this, and it would need the mozconfigs changed as well? Or are w just deferring the buildbotcustom changes until then?) then please also enable it for SeaMonkey right away.


Now we only need to make sure we have the config changes deployed _before_ we add the changes to buildbotcustom ;-)

Comment 61

[Serge Gautherie (:sgautherie)]

Comment on attachment 374515 [details] [diff] [review]
one more time, enabling mac a11y in production m-c builds this time
[Checkin: See comment 72]

>diff -r 6735f61c2812 seamonkey-unittest/master.cfg
>+                    run_a11y=runA11y

Nit: may as well add a comma here too.

Comment 62

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1-SM) Add --enable-accessibility to unittest [See bug 492421]

To be clear, MacOSX SM/1.9.1 will end up running this suite (:-)),
whereas MacOSX FF/1.9.1 will not (:-().

Comment 63

[Serge Gautherie (:sgautherie)]

(In reply to comment #44)
> but you're right that we should first check whether SM needs a threshold for
> bug 470710 for a11y too or not.

That bug is now fixed :-)

Comment 64

[Robert Kaiser]

Comment on attachment 374539 [details] [diff] [review]
(Dv1-SM) Add --enable-accessibility to unittest
[See bug 492421]

Umm, right, we are 1.9.1 and this is supposed to be only 1.9.2 and up, let's stay at the same level as Firefox here.

Ben, forget my comment on turning it on for SeaMonkey, I forgot for a moment that we are 1.9.1 still.

Comment 65

[Marco Zehe (:MarcoZ)]

Robert, if you're running any SeaMonkey builds against mozilla-central, too, they could be turned on there, for the post SeaMonkey 2.0.x aera. So this may still be a vaild point, but not for the platform for SeaMonkey 2.

Comment 66

[Robert Kaiser]

Marco, sure, I was just talking about the current configuration we have (we are still waiting for more machines to be able to spin mozilla-central-based builds) and that is being 1.9.1-only still. Of course, that should change in the future and we'll enable mac-a11y for 1.9.2-based builds.

Comment 67

[[e-mail] (:bhearsum)]

(I'm not sure why this bug got fixed1.9.1 in it, since we're not enabling it there...)

Comment 68

[[e-mail] (:bhearsum)]

Attachment: mozconfig changes for a11y

This patch probably isn't the clearest, but here's what it does:
* --enable-accessibility on m-c, tracemonkey
* remove --enable-accessibility on staging 1.9.1 (was previously enabled for testing)

This required a bunch of symlink shuffling in mozilla2{-staging,}/macosx since mozilla-1.9.1 isn't the same as mozilla-central anymore.

Comment 69

[Serge Gautherie (:sgautherie)]

(In reply to comment #67)
> (I'm not sure why this bug got fixed1.9.1 in it, since we're not enabling it
> there...)

Because patch Bv1 was checked in to 1.9.1,
and because, at that time, it seemed the 1.9.1 tinderbox would run a11y too...

Comment 70

[Chris AtLee [:catlee]]

Comment on attachment 375992 [details] [diff] [review]
mozconfig changes for a11y

A bit hard to follow all the symlink rearrangement, but it looks right.

Comment 71

[[e-mail] (:bhearsum)]

Attachment: control a11y tests by parameter rather hardcoding

Comment 72

[[e-mail] (:bhearsum)]

Comment on attachment 374515 [details] [diff] [review]
one more time, enabling mac a11y in production m-c builds this time
[Checkin: See comment 72]

changeset:   1134:603bf322b0f7

Comment 73

[[e-mail] (:bhearsum)]

Comment on attachment 376701 [details] [diff] [review]
control a11y tests by parameter rather hardcoding

changeset:   272:bc15691444b1

Comment 74

[[e-mail] (:bhearsum)]

Comment on attachment 375992 [details] [diff] [review]
mozconfig changes for a11y

changeset:   1133:62f06b05186c

Comment 75

[[e-mail] (:bhearsum)]

This landed without a lot of fanfare. There's a bit of orange on mozilla-central right now, but davidb says he's going to backout the offending tests.

Comment 76

[[e-mail] (:bhearsum)]

The offending test got disabled. We're all done here on the Firefox side. Serge/KaiRo - is there anything you guys need to do here still?

Comment 77

[David Bolter [:davidb] (NeedInfo me for attention)]

Ben, great work! I think this bug is resolved.

Comment 78

[[e-mail] (:bhearsum)]

I caught up with KaiRo on IRC - he says  there's nothing to do for SeaMonkey here. Closing this.

Comment 79

[Serge Gautherie (:sgautherie)]

V.Fixed, per 'OS X 10.5.2 mozilla-central unit test'.

Comment 80

[Marco Zehe (:MarcoZ)]

Now that bug 472773 is marked fixed, can we remove the memory leak threshold so that we will catch other potential memory leaks?

Comment 81

[Serge Gautherie (:sgautherie)]

Comment on attachment 370881 [details] [diff] [review]
(Bv1) Force a leak threshold for test suite
[Checkin: Comment 34 & 35 & 81]


Leak fixed by bug 472773 between
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1245252054.1245257420.32270.gz&fulltext=1
OS X 10.5.2 mozilla-central unit test on 2009/06/17 08:20:54
and
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1245253221.1245262339.9756.gz&fulltext=1
OS X 10.5.2 mozilla-central unit test on 2009/06/17 08:40:21

*****

http://hg.mozilla.org/mozilla-central/rev/126af18be989
(Jv1) Remove previously forced leak threshold