Last Comment Bug 534647 - mochitest-browser-chrome: perma/random "browser_ApplicationPrefs.js | Timed out" after bug 152526 landing, caused by browser_bug431826.js
: mochitest-browser-chrome: perma/random "browser_ApplicationPrefs.js | Timed o...
Status: VERIFIED FIXED
: fixed-seamonkey2.0.3, regression, verified1.9.1
Product: SeaMonkey
Classification: Client Software
Component: Testing Infrastructure (show other bugs)
: Trunk
: All All
: -- major (vote)
: seamonkey2.1a1
Assigned To: Serge Gautherie (:sgautherie)
:
:
Mentors:
Depends on: 533290 488587 536940 539355 541398
Blocks: SmTestFail 152526
  Show dependency treegraph
 
Reported: 2009-12-14 09:25 PST by Serge Gautherie (:sgautherie)
Modified: 2010-01-28 06:25 PST (History)
3 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
.1-fixed
.8-fixed


Attachments
(Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41 & 42] (2.33 KB, patch)
2010-01-05 02:45 PST, Serge Gautherie (:sgautherie)
neil: review+
Details | Diff | Splinter Review
(Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order [Checkin: Comment 17+18] (1.36 KB, patch)
2010-01-06 04:05 PST, Serge Gautherie (:sgautherie)
kairo: review+
Details | Diff | Splinter Review
(Cv1) browser_bug431826.js: work around extra about:blank load event (1.54 KB, patch)
2010-01-13 06:31 PST, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
(Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb [Checkin: See comment 38 & 36] (2.59 KB, patch)
2010-01-22 09:08 PST, Serge Gautherie (:sgautherie)
neil: review+
Details | Diff | Splinter Review
(Fv1-FF) Improve a few log messages [Checkin: Comment 40 & 44 & 45] (2.34 KB, patch)
2010-01-22 20:17 PST, Serge Gautherie (:sgautherie)
gavin.sharp: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2009-12-14 09:25:41 PST
Bug 152526 comment 31:
{
Serge Gautherie (:sgautherie)      2009-12-02 07:40:12 PST

[
TEST-PASS |
chrome://mochikit/content/browser/suite/smile/test/browser_ApplicationPrefs.js
| A single preference should not be locked.
TEST-UNEXPECTED-FAIL |
chrome://mochikit/content/browser/suite/smile/test/browser_ApplicationPrefs.js
| Timed out
]
on all platforms.
}

Neil and I have already pushed a few fixes in other bugs.
The situation has improved, but our unit tests tinderboxes are still perma-orange, except Windows which is now random-orange only.

Needs to continue investigations to find out which other test(s) is interfering with this one.
Comment 1 Robert Kaiser 2009-12-14 18:02:28 PST
Same as bug 533290?

In any case, this is annoying, any idea what could be going on there?
Comment 2 Serge Gautherie (:sgautherie) 2009-12-14 18:24:23 PST
(In reply to comment #1)

> Same as bug 533290?

Oh, sure: I would I knew if Neil had linked that bug :-/

> In any case, this is annoying, any idea what could be going on there?

It seems to be +/- explained there!
Comment 3 neil@parkwaycc.co.uk 2009-12-15 14:02:59 PST
Assuming bug 488587 is the cause, it's a pity that Firefox passes :-( Perhaps they never use Application.prefs in their code?
Comment 4 Serge Gautherie (:sgautherie) 2009-12-15 14:14:13 PST
Yeah, I plan on asking for a test to be added when we know for sure what the actual cause is...
Comment 5 Serge Gautherie (:sgautherie) 2009-12-18 16:26:04 PST
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a1pre) Gecko/20091218
SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)

Bug still there: not fixed by bug 533355.
Comment 6 Serge Gautherie (:sgautherie) 2009-12-18 18:52:55 PST
(In reply to comment #0)
> Need to continue investigations to find out which other test(s) is interfering
> with this one.

I tried with my comment 5 build, but it's tedious as this failure is now intermittent on Windows.

Helpwanted: Could someone else do it on Linux or MacOSX?
Comment 7 Serge Gautherie (:sgautherie) 2010-01-05 02:45:56 PST
Created attachment 420056 [details] [diff] [review]
(Av1) Disable test on non-Windows ftb, Improve a few log messages
[Checkin: See comment 41 & 42]

Our tree has been perma-orange for too long already...
Comment 8 neil@parkwaycc.co.uk 2010-01-05 16:11:07 PST
Does browser_bug463504.js run before or after browser_ApplicationPrefs.js? If it runs first then it might be inadvertently affecting the test.
Comment 9 Serge Gautherie (:sgautherie) 2010-01-06 02:16:23 PST
(In reply to comment #8)

chrome://mochikit/content/browser/suite/common/tests/browser/browser_bug463504.js
does run before
chrome://mochikit/content/browser/suite/smile/test/browser_ApplicationPrefs.js

(In reply to comment #6)
> Helpwanted: Could someone else do it on Linux or MacOSX?

Still applies: I'll delay checking in a little...
Comment 10 neil@parkwaycc.co.uk 2010-01-06 02:49:19 PST
(In reply to comment #9)
>(In reply to comment #8)
>chrome://mochikit/content/browser/suite/common/tests/browser/browser_bug463504.js
>does run before
>chrome://mochikit/content/browser/suite/smile/test/browser_ApplicationPrefs.js
Well that makes it a possible "culprit". Could we try temporarily disabling it?
Comment 11 Serge Gautherie (:sgautherie) 2010-01-06 04:05:05 PST
Created attachment 420302 [details] [diff] [review]
(Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order
[Checkin: Comment 17+18]

I missed to do that previously: it makes it more obvious which files should be in sync'.


(In reply to comment #10)
> Well that makes it a possible "culprit". Could we try temporarily disabling it?

This reminded me of:

Bug 533176 comment 1:
{
Serge Gautherie (:sgautherie)      2009-12-06 17:00:59 PST

I'll post a workaround I have for that if need be, but I prefer to leave this
"unrelated" issue for later...
}

Before attaching a patch with that workaround,
I would just push the current patch on branch then try |$(warning browser_bug431826.js)|.
Comment 12 neil@parkwaycc.co.uk 2010-01-07 13:59:09 PST
(In reply to comment #11)
> I missed to do that previously: it makes it more obvious which files should be
> in sync'.
Which files are already in sync? None of those other five appear to be.
Comment 13 Serge Gautherie (:sgautherie) 2010-01-07 21:44:06 PST
Comment on attachment 420302 [details] [diff] [review]
(Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order
[Checkin: Comment 17+18]


(In reply to comment #12)
> Which files are already in sync? None of those other five appear to be.

Argh, the following part doesn't show up in bugzilla diff :-/
{
diff --git a/suite/common/tests/browser/browser_bug463504.js b/suite/common/tests/browser/browser_bug431826.js
rename from suite/common/tests/browser/browser_bug463504.js
rename to suite/common/tests/browser/browser_bug431826.js
}
Comment 14 neil@parkwaycc.co.uk 2010-01-08 09:24:16 PST
I'm not sure what you mean there; I saw the rename, but I don't understand why.
Comment 15 Serge Gautherie (:sgautherie) 2010-01-08 09:32:34 PST
Ah, well,
http://mxr.mozilla.org/comm-central/source/suite/common/tests/browser/browser_bug463504.js
should stay in sync with
http://mxr.mozilla.org/mozilla-central/source/browser/components/certerror/test/browser_bug431826.js
as much as possible.
Having same name makes that more obvious, ease MXR file search, ...

I didn't check the other tests (names): I'm just aligning/sorting them.
Comment 16 neil@parkwaycc.co.uk 2010-01-11 03:26:46 PST
Comment on attachment 420302 [details] [diff] [review]
(Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order
[Checkin: Comment 17+18]

I'm still unconvinced either way, so here's hoping KaiRo can decide for me ;-)
Comment 17 Serge Gautherie (:sgautherie) 2010-01-12 15:28:54 PST
Comment on attachment 420302 [details] [diff] [review]
(Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order
[Checkin: Comment 17+18]


http://hg.mozilla.org/comm-central/rev/6f191f4def81
http://hg.mozilla.org/releases/comm-1.9.1/rev/f17cff9471d9
Comment 18 Serge Gautherie (:sgautherie) 2010-01-12 16:59:52 PST
Comment on attachment 420302 [details] [diff] [review]
(Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order
[Checkin: Comment 17+18]


http://hg.mozilla.org/comm-central/rev/ebda35e9d815
http://hg.mozilla.org/releases/comm-1.9.1/rev/2b9df1f26d4e
(Cv1) Bustage fix
Comment 19 Serge Gautherie (:sgautherie) 2010-01-13 06:31:26 PST
Created attachment 421436 [details] [diff] [review]
(Cv1) browser_bug431826.js: work around extra about:blank load event

This is the workaround I was talking about in bug 533176 comment 1.
I can't reproduce the random failure on my local Windows anymore :-/

Before pushing patch A, I'd like to try this one:
it would be even better if someone could try it locally on Linux/MacOSX...
Comment 20 neil@parkwaycc.co.uk 2010-01-13 06:41:04 PST
Comment on attachment 421436 [details] [diff] [review]
(Cv1) browser_bug431826.js: work around extra about:blank load event

>   gBrowser.selectedTab = gBrowser.addTab();
Is this the "about:blank" addTab bug?
Comment 21 neil@parkwaycc.co.uk 2010-01-13 06:47:09 PST
(The tests pass in my trunk Linux VM)
Comment 22 Serge Gautherie (:sgautherie) 2010-01-13 09:34:08 PST
(In reply to comment #20)
> Is this the "about:blank" addTab bug?

I don't know: what is that bug?

(In reply to comment #21)
> (The tests pass in my trunk Linux VM)

Good. With or without my patch?
Though I'm most interested in SeaMonkey 2.0 (tinderboxes)...
Comment 23 neil@parkwaycc.co.uk 2010-01-13 09:42:11 PST
(In reply to comment #22)
> (In reply to comment #20)
> > Is this the "about:blank" addTab bug?
> I don't know: what is that bug?
That's the bug where calling addTab("about:blank") doesn't load about:blank (because it's the default) but calling addTab() does. It got fixed on trunk.

> (In reply to comment #21)
> > (The tests pass in my trunk Linux VM)
> Good. With or without my patch?
Without. Also my VM only has a trunk Linux build.
Comment 24 Serge Gautherie (:sgautherie) 2010-01-18 07:55:29 PST
(In reply to comment #11)
> (In reply to comment #10)
> > Could we try temporarily disabling it?
> 
> I would [...] try |$(warning browser_bug431826.js)|.

http://hg.mozilla.org/releases/comm-1.9.1/rev/a783a17de6cd
(Dv1-191) Disable browser_bug431826.js.
Comment 25 Serge Gautherie (:sgautherie) 2010-01-18 10:52:21 PST
(In reply to comment #23)
> That's the bug where calling addTab("about:blank") doesn't load about:blank
> (because it's the default) but calling addTab() does. It got fixed on trunk.

That would be bug 536940 ;-)
Comment 26 Serge Gautherie (:sgautherie) 2010-01-19 06:23:00 PST
(In reply to comment #24)
> http://hg.mozilla.org/releases/comm-1.9.1/rev/a783a17de6cd
> (Dv1-191) Disable browser_bug431826.js.

Backout after confirmation:
http://hg.mozilla.org/releases/comm-1.9.1/rev/0f09c0194431
http://hg.mozilla.org/releases/comm-1.9.1/rev/ea93572525a5
Comment 27 neil@parkwaycc.co.uk 2010-01-19 06:34:05 PST
So what happens is that browser_bug463504^H^H^H^H^H^H431826.js invokes Application.prefs which registers a weak preference observer. XPConnect manages to forget that there's still a reference to it and allows the xptcstub to get cycle collected between the two tests, if cycle collection runs. Then when browser_ApplicationPrefs.js runs it fails because the observer won't fire.

I filed the xptcstub cycle collection bug as bug 533290.
Comment 28 Robert Kaiser 2010-01-19 06:49:45 PST
Hmm, from the description, the problem sounds as if Firefox would have to run into it as well - any idea why they don't?
Comment 29 neil@parkwaycc.co.uk 2010-01-19 07:17:11 PST
(In reply to comment #28)
> Hmm, from the description, the problem sounds as if Firefox would have to run
> into it as well - any idea why they don't?
To run in to the problem, the XPConnect stub for the Application._prefs object has to be garbage collected before the test completes. So it depends on whether any of their other tests use Application._prefs and in which order.
Comment 30 Serge Gautherie (:sgautherie) 2010-01-19 09:56:19 PST
Comment on attachment 421436 [details] [diff] [review]
(Cv1) browser_bug431826.js: work around extra about:blank load event


Bug 536940 landed on c-1.9.1, "but" this failure is still there.
I'd like to try(!) this workaround (on branch) nontheless: I'll back it out if it doesn't help at all.
Comment 31 neil@parkwaycc.co.uk 2010-01-20 04:29:17 PST
Comment on attachment 421436 [details] [diff] [review]
(Cv1) browser_bug431826.js: work around extra about:blank load event

Sorry, but I don't see the how this patch relates to the browser_ApplicationPrefs.js test failure.
Comment 32 Serge Gautherie (:sgautherie) 2010-01-20 04:50:47 PST
(In reply to comment #31)

This workaround helped on my local Windows, before a few other issues were fixed...
Now, do you prefer that I re-disable browser_bug431826.js or land patch A?
Comment 33 neil@parkwaycc.co.uk 2010-01-20 04:55:33 PST
You don't have to disable browser_bug431826.js, just patch it not to use Application.prefs, which is what's causing the problem.
Comment 34 Serge Gautherie (:sgautherie) 2010-01-22 09:08:34 PST
Created attachment 423002 [details] [diff] [review]
(Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb
[Checkin: See comment 38 & 36]

Per your comment 33:
this comments out part of the change from bug 533210 and reverts code to be like Firefox.
Comment 35 neil@parkwaycc.co.uk 2010-01-22 09:17:09 PST
Comment on attachment 423002 [details] [diff] [review]
(Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb
[Checkin: See comment 38 & 36]

>+// Workaround until Application.prefs can be used safely. (Bug 534647)
>+var gPrefService =
>+  Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
Nit: declare in full, like browser_bug515006/524345 please.
Comment 36 Serge Gautherie (:sgautherie) 2010-01-22 16:55:51 PST
Comment on attachment 423002 [details] [diff] [review]
(Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb
[Checkin: See comment 38 & 36]


http://hg.mozilla.org/releases/comm-1.9.1/rev/b3c0757d7437
Dv1, with comment 35 suggestion(s).
Comment 37 Serge Gautherie (:sgautherie) 2010-01-22 20:17:01 PST
Created attachment 423121 [details] [diff] [review]
(Fv1-FF) Improve a few log messages
[Checkin: Comment 40 & 44 & 45]

Keep FF and SM in sync'.
Comment 38 Serge Gautherie (:sgautherie) 2010-01-22 20:22:08 PST
Comment on attachment 423002 [details] [diff] [review]
(Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb
[Checkin: See comment 38 & 36]


http://hg.mozilla.org/comm-central/rev/0e2fd3b00c96
Comment 39 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-01-23 21:10:21 PST
Comment on attachment 423121 [details] [diff] [review]
(Fv1-FF) Improve a few log messages
[Checkin: Comment 40 & 44 & 45]

This seems like a waste of time. Whether messages are positive or negative hardly matters, as long as they're unique enough to find the correct failure case.
Comment 40 Serge Gautherie (:sgautherie) 2010-01-24 06:05:51 PST
Comment on attachment 423121 [details] [diff] [review]
(Fv1-FF) Improve a few log messages
[Checkin: Comment 40 & 44 & 45]


http://hg.mozilla.org/mozilla-central/rev/56a74ab99f43
Comment 41 Serge Gautherie (:sgautherie) 2010-01-24 06:11:22 PST
Comment on attachment 420056 [details] [diff] [review]
(Av1) Disable test on non-Windows ftb, Improve a few log messages
[Checkin: See comment 41 & 42]


http://hg.mozilla.org/comm-central/rev/80f4c1737e2b
'Improve a few log messages' part only.
Comment 42 Serge Gautherie (:sgautherie) 2010-01-24 19:34:22 PST
Comment on attachment 420056 [details] [diff] [review]
(Av1) Disable test on non-Windows ftb, Improve a few log messages
[Checkin: See comment 41 & 42]


http://hg.mozilla.org/releases/comm-1.9.1/rev/19f1cc8894f3
Comment 43 Serge Gautherie (:sgautherie) 2010-01-24 19:36:01 PST
V.Fixed, per tinderboxes.
Comment 44 Serge Gautherie (:sgautherie) 2010-01-26 10:40:52 PST
Comment on attachment 420056 [details] [diff] [review]
(Av1) Disable test on non-Windows ftb, Improve a few log messages
[Checkin: See comment 41 & 42]


http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cc946b1b0fba
Comment 45 Serge Gautherie (:sgautherie) 2010-01-26 10:50:49 PST
Comment on attachment 423121 [details] [diff] [review]
(Fv1-FF) Improve a few log messages
[Checkin: Comment 40 & 44 & 45]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c4a9da11f7e4
Comment 46 Henrik Skupin (:whimboo) 2010-01-28 06:25:32 PST
(In reply to comment #45)
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c4a9da11f7e4

Since the check-in the tree didn't turn orange due to this described issue. Marking as verified1.9.1.

Note You need to log in before you can comment on or make changes to this bug.