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.

Same as bug 533290? In any case, this is annoying, any idea what could be going on there?

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

Assuming bug 488587 is the cause, it's a pity that Firefox passes :-( Perhaps they never use Application.prefs in their code?

Yeah, I plan on asking for a test to be added when we know for sure what the actual cause is...

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

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

Our tree has been perma-orange for too long already...

Does browser_bug463504.js run before or after browser_ApplicationPrefs.js? If it runs first then it might be inadvertently affecting the test.

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

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

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

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

I'm not sure what you mean there; I saw the rename, but I don't understand why.

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

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

(The tests pass in my trunk Linux VM)

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

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

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

(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 ;-)

(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

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.

Hmm, from the description, the problem sounds as if Firefox would have to run into it as well - any idea why they don't?

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

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

You don't have to disable browser_bug431826.js, just patch it not to use Application.prefs, which is what's causing the problem.

Per your comment 33: this comments out part of the change from bug 533210 and reverts code to be like Firefox.

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

Keep FF and SM in sync'.

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

V.Fixed, per tinderboxes.

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

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