Note: There are a few cases of duplicates in user autocompletion which are being worked on.

mochitest-browser-chrome: perma/random "browser_ApplicationPrefs.js | Timed out" after bug 152526 landing, caused by browser_bug431826.js

VERIFIED FIXED in seamonkey2.1a1

Status

SeaMonkey
Testing Infrastructure
--
major
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {fixed-seamonkey2.0.3, regression, verified1.9.1})

Trunk
seamonkey2.1a1
fixed-seamonkey2.0.3, regression, verified1.9.1
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(status1.9.2 .1-fixed, status1.9.1 .8-fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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

8 years ago
Same as bug 533290?

In any case, this is annoying, any idea what could be going on there?
(Assignee)

Comment 2

8 years ago
(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!
Depends on: 533290

Comment 3

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

Comment 4

8 years ago
Yeah, I plan on asking for a test to be added when we know for sure what the actual cause is...
(Assignee)

Comment 5

8 years ago
[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.
Depends on: 488587
Version: SeaMonkey 2.0 Branch → Trunk
(Assignee)

Comment 6

8 years ago
(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?
Keywords: helpwanted
(Assignee)

Updated

8 years ago
Depends on: 537898
(Assignee)

Comment 7

8 years ago
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...
Attachment #420056 - Flags: review?(neil)

Comment 8

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

Updated

8 years ago
Attachment #420056 - Flags: review?(neil) → review+
(Assignee)

Comment 9

8 years ago
(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...
(Assignee)

Updated

8 years ago
No longer depends on: 537898
(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?
(Assignee)

Comment 11

8 years ago
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)|.
Attachment #420302 - Flags: review?(neil)
(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.
(Assignee)

Comment 13

8 years ago
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.
(Assignee)

Comment 15

8 years ago
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 ;-)
Attachment #420302 - Flags: review?(neil) → review?(kairo)

Updated

8 years ago
Attachment #420302 - Flags: review?(kairo) → review+
(Assignee)

Comment 17

8 years ago
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
Attachment #420302 - Attachment description: (Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order → (Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order [Checkin: Comment 17]
(Assignee)

Comment 18

8 years ago
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
Attachment #420302 - Attachment description: (Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order [Checkin: Comment 17] → (Bv1) Rename (SM) browser_bug463504.js to (FF) browser_bug431826.js, Fix sort order [Checkin: Comment 17+18]
(Assignee)

Updated

8 years ago
Depends on: 539355
(Assignee)

Comment 19

8 years ago
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...
Attachment #421436 - Flags: review?(neil)
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)
(Assignee)

Comment 22

8 years ago
(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.
(Assignee)

Comment 24

8 years ago
(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.
(Assignee)

Comment 25

8 years ago
(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 ;-)
Depends on: 536940
(Assignee)

Updated

8 years ago
Whiteboard: [browser_bug431826.js is disabled on c-191 ftb]
(Assignee)

Comment 26

8 years ago
(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
Whiteboard: [browser_bug431826.js is disabled on c-191 ftb]
(Assignee)

Updated

8 years ago
Summary: mochitest-browser-chrome: perma/random "browser_ApplicationPrefs.js | Timed out" after bug 152526 landing → mochitest-browser-chrome: perma/random "browser_ApplicationPrefs.js | Timed out" after bug 152526 landing, caused by browser_bug431826.js
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

8 years ago
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.
(Assignee)

Comment 30

8 years ago
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.
Attachment #421436 - Flags: review?(neil)
(Assignee)

Comment 32

8 years ago
(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.
(Assignee)

Updated

8 years ago
Depends on: 541398
(Assignee)

Comment 34

8 years ago
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.
Attachment #423002 - Flags: review?(neil)
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.
Attachment #423002 - Flags: review?(neil) → review+
(Assignee)

Comment 36

8 years ago
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).
Attachment #423002 - Attachment description: (Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb. → (Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb [Checkin: See comment 36]
(Assignee)

Updated

8 years ago
Attachment #421436 - Attachment is obsolete: true
(Assignee)

Comment 37

8 years ago
Created attachment 423121 [details] [diff] [review]
(Fv1-FF) Improve a few log messages
[Checkin: Comment 40 & 44 & 45]

Keep FF and SM in sync'.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #423121 - Flags: review?
(Assignee)

Updated

8 years ago
Attachment #423121 - Flags: review? → review?(gavin.sharp)
(Assignee)

Comment 38

8 years ago
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
Attachment #423002 - Attachment description: (Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb [Checkin: See comment 36] → (Dv1) Revert to using "gPrefService" in browser_bug431826.js ftb [Checkin: See comment 38 & 36]
(Assignee)

Updated

8 years ago
Component: General → Testing Infrastructure
Flags: in-testsuite+
Keywords: helpwanted → fixed-seamonkey2.0.3
QA Contact: general → testing-infrastructure
Target Milestone: --- → seamonkey2.1a1
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.
Attachment #423121 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 40

8 years ago
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
Attachment #423121 - Attachment description: (Fv1-FF) Improve a few log messages → (Fv1-FF) Improve a few log messages [Checkin: Comment 40]
(Assignee)

Comment 41

8 years ago
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.
Attachment #420056 - Attachment description: (Av1) Disable test on non-Windows ftb, Improve a few log messages → (Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41]
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 42

8 years ago
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
Attachment #420056 - Attachment description: (Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41] → (Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41 & 42]
(Assignee)

Comment 43

8 years ago
V.Fixed, per tinderboxes.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 44

8 years ago
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
Attachment #420056 - Attachment description: (Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41 & 42] → (Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41 & 43 & 42]
(Assignee)

Updated

8 years ago
Attachment #420056 - Attachment description: (Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41 & 43 & 42] → (Av1) Disable test on non-Windows ftb, Improve a few log messages [Checkin: See comment 41 & 42]
(Assignee)

Updated

8 years ago
Attachment #423121 - Attachment description: (Fv1-FF) Improve a few log messages [Checkin: Comment 40] → (Fv1-FF) Improve a few log messages [Checkin: Comment 40 & 43]
(Assignee)

Updated

8 years ago
status1.9.2: --- → .1-fixed
(Assignee)

Comment 45

8 years ago
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
Attachment #423121 - Attachment description: (Fv1-FF) Improve a few log messages [Checkin: Comment 40 & 43] → (Fv1-FF) Improve a few log messages [Checkin: Comment 40 & 44 & 45]
(Assignee)

Updated

8 years ago
status1.9.1: --- → .8-fixed
(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.
Keywords: verified1.9.1
You need to log in before you can comment on or make changes to this bug.