Last Comment Bug 785860 - Permanent orange: TEST-UNEXPECTED-FAIL | test_sts_preloadlist.js:6: TypeError: Cc['@mozilla.org/privatebrowsing;1'] is undefined
: Permanent orange: TEST-UNEXPECTED-FAIL | test_sts_preloadlist.js:6: TypeError...
Status: RESOLVED FIXED
: intermittent-failure
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: mozilla18
Assigned To: Sid Stamm [:geekboy or :sstamm]
:
Mentors:
Depends on:
Blocks: preload-hsts
  Show dependency treegraph
 
Reported: 2012-08-27 06:54 PDT by Mike Conley (:mconley) - (Away until June 29th)
Modified: 2012-11-25 19:31 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
proposed patch (4.67 KB, patch)
2012-08-27 09:57 PDT, Sid Stamm [:geekboy or :sstamm]
brian: review+
mconley: feedback+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review
Fix missing run_next_test calls (869 bytes, patch)
2012-08-28 03:04 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
conditionally run_next_test (1.81 KB, patch)
2012-08-28 10:24 PDT, David Keeler [:keeler] (use needinfo?)
brian: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description Mike Conley (:mconley) - (Away until June 29th) 2012-08-27 06:54:57 PDT
We've got the following permanent orange showing up on our XPCShell tests:

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
>>>>>>>
### XPCOM_MEM_LEAK_LOG defined -- logging leaks to /tmp/tmpAyQpHa/runxpcshelltests_leaks.log
/home/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js:6: TypeError: Cc['@mozilla.org/privatebrowsing;1'] is undefined
WARNING: nsExceptionService ignoring thread destruction after shutdown: file ../../../../mozilla/xpcom/base/nsExceptionService.cpp, line 166
WARNING: OOPDeinit() without successful OOPInit(): file ../../../../mozilla/toolkit/crashreporter/nsExceptionHandler.cpp, line 2219
nsStringStats
 => mAllocCount:           2231
 => mReallocCount:          185
 => mFreeCount:            2231
 => mShareCount:           7613
 => mAdoptCount:             78
 => mAdoptFreeCount:         78
<<<<<<<

Thunderbird doesn't have the private browsing component registered, so this is why the test is failing.

The test should be altered to account for this possibility - example test: http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/contentprefs/tests/unit/test_bug248970.js#12
Comment 1 Sid Stamm [:geekboy or :sstamm] 2012-08-27 09:57:39 PDT
Created attachment 655626 [details] [diff] [review]
proposed patch

Here's a proposed fix.  mconley: can you check it out and let me know if it works for you?
Comment 2 Mike Conley (:mconley) - (Away until June 29th) 2012-08-27 09:59:28 PDT
Comment on attachment 655626 [details] [diff] [review]
proposed patch

Yes, this looks like it'll do the job.
Comment 3 Sid Stamm [:geekboy or :sstamm] 2012-08-27 10:33:01 PDT
Comment on attachment 655626 [details] [diff] [review]
proposed patch

Honza, can you do a quick review?  The patch is pretty small, and I think you've recently seen this xpcshell test.
Comment 4 Sid Stamm [:geekboy or :sstamm] 2012-08-27 11:58:38 PDT
Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/807627473028
Comment 5 Treeherder Robot 2012-08-27 14:03:24 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14754943&tree=Thunderbird-Trunk
TB Rev3 Fedora 12x64 comm-central opt test xpcshell on 2012-08-27 13:14:12
slave: talos-r3-fed64-030

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 6 Treeherder Robot 2012-08-27 14:03:36 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14754926&tree=Thunderbird-Trunk
TB Rev3 Fedora 12 comm-central opt test xpcshell on 2012-08-27 13:14:12
slave: talos-r3-fed-012

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 7 Treeherder Robot 2012-08-27 14:04:05 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14754564&tree=Thunderbird-Trunk
TB Rev4 MacOSX Snow Leopard 10.6 comm-central opt test xpcshell on 2012-08-27 13:14:20
slave: talos-r4-snow-052

TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 8 Treeherder Robot 2012-08-27 14:04:12 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14753442&tree=Thunderbird-Trunk
TB Rev4 MacOSX Snow Leopard 10.6 comm-central debug test xpcshell on 2012-08-27 11:33:11
slave: talos-r4-snow-077

TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 9 Treeherder Robot 2012-08-27 14:05:16 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14753890&tree=Thunderbird-Trunk
TB Rev3 Fedora 12 comm-central opt test xpcshell on 2012-08-27 11:46:48
slave: talos-r3-fed-075

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 10 Treeherder Robot 2012-08-27 14:54:36 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14755114&tree=Thunderbird-Trunk
TB Rev3 Fedora 12 comm-central debug test xpcshell on 2012-08-27 13:14:12
slave: talos-r3-fed-038

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 11 Treeherder Robot 2012-08-27 16:37:54 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14757325&tree=Thunderbird-Trunk
TB Rev3 Fedora 12 comm-central opt test xpcshell on 2012-08-27 15:10:54
slave: talos-r3-fed-060

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/security/manager/ssl/tests/unit/test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-08-27 19:17:38 PDT
https://hg.mozilla.org/mozilla-central/rev/807627473028
Comment 13 Treeherder Robot 2012-08-27 19:30:35 PDT
RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=14758775&tree=Thunderbird-Trunk
TB Rev3 WINNT 6.1 comm-central opt test xpcshell on 2012-08-27 15:44:08
slave: talos-r3-w7-039

TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\xpcshell\tests\security\manager\ssl\tests\unit\test_sts_preloadlist.js | test failed (with xpcshell return code: 3), see following log:
Comment 14 Mark Banner (:standard8) 2012-08-28 03:00:23 PDT
Unfortunately, this actually causes hangs now on Thunderbird's tests. The cause is that we're missing run_next_test() that should be at the end of each function added via add_test(). What I don't quite understand is how the test currently passes for Firefox.
Comment 15 Mark Banner (:standard8) 2012-08-28 03:04:13 PDT
Created attachment 655932 [details] [diff] [review]
Fix missing run_next_test calls

This fixes the missing run_next_test calls.

Given I'm not sure about the FF case, I've pushed this to try on one platform just to make sure:

https://tbpl.mozilla.org/?tree=Try&rev=b32fbca7a5f7
Comment 16 David Keeler [:keeler] (use needinfo?) 2012-08-28 10:24:35 PDT
Created attachment 656075 [details] [diff] [review]
conditionally run_next_test

We can't unconditionally call run_next_test() because if we are running the private browsing mode tests, we rely on an observer to notify when the private browsing transitions have completed.
Comment 17 Mark Banner (:standard8) 2012-08-28 14:38:20 PDT
(In reply to David Keeler from comment #16)
> Created attachment 656075 [details] [diff] [review]
> conditionally run_next_test
> 
> We can't unconditionally call run_next_test() because if we are running the
> private browsing mode tests, we rely on an observer to notify when the
> private browsing transitions have completed.

Sorry, but I don't understand this. Assuming you are using the globally defined for xpcshell-tests run_next_test, it states:

* Each test function must call run_next_test() when it's done. Test files
* should call run_next_test() in their run_test function to execute all
* async tests. 

http://hg.mozilla.org/mozilla-central/annotate/271ca35d7645/testing/xpcshell/head.js#l857

So I believe the test as-is isn't working correctly.

The actual route it takes, I think, is that once the first run_next_test is called, it queues the first test for running, but because of the do_execute_soon, it doesn't actually queue the test - and instead, run_test() gets to exit before that do_execute_soon activates, and because there's no extra do_test_pending/do_test_finished, the test just exists, because the pending count is 0.

If I'm wrong, I'd be grateful if you could point me to where run_next_test() is being called for the private browsing tests, as that's the bit I'm currently missing.
Comment 18 David Keeler [:keeler] (use needinfo?) 2012-08-28 14:49:40 PDT
This is where run_next_test() gets called:

http://hg.mozilla.org/mozilla-central/file/271ca35d7645/security/manager/ssl/tests/unit/test_sts_preloadlist.js#l29

due to the observer on "private-browsing-transition-complete" (which gets triggered at the end of test_part1 and test_private_browsing1). The issue is we need to wait for that transition to complete before running the next test in each case.
Comment 19 Mark Banner (:standard8) 2012-08-28 23:37:57 PDT
Comment on attachment 655932 [details] [diff] [review]
Fix missing run_next_test calls

Thanks, somehow I'd missed that.
Comment 20 David Keeler [:keeler] (use needinfo?) 2012-08-29 17:03:36 PDT
I've started a try run for firefox: https://tbpl.mozilla.org/?tree=Try&rev=9d16f4036117
Mark - I'm assuming the process for running the patch through thunderbird-try is similar, but I don't know the details. Would you mind taking care of that or telling me how?
Comment 21 Mark Banner (:standard8) 2012-08-30 02:17:02 PDT
(In reply to David Keeler from comment #20)
> I've started a try run for firefox:
> https://tbpl.mozilla.org/?tree=Try&rev=9d16f4036117

OT: I'm surprised that you didn't limit that just to xpcshell.

> Mark - I'm assuming the process for running the patch through
> thunderbird-try is similar, but I don't know the details. Would you mind
> taking care of that or telling me how?

I've done it here:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=b03244136e20

For future reference, instructions here: https://wiki.mozilla.org/ReleaseEngineering/ThunderbirdTryServer
Comment 22 Mark Banner (:standard8) 2012-08-30 02:59:11 PDT
Ignore my previous try push, I'd forgotten to add the actual patch. This one includes the patch:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=46861c0a084b
Comment 23 Mark Banner (:standard8) 2012-08-30 05:18:41 PDT
(In reply to Mark Banner (:standard8) from comment #22)
> Ignore my previous try push, I'd forgotten to add the actual patch. This one
> includes the patch:
> 
> https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=46861c0a084b

Sorry, messed up the file name, one last push:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=0cb753b91d9b
Comment 24 Mark Banner (:standard8) 2012-08-30 08:57:59 PDT
(In reply to Mark Banner (:standard8) from comment #23)
> https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=0cb753b91d9b

Finally this was right and the tests have passed :-)
Comment 25 Mark Banner (:standard8) 2012-08-30 08:58:11 PDT
Thanks.
Comment 26 David Keeler [:keeler] (use needinfo?) 2012-08-30 09:23:07 PDT
(In reply to Mark Banner (:standard8) from comment #21)
> OT: I'm surprised that you didn't limit that just to xpcshell.

d'oh!

> I've done it here:
> 
> https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=b03244136e20
> 
> For future reference, instructions here:
> https://wiki.mozilla.org/ReleaseEngineering/ThunderbirdTryServer

Thanks.

This looks good - marking checkin-needed.
Comment 27 Mark Banner (:standard8) 2012-08-30 13:27:37 PDT
Landed on inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd51f6ca80e2
Comment 28 Ryan VanderMeulen [:RyanVM] 2012-08-30 19:00:13 PDT
https://hg.mozilla.org/mozilla-central/rev/cd51f6ca80e2
Comment 29 Mark Banner (:standard8) 2012-08-31 02:49:18 PDT
Comment on attachment 655626 [details] [diff] [review]
proposed patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 760307 - this causes a perma-orange unit test failure for applications that don't have the private browsing service installed.
User impact if declined: None
Testing completed (on m-c, etc.): Landed on m-c
Risk to taking this patch (and alternatives if risky): None, test-only
String or UUID changes made by this patch: None

Both of the patches on this bug need to be accepted, however the statements are the same for both.
Comment 30 Mark Banner (:standard8) 2012-08-31 02:49:41 PDT
Comment on attachment 656075 [details] [diff] [review]
conditionally run_next_test

See previous comment.
Comment 31 Alex Keybl [:akeybl] 2012-09-04 10:22:42 PDT
Comment on attachment 655626 [details] [diff] [review]
proposed patch

[Triage Comment]
Test-only fix approved for Aurora 17.

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