Last Comment Bug 751873 - Permanent orange: TEST-UNEXPECTED-FAIL | test_iteratorUtils.js | TypeError: childArray is null
: Permanent orange: TEST-UNEXPECTED-FAIL | test_iteratorUtils.js | TypeError: c...
Status: VERIFIED FIXED
[perma-orange]
: intermittent-failure, regression
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- blocker (vote)
: Thunderbird 15.0
Assigned To: Mike Conley (:mconley) - (Away until June 29th)
:
Mentors:
Depends on:
Blocks: cpg
  Show dependency treegraph
 
Reported: 2012-05-04 06:15 PDT by Mark Banner (:standard8)
Modified: 2012-11-25 19:31 PST (History)
7 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (1.09 KB, patch)
2012-05-07 07:21 PDT, Mike Conley (:mconley) - (Away until June 29th)
no flags Details | Diff | Review
Patch v2 (5.83 KB, patch)
2012-05-07 10:33 PDT, Mike Conley (:mconley) - (Away until June 29th)
sid.bugzilla: review+
Details | Diff | Review
Patch v3 (carrying over r+ from sid0) (5.91 KB, patch)
2012-05-07 12:28 PDT, Mike Conley (:mconley) - (Away until June 29th)
mconley: review+
Details | Diff | Review

Description Mark Banner (:standard8) 2012-05-04 06:15:02 PDT
Since the compartments landing, we've been seeing this failure:

>>>>>>>

TEST-INFO | (xpcshell/head.js) | test 1 pending

TEST-PASS | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js | [test_toArray_NodeList : 68] rootnode == rootnode

TEST-PASS | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js | [test_toArray_NodeList : 72] true == true

TEST-UNEXPECTED-FAIL | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:74 | TypeError: childArray is null - See following stack:
JS frame :: test_toArray_NodeList@/buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:74
JS frame :: run_test@/buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:123
JS frame :: _execute_test@/buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/head.js:326
JS frame :: @-e:1
<<<<<<<
Comment 1 Mark Banner (:standard8) 2012-05-04 06:21:15 PDT
Links to relevant code:

http://hg.mozilla.org/comm-central/annotate/9b83555adeb2/mailnews/base/util/iteratorUtils.jsm#l58
http://hg.mozilla.org/comm-central/annotate/9b83555adeb2/mailnews/base/test/unit/test_iteratorUtils.js#l74

Changing the function to return just Array.prototype.slice.call(aObj) "worked". Using items like aObj = XPCNativeWrapper.unwrap(aObj) didn't work.
Comment 2 Bobby Holley (busy) 2012-05-04 06:26:50 PDT
As mentioned in irc, it looks like this test relies on .constructor.name for a dom object, which isn't something that can be relied on. It just happened to work for some reason before CPG.
Comment 3 Siddharth Agarwal [:sid0] (inactive) 2012-05-04 06:44:59 PDT
So if NodeLists aren't nsIDOMNodeLists any more and constructor.name is out, how would you check whether something's a node list or iterator in a way that works across globals?
Comment 4 Bobby Holley (busy) 2012-05-04 07:04:48 PDT
You could toString the constructor. You could also just duck-type it and expect it to do the right thing.
Comment 5 Serge Gautherie (:sgautherie) 2012-05-05 07:18:42 PDT
SM 2.12 has this bug too:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1336185053.1336188629.28028.gz
WINNT 5.2 comm-central-trunk debug test xpcshell on 2012/05/04 19:30:53
{
TEST-UNEXPECTED-FAIL | e:\builds\slave\test\build\xpcshell\tests\mailnews\base\test\unit\test_iteratorUtils.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | e:/builds/slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:74 | TypeError: childArray is null - See following stack:
}
Comment 6 Mike Conley (:mconley) - (Away until June 29th) 2012-05-07 07:21:12 PDT
Created attachment 621590 [details] [diff] [review]
Patch v1

Taking bholley's advice and checking the constructor toString.  Test passes with this change.

What do you think, Sid?
Comment 7 Mike Conley (:mconley) - (Away until June 29th) 2012-05-07 10:33:24 PDT
Created attachment 621658 [details] [diff] [review]
Patch v2

Sid gave me a review over IRC.  Addressing his comments in this patch, and adding tests for content iterators as requested.
Comment 8 Siddharth Agarwal [:sid0] (inactive) 2012-05-07 12:20:40 PDT
Comment on attachment 621658 [details] [diff] [review]
Patch v2

Review of attachment 621658 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't run the patch, but r+ assuming tests pass.

::: mail/test/mozmill/utils/test-iteratorUtils.js
@@ +11,5 @@
> +const RELATIVE_ROOT = '../shared-modules';
> +const MODULE_REQUIRES = ['folder-display-helpers',
> +                         'content-tab-helpers',]
> +
> +var iteratorUtils = {};

At this point I think we should exclusively use let for new files.

@@ +27,5 @@
> +
> +  gOriginalWhatsNew = Services.prefs.getCharPref(kWhatsNewPref);
> +  Services.prefs.setCharPref(kWhatsNewPref, gCollectionsUrl);
> +  gTab = open_content_tab_with_click(mc.menus.helpMenu.whatsNew,
> +                                        gCollectionsUrl);

- Please fix the indentation.
- Can we do the open_content_tab outside setupModule?
Comment 9 Mike Conley (:mconley) - (Away until June 29th) 2012-05-07 12:28:54 PDT
Created attachment 621685 [details] [diff] [review]
Patch v3 (carrying over r+ from sid0)

Thanks Sid!  Comments addressed in this patch.
Comment 10 Mike Conley (:mconley) - (Away until June 29th) 2012-05-07 12:32:03 PDT
comm-central: https://hg.mozilla.org/comm-central/rev/945f1b82c5ad
Comment 11 Serge Gautherie (:sgautherie) 2012-05-08 06:23:01 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1336469848.1336473505.24387.gz
WINNT 5.2 comm-central-trunk debug test xpcshell on 2012/05/08 02:37:28

V.Fixed

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