1337872 - Replace remaining in-tree consumer of non-standard Iterator() in mailnews/ in comm-central

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Tooru Fujisawa [:arai]]

there are 2 remaining consumers of Iterator in mailnews/db/gloda/modules/index_msg.js
will post a patch shortly.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Remove remaining Iterator consumer from gloda.

for (_ in Iterator(obj, true)) can be replaced with for (_ of Object.keys(obj))
and Iterator(fixIterator(_)) can be replaced with XPCOMUtils.IterSimpleEnumerator(_).

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8835007 [details] [diff] [review]
Remove remaining Iterator consumer from gloda.

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

This looks good to me, but since I no longer have review authority, someone with authority like :aceman should review or otherwise sign-off on my review.

::: mailnews/db/gloda/modules/index_msg.js
@@ +1185,2 @@
>            }
> +        } else {

I think you can/should remove this else case here. I'm assuming the very confusing redundant false assignment was the result of a code refactoring I didn't clean-up appropriately in the original landing.  (keepIterHeader is only ever true/false, so there's no deeper meaning.)

Comment 3

[:aceman]

Comment on attachment 8835007 [details] [diff] [review]
Remove remaining Iterator consumer from gloda.

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

Thanks for cleaning up c-c.
Some new weird syntax we get here ;)
Please do what asuth says and then I guess it should be OK.

Comment 4

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/comm-central/rev/6f73de9faa96ace32345213be22656e91d336e93
Bug 1337872 - Remove remaining Iterator consumer from gloda. r=aceman, f=asuth

Comment 5

[Jorg K (CEST = GMT+2)]

Backout:
https://hg.mozilla.org/comm-central/rev/a81dbcf91a42c909817cf47aa9693c20fcfe02a6

Sorry, I had to back this out for the following test failure:
TEST-UNEXPECTED-FAIL | mailnews/db/gloda/test/unit/test_index_compaction.js | xpcshell return code: 0

https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=6f73de9faa96ace32345213be22656e91d336e93&selectedJob=76078291

This failure appeared with this push.

The other four proxy failures are due to bug 791645.

Sorry for the late backout, the tree is too "colourful" currently to see failures immediately.

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Remove remaining Iterator consumer from gloda. r=aceman, f=asuth

fixed scope for msgHdr.

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Remove remaining Iterator consumer from gloda. r=aceman, f=asuth

I think using temporary variable is simpler now.

Comment 8

[Jorg K (CEST = GMT+2)]

Thanks for the quick reaction and sorry about the rude backout ;-) I just thought another bug wasn't worth the hassle, and I wanted to get rid of the failure quickly.

Comment 9

[:aceman]

Sure, sorry for the breakage. A doubly declared variable go hidden in the exotic syntax :)
I should have run the tests first.

Comment 10

[:aceman]

Comment on attachment 8836250 [details] [diff] [review]
Remove remaining Iterator consumer from gloda. r=aceman, f=asuth

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

Try server is busted. This passed for me locally. Thanks for quick fix.

Comment 11

[Tooru Fujisawa [:arai]]

thank you for reviewing :)

Comment 12

[Jorg K (CEST = GMT+2)]

I'll land this after the next M-I to M-C merge. I always like to have a patch "in stock" to trigger a full build. Since it's the weekend, there are less merges, so it might have to wait a day or so.

Comment 13

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/0186ecd2dfab4e5754de6f88ddd988f4bff990dc