1187782 - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nicholas Nethercote [inactive]]

Because iterators are so much nicer than enumerate functions.

There are nine occurrences of EnumerateRead() in these directories.

A note to the assignee: to preserve existing behaviour, you should probably use
nsBaseHashtable::Iterator::UserData() rather than nsBaseHashtable::Iterator::Data(). (The latter should be used when replacing nsBaseHashtable::Enumerate()).

Comment 1

[Nicholas Nethercote [inactive]]

> There are nine occurrences of EnumerateRead() in these directories.

There are now a bunch more because some of the patches in bug 1186780 were backed out.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8679262 [details] [diff] [review]
(part 2) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

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

I think this would be nicer if we made a DisconnectGroupMessageManagers and called it from both places.

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

New version, as requested.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8679264 [details] [diff] [review]
(part 5) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

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

::: dom/base/nsDocument.cpp
@@ +7636,5 @@
> +
> +      // This non-standard style of iteration is presumably used because some
> +      // of the code in the loop body can trigger element removal, which
> +      // invalidates the iterator.
> +      while (true) {

Unfortunately I can't actually look up blame because they blew up the CVS server ...

but yes, I assume that is the concern here.  The loop obviously never exits if BlastSubtreeToPieces and UnsetAttr don't end up removing the attribute from the map.

Comment 10

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/14b02d1a0b2bc79e4804e12f029d477af98a22f7
Bug 1187782 (part 1) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c46fb7b4d2a956d5c277c282bcbf0f3cec17012a
Bug 1187782 (part 2) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/78202fd2b4d525ceb48a759492df86eca1669f2a
Bug 1187782 (part 3) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/40e5654d4bdd3ed4939de46a7ad89288285c6039
Bug 1187782 (part 4) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e2e66bf01dbbf7d5018185e925d975aa6189786
Bug 1187782 (part 5) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

Comment 11

[Nicholas Nethercote [inactive]]

The abandoned cases in bug 1186780 still need to be converted.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/14b02d1a0b2b
https://hg.mozilla.org/mozilla-central/rev/c46fb7b4d2a9
https://hg.mozilla.org/mozilla-central/rev/78202fd2b4d5
https://hg.mozilla.org/mozilla-central/rev/40e5654d4bdd
https://hg.mozilla.org/mozilla-central/rev/1e2e66bf01db

Comment 13

[Nicholas Nethercote [inactive]]

Attachment: (part 6) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 14

[Nicholas Nethercote [inactive]]

Attachment: (part 7) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 15

[Nicholas Nethercote [inactive]]

Attachment: (part 8) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 16

[Nicholas Nethercote [inactive]]

Attachment: (part 8) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

(Fixed a compile error.)

Comment 17

[Nicholas Nethercote [inactive]]

Attachment: (part 9) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 18

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/97aab3af2b2d1097e24fb9bb391baa3eb930d7cd
Bug 1187782 (part 6) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/22f17b724bda3bd67747a12957cd057eddacce38
Bug 1187782 (part 7) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a9d854faf3593509575b682caac87c84bae7a7d
Bug 1187782 (part 8) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6011756702d34de3872314fbcb4b7c85f53b0d1b
Bug 1187782 (part 9) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/97aab3af2b2d
https://hg.mozilla.org/mozilla-central/rev/22f17b724bda
https://hg.mozilla.org/mozilla-central/rev/7a9d854faf35
https://hg.mozilla.org/mozilla-central/rev/6011756702d3

Comment 20

[Nicholas Nethercote [inactive]]

Attachment: (part 10) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 21

[Nicholas Nethercote [inactive]]

Attachment: (part 11) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 22

[Nicholas Nethercote [inactive]]

Attachment: (part 12) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 23

[Nicholas Nethercote [inactive]]

Attachment: (part 13) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 24

[Nicholas Nethercote [inactive]]

Attachment: (part 14) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators

Comment 25

[Nicholas Nethercote [inactive]]

khuey: thank you for the reviews. Your penance for abandoning bug 1186780 is now complete :)

Comment 26

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It says RESOLVED FIXED here ;)

Comment 27

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/973e6cc74295021ff49cd28a6c78ef15bfb50a09
Bug 1187782 (part 10) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e24122a5c63636a63bc0145bf76045a70523509e
Bug 1187782 (part 11) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/973e6cc74295
https://hg.mozilla.org/mozilla-central/rev/e24122a5c636

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/973e6cc74295
https://hg.mozilla.org/mozilla-central/rev/e24122a5c636

Comment 30

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a138819046955099a8ea10a9d885ca1444edacd
Bug 1187782 (part 12) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/696889d8782c8429145aeddddb5d4a3e79ee04bb
Bug 1187782 (part 13) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc19fde35da4ed6f26dad8ff371f291931840e4e
Bug 1187782 (part 14) - Replace nsBaseHashtable::EnumerateRead() calls in dom/base/ with iterators. r=khuey.

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9a1388190469
https://hg.mozilla.org/mozilla-central/rev/696889d8782c
https://hg.mozilla.org/mozilla-central/rev/bc19fde35da4